Edit report at https://pear.php.net/bugs/bug.php?id=20357&edit=1
ID: 20357
Updated by: [email protected]
Reported By: aik dot bold at gmail dot com
Summary: The "FunctionComment" mistaking DocBlock comments
Status: Feedback
Type: Bug
Package: PHP_CodeSniffer
Operating System: Linux
Package Version: 1.5.3
PHP Version: 5.4.20
Assigned To: squiz
Roadmap Versions:
New Comment:
> I can't include the comment parser in 2.x because it's just a mess.
But the
comment start token in 2.x is different from 1.x, so it's easy to detect
which
version you are in.
I see. But we can create helper class in same file as the
CodingStandard_Sniffs_Commenting_FunctionCommentSniff that will have 2
methods:
- getShortComment()
- getReturn()
Then we check if there is no commentParser property on the sniff, then
it's PHPCS 2.x
and we put our fake replacement class there that will do token-based
search. I guess
that will be in next PR.
> To get started, I've created a PR here:
https://github.com/aik099/CodingStandard/pull/24
Looks exactly as we discussed. Thanks. I guess it's ready to merge?
> If you think it is still working, I'll take a look at what needs to be
done to include
2.x support over there.
Please do so.
Previous Comments:
------------------------------------------------------------------------
[2014-08-15 03:29:35] squiz
I can't include the comment parser in 2.x because it's just a mess. But
the
comment start token in 2.x is different from 1.x, so it's easy to detect
which
version you are in.
To get started, I've created a PR here:
https://github.com/aik099/CodingStandard/pull/24
This doesn't make the FunctionComment sniff compatible with 2.x, but
hopefully
makes it easier in the future.
The unit tests still pass, so I'm hopefully just doing the same thing a
slightly
different way.
If you think it is still working, I'll take a look at what needs to be
done to include
2.x support over there.
The InlineComment sniff should be easier because it doesn't extend
anything.
------------------------------------------------------------------------
[2014-08-14 11:48:45] aik099
> Instead of overriding the process() method, it may actually be better
to just include
the sniff as is, then mute the ShortNotCapital error in your custom
coding standard.
Then you can include a custom sniff that only checks this one thing.
Yes, that maybe a solution indeed. Error hiding by code is done through
setting 0 severity if I'm not mistaken. Hopefully in 2.x
version the original Squiz_Sniffs_Commenting_FunctionCommentSniff is
checking for exact same things as it's version in 1.x.
However I still need to use CommentParser->getShortComment() to get the
short comment to validate. Is there any possibility to
keep CommentParser class with it's public API, but in 2.x it will just
do token-based analysis. This way whoever are using it in
sniffs won't need to change much.
> You've also modified the code that checks the return tag, so you could
mute a
bunch of errors in there as well.
Same approach as with short comment check. That sounds good.
> Or, to make it slightly better, you could write a custom sniff that
includes and runs
the new DocCommentSniff only if running on 2.x.
1. In the ruleset.xml we can always include
Squiz_Sniffs_Commenting_FunctionCommentSniff and mute "ShortNotCapital"
error
(that won't match to any error from 2.0 version of this sniff though)
2. Create sniff that will check for short description of a function
(ways of obtaining short description differs from 1.x and 2.x and
that's a problem)
3. create a clever sniff, that will determine PHPCS version used and:
- for 2.x include DocCommentSniff instead of it (with muted error
message for "ShortNotCapital")
- for 1.x do nothing
- we can't mention "DocCommentSniff" in ruleset.xml I guess, because
this would result in Exception in PHPCS 1.x
Problem with @return tag processing stays. It's the
Squiz_Sniffs_Commenting_FunctionCommentSniff which is aware:
- when function comment starts and passes that stack pointer to
"processReturn"
- how to get return content
Having some migration layer that will use tokens for 2.x and
CommentParser for 1.x is still needed I'm afraid.
Ideas?
------------------------------------------------------------------------
[2014-08-14 07:00:40] squiz
Should also say, if it is easier, contact me directly at: gsherwood at
squiz dot com dot
au
------------------------------------------------------------------------
[2014-08-14 07:00:05] squiz
Sorry it's taken so long, but I have taken a look at the function
comment sniff and I
can see what you've changed.
The bad news is that the sniff is drastically different in 2.x, to the
point where most
of the checks actually live in a different sniff now:
https://github.com/squizlabs/PHP_CodeSniffer/blob/phpcs-
fixer/CodeSniffer/Standards/Generic/Sniffs/Commenting/DocCommentSniff.php
Instead of overriding the process() method, it may actually be better to
just include
the sniff as is, then mute the ShortNotCapital error in your custom
coding standard.
Then you can include a custom sniff that only checks this one thing.
You've also modified the code that checks the return tag, so you could
mute a
bunch of errors in there as well.
But the worse news is that doing this, while cleaner, means that you
would need a
different ruleset.xml file for 1.x and 2.x.
Or, to make it slightly better, you could write a custom sniff that
includes and runs
the new DoCommentSniff only if running on 2.x.
I'd need to have a bit of a play around to get something going. Before I
do, does
this sound like something you would considered doing? Specifically:
- including all of the Squiz (or PEAR) FunctionCommentSniff but turning
off some of
the errors
- including the DocCommentSniff in your standard, probably via a custom
sniff
- including a custom sniff (probably the same one as above) that checks
your event
function short descriptions
BW, I haven't checked the inline comment sniff yet.
------------------------------------------------------------------------
[2014-08-07 09:53:01] aik099
Link in above comment has been wrapped by pear bug tracker and became
broken as
a result. Beware.
Actually if it's easier for you, then you can compare the Squiz/PEAR
version of
FunctionComment sniff to mine, then you'll note some minor differences.
This way
might be easier to determine what needs to be changed.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
http://pear.php.net/bugs/bug.php?id=20357
--
Edit this bug report at https://pear.php.net/bugs/bug.php?id=20357&edit=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.