Report @inheritDoc when there is no PHPDoc to inherit from#5583
Report @inheritDoc when there is no PHPDoc to inherit from#5583kubawerlos wants to merge 4 commits intophpstan:2.1.xfrom
@inheritDoc when there is no PHPDoc to inherit from#5583Conversation
|
I think we should distinguish two different case
|
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
The correct way to find parent methods is ParentMethodHelper.
|
@VincentLanglet do you mean as separate rules? |
|
Not a class, but an error identifier for sure. |
kubawerlos
force-pushed
the
invalid-inherit-doc-tag
branch
from
bc204fc to
fd71add
Compare
|
Hi @kubawerlos could you rebase and solve the conflict ? |
| continue; | ||
| } | ||
|
|
||
| if (preg_match(self::INLINE_INHERIT_DOC_REGEX, $child->text, $matches) !== 1) { |
There was a problem hiding this comment.
Can't we do better ?
I know it'll be an edge case but you won't catch description like
/**
* Please do not add `{@inheritDoc}` to this method
*/
There was a problem hiding this comment.
I'm not very good in regex but looking at the test called InheritDocInsideBackticks i'm not sure we understood each other.
My point was that, we might not want to consider that a method with
/**
* Foo @inheritDoc
*/
has an inheritDoc, cause it might be just a comment.
It wasn't related to backticks
(On the opposite,
/**
* @inheritDoc Bar
*/
might be considered as an inherit doc with an extra comment... ; I dunno)
I never use inheritDoc so I can't stay what we can find in codebase.
There was a problem hiding this comment.
Updated test.
@inheritDoc Bar will be tokenised as a tag with extra description.
kubawerlos
force-pushed
the
invalid-inherit-doc-tag
branch
from
fd71add to
00f4776
Compare
| if ($parentMethods === []) { | ||
| return [ | ||
| RuleErrorBuilder::message(sprintf( | ||
| 'PHPDoc tag %s on method %s::%s() does not override or implement any other method.', |
There was a problem hiding this comment.
I feel this can be improved.
| 'PHPDoc tag %s on method %s::%s() does not override or implement any other method.', | |
| 'PHPDoc tag %s on method %s::%s() refers to non-existent parent method.', |
... I am pretty sure ondrej has some concrete ideas about it :)
| final class InvalidInheritDocTagRule implements Rule | ||
| { | ||
|
|
||
| private const INLINE_INHERIT_DOC_REGEX = '~`[^`]*`(*SKIP)(*FAIL)|(?<![a-zA-Z0-9])\{@inheritDoc\b[^}]*\}~i'; |
There was a problem hiding this comment.
can this regex be simplified because it uses i modifier (case-less), so the pattern itself does not need to handle both upper and lower case things?
| break; | ||
| } | ||
|
|
||
| if ($inheritDocTagName === null) { |
There was a problem hiding this comment.
I am not sure about this additional regex matching on-top of regular phpDoc parsing.
all relevant cases should be detected by the phpdoc parser, if it is doing a good job.
if not we should improve the phpdoc parser IMO
staabm
left a comment
staabm
left a comment
There was a problem hiding this comment.
take my feedback with a grain of salt. I think this is a gray area which needs @ondrejmirtes opinion.
4 participants
Closes phpstan/phpstan#5561