Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude class methods with inheritdoc for Type rules #18

Merged
merged 1 commit into from
Mar 9, 2021

Conversation

matks
Copy link
Collaborator

@matks matks commented Mar 8, 2021

Questions Answers
Description? Handle @inheritdoc class methods by excluding them. See why below.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes half of #15
How to test? Ci is 😊

Why exclude class methods that have {@inheritDoc} tag?

Without this PR, when the rules UseTypeHintForNewMethodsRule and seTypedReturnForNewMethodsRule find a class method without typed parameters and type return, they consider it a violation.

When we encounter a class method with {@inheritDoc} though, it indicates it extends a parent method.
If parent method

  • is part of the exclusion list, it's not a violation => that's already handled
  • is not part of PrestaShop code, but comes from a dependency => we should not flag it as a violation

If the parent method is not part of the exclusion list and is part of PrestaShop code, the UseTypeHintForNewMethodsRule and seTypedReturnForNewMethodsRule will also find it and flag it as a violation!

So we can safely say "if this method has has {@inheritDoc} tag, then either the parent method tag will be flagged invalid as well or we can assume it's not a violation". This is why we can ignore them.

@matks matks changed the title Handle inheritdoc class methods Exclude class methods with inheritdoc for Type rules Mar 8, 2021
@@ -19,7 +19,8 @@
"phpstan/phpstan-phpunit": "^0.12.16",
"phpstan/phpstan-strict-rules": "^0.12.5",
"phpunit/phpunit": "^7.5.20",
"symfony/console": "^5.2"
"symfony/console": "^5.2",
"nikic/php-parser": "^4.10"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed in PhpDocAnalyzer use PhpParser\Comment\Doc

@kpodemski
Copy link
Contributor

Rebase needed?

@matks
Copy link
Collaborator Author

matks commented Mar 9, 2021

Rebase done

@matks matks merged commit 7bdd8ca into PrestaShop:main Mar 9, 2021
@matks matks deleted the inheritdoc branch March 9, 2021 13:10
@matks matks added this to the 1.1.1 milestone Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants