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

Enhancement: Implement Methods\FinalInAbstractClassRule #123

Merged
merged 2 commits into from
Sep 13, 2019

Conversation

localheinz
Copy link
Member

@localheinz localheinz commented Sep 13, 2019

This PR

  • implements a Methods\FinalInAbstractClassRule, which reports an error when a concrete public or protected method in an abstract class is not final
  • declares a concrete method in an abstract class as final

Fixes #88.

@localheinz localheinz self-assigned this Sep 13, 2019
@localheinz localheinz marked this pull request as ready for review September 13, 2019 14:03
@codecov
Copy link

codecov bot commented Sep 13, 2019

Codecov Report

Merging #123 into master will decrease coverage by 0.4%.
The diff coverage is 76.19%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #123      +/-   ##
============================================
- Coverage     84.28%   83.88%   -0.41%     
- Complexity      118      125       +7     
============================================
  Files            20       21       +1     
  Lines           401      422      +21     
============================================
+ Hits            338      354      +16     
- Misses           63       68       +5
Impacted Files Coverage Δ Complexity Δ
src/Methods/FinalInAbstractClassRule.php 76.19% <76.19%> (ø) 7 <7> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab8765e...974f34b. Read the comment docs.


abstract class AbstractClassWithPublicMethod
{
protected function method(): void

Choose a reason for hiding this comment

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

Should be public

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you, @OskarStark!

README.md Outdated

```neon
parameters:
allowAbstractClasses: true

Choose a reason for hiding this comment

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

Is the name correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for taking a look, but yes, it's correct - it's the same parameter that is currently used by Localheinz\PHPStan\Rules\Classes\FinalRule, see rules.neon.

On the other hand, perhaps the parameter is not even needed.

@localheinz localheinz force-pushed the feature/methods-final-in-abstract-class-rule branch 4 times, most recently from d7177d3 to 7ed2763 Compare September 13, 2019 15:15
@localheinz localheinz force-pushed the feature/methods-final-in-abstract-class-rule branch from c8d37a4 to 4b1ed80 Compare September 13, 2019 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FinalMethodRule
2 participants