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

Generic/[Upper|Lower]CaseConstant: ignore type declarations #3429

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Sep 6, 2021

This commit adjusts the Generic.PHP.UpperCaseConstant and the Generic.PHP.LowerCaseConstant sniffs to ignore property, parameter and return type declarations.

Type declarations have their own rules and should not be treated the same as other false, true and null uses.

Includes unit tests.

Fixes #3332

This commit adjusts the `Generic.PHP.UpperCaseConstant` and the `Generic.PHP.LowerCaseConstant` sniffs to ignore property, parameter and return type declarations.

Type declarations have their own rules and should not be treated the same as other `false`, `true` and `null` uses.

Includes unit tests.

Fixes 3332
@jrfnl jrfnl force-pushed the feature/3332-generic-upperlowercaseconstant-ignore-type-declarations branch from 022c14c to 2c4f3cc Compare December 12, 2021 21:52
@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 12, 2021

Rebased to get a passing build on PHP 8.1.

gsherwood added a commit that referenced this pull request Jan 16, 2022
@gsherwood gsherwood merged commit 0f31749 into squizlabs:master Jan 16, 2022
@gsherwood
Copy link
Member

Thanks for changing these sniffs. I ended up having upper extend lower to remove much of the duplicate logic and to force these to always be in sync.

@jrfnl jrfnl deleted the feature/3332-generic-upperlowercaseconstant-ignore-type-declarations branch January 17, 2022 00:21
@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 17, 2022

Thanks for changing these sniffs. I ended up having upper extend lower to remove much of the duplicate logic and to force these to always be in sync.

Makes sense as these sniffs will (should) never be used within the same standard as they contradict each other.

I've become a bit wary of sniffs extending each other after seeing too many problems with that in external standards, but in this case it shouldn't be problematic.

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.

Generic.PHP.UpperCaseConstant: don't check null and false types
2 participants