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

NamingConventions/ObjectNameDepth: bug fix, support modern PHP and other improvements #334

Merged
merged 8 commits into from
Nov 4, 2023

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Nov 4, 2023

NamingConventions/ObjectNameDepth: decouple from WPCS sniff

.... as extending the WPCS sniff is no longer needed, as the utility functions we used from that sniff before, have all been replaced with utilities from PHPCSUtils and utilities from the new WPCS 3.0.0 Helper classes/traits.

Follow up on PRs #322 and #323.

NamingConventions/ObjectNameDepth: add extra tests

... for a few edge cases previously not covered by tests.

Includes updating a couple of pre-existing tests to expand the scope of what these are testing.

NamingConventions/ObjectNameDepth: minor documentation improvements

NamingConventions/ObjectNameDepth: bug fix for test classes using CamelCaps

Test classes and Double/Mock classes are allowed one extra word in the name to account for the Test/Mock/Double.

However, this was not handled correctly for test classes in CamelCaps as those are first converted to snake_case, which means that the suffix used in the comparison is now lowercase, while the name comparison was done in a case-sensitive manner.

Along the same lines, underscore separated class names where the suffix would not be "proper case" would also not be recognized correctly as, again, the name comparison was done in a case-sensitive manner.

As the casing of the class name is not the concern of this sniff, the name comparison should be done in a case-INsensitive manner.

Fixed now.

Includes tests.

NamingConventions/ObjectNameDepth: allow for TestCase classes

Aside from a generic TestCase, typically called TestCase, a test suite may contain more specialized TestCase classes with longer names.

Those classes should also get the "one extra word" treatment (with TestCase counted as one word).

Fixed now.

Includes tests.
Includes update to the XML docs.

NamingConventions/ObjectNameDepth: remove some redundant code

The WPCS IsUnitTestTrait::is_test_class() method does not (and never has so far) handle import use statements, which means that in the context of namespaced files, the results of the function are unreliable.

As documented with the TEST_SUFFIXES constant, the Test and TestCase suffixes should only be given the "one extra word" treatment when these classes extend a "known" test class.

As things were, this wasn't respected properly by the sniff anyhow (the $this->is_test_class() should have been nested within the self::TEST_SUFFIXES[ $last ] === true condition).

And as it cannot reliably be enforced at this time, we're better off removing the useless condition and just checking if a class with one of the "extra word" suffixes extends.

NamingConventions/ObjectNameDepth: examine PHP 8.1+ enums

The names of enum structures should comply with the same rules as classes.

This commit:

  • Adds the T_ENUM token to the tokens being sniffed for.
  • Makes an exception in the "one extra word for test/mocks/double" logic as enums cannot extend, though the may still need to be mocked.
  • Ensures the error message for enums is grammatically correct.

Includes tests.
Includes update to the XML docs.

NamingConventions/ObjectNameDepth: support PHP 8.2+ readonly classes

To check whether a class is deprecated, the sniff needs to skip over the potential class modifier keywords, like final and abstract.

As of PHP 8.2, it will also need to skip over the new readonly keyword.

Includes unit tests.

jrfnl added 8 commits November 4, 2023 11:47
.... as extending the WPCS sniff is no longer needed, as the utility functions we used from that sniff before, have all been replaced with utilities from PHPCSUtils and utilities from the new WPCS 3.0.0 Helper classes/traits.

Follow up on PRs 322 and 323.
... for a few edge cases previously not covered by tests.

Includes updating a couple of pre-existing tests to expand the scope of what these are testing.
…elCaps

Test classes and Double/Mock classes are allowed one extra word in the name to account for the `Test`/`Mock`/`Double`.

However, this was not handled correctly for test classes in CamelCaps as those are first converted to `snake_case`, which means that the suffix used in the comparison is now lowercase, while the name comparison was done in a case-sensitive manner.

Along the same lines, underscore separated class names where the suffix would not be "proper case" would also not be recognized correctly as, again, the name comparison was done in a case-sensitive manner.

As the casing of the class name is not the concern of this sniff, the name comparison should be done in a case-INsensitive manner.

Fixed now.

Includes tests.
Aside from a generic `TestCase`, typically called `TestCase`, a test suite may contain more specialized `TestCase` classes with longer names.

Those classes should also get the "one extra word" treatment (with `TestCase` counted as one word).

Fixed now.

Includes tests.
Includes update to the XML docs.
The WPCS `IsUnitTestTrait::is_test_class()` method does not (and never has so far) handle import `use` statements, which means that in the context of namespaced files, the results of the function are unreliable.

As documented with the `TEST_SUFFIXES` constant, the `Test` and `TestCase` suffixes should only be given the "one extra word" treatment when these classes extend a "known" test class.

As things were, this wasn't respected properly by the sniff anyhow (the `$this->is_test_class()` should have been _nested_ within the `self::TEST_SUFFIXES[ $last ] === true` condition).

And as it cannot reliably be enforced at this time, we're better off removing the useless condition and just checking if a class with one of the "extra word" suffixes extends.
The names of `enum` structures should comply with the same rules as classes.

This commit:
* Adds the `T_ENUM` token to the tokens being sniffed for.
* Makes an exception in the "one extra word for test/mocks/double" logic as `enum`s cannot extend, though the may still need to be mocked.
* Ensures the error message for enums is grammatically correct.

Includes tests.
Includes update to the XML docs.
To check whether a class is deprecated, the sniff needs to skip over the potential class modifier keywords, like `final` and `abstract`.

As of PHP 8.2, it will also need to skip over the new `readonly` keyword.

Includes unit tests.
@coveralls
Copy link

Coverage Status

coverage: 98.404% (+0.2%) from 98.18%
when pulling dc6695c on JRF/yoastcs-objectnamedepth-various-improvements
into 80457d5 on develop.

@jrfnl jrfnl merged commit bc72022 into develop Nov 4, 2023
25 checks passed
@jrfnl jrfnl deleted the JRF/yoastcs-objectnamedepth-various-improvements branch November 4, 2023 10:56
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.

2 participants