-
Notifications
You must be signed in to change notification settings - Fork 3
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
Files/FileName: bug fix, support modern PHP and other improvements #342
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Always return a stack pointer at the end of the file, even when the file passed comes from `STDIN`, as we ignore `STDIN` files for this sniff, so there is no need to continue listening for tokens in the file. Includes ignoring this line for code coverage checks as the line is untestable in a unit test situation.
As the sniff class is now `final` (since PR 319), there is no need for any `protected` methods, so let's make these `private`.
Lower nesting levels and remove `elseif` when not needed. :point_right: This commit will be easiest to review while ignoring whitespace changes.
`$name` is very generic, so let's rename this variable to `$oo_name` to make it more descriptive.
Prevents a PHP notice in case of a IDE check during live coding. Includes test. :point_right: this change will be easiest to review while ignoring whitespace changes.
Always have quotes around the expected + found file names. As things were, this was not the case for the generic "lowercase/hyphenated" message + the class file name message. Fixed now.
Files using _only_ short open tags were never examined. Fixed now. Includes additional tests.
These type of selective ignore annotations were introduced in PHPCS 3.2.0. This commit now allows for disabling this sniff through an annotation, though error code based ignores are still not supported. Includes unit tests.
…ash word separators This mirrors the same change as made upstream in the WPCS native FileName sniff in PR WordPress/WordPress-Coding-Standards 2285. Files using non-underscore punctuation characters as word separators were previously not being flagged by the sniff. Fixed now. Includes extra tests.
The switch cases for `interface`s and `trait`s are basically duplicates of each other with the term being variable. This refactors the code to remove the duplicate code and use the term as a variable. Includes updated unit tests to ensure the message creation is stable.
Enums should be treated the same as interfaces/traits, i.e.: * File name should reflect the name of the `enum`. * Prefixes should be stripped from the name. * The file name should end with `-enum`. The sniff has been updated to handle enums in this way. Includes additional unit tests. Includes updated documentation.
…orrectly The `is_file_excluded()` method would first call the `clean_custom_array_property()` method with the `$flip` parameter set to `true`, resulting in an array containing the excluded files as the keys. After that it would call the `normalize_directory_separators()` method on all _values_ via an `array_map()`. In effect, this means that the actual values, which are stored in the array _keys_ would still not be normalized, meaning the "is file excluded" check could return the wrong result for ruleset provided paths containing a backslash directory separator. Even though, in practice, this didn't lead to problems as the paths in rulesets are set using forward slashes, it is still a bug. Fixed now by no longer "flipping" the array. And as the `$flip` parameter of the `clean_custom_array_property()` method is now no longer used, the parameter and its handling has been removed from the function. Includes test.
There is no need to lowercase the "excluded files" + the file name for the `is_file_excluded()` check. Checking these in their original, proper case should be just fine. And as the `$to_lower` parameter of the `clean_custom_array_property()` method is now no longer used, the parameter and its handling has been removed from the function. Includes additional test safeguarding that the comparison is done case-sensitively.
Efficiency tweak. Cleaning the ruleset passed OO prefixes doesn't need to be done time and again every single time this sniff is called. For a normal PHPCS run, where the property is set in a ruleset, doing it once and reusing the cleaned up version in subsequent calls to the sniff will be sufficient. For test runs, this may need to be done more often, but that can be handled by storing the previous value of `$oo_prefixes` property and checking if it has changed before doing the validation. This commit implements this.
Efficiency tweak. Cleaning the ruleset passed "excluded files" doesn't need to be done time and again every single time this sniff is called. For a normal PHPCS run, where the property is set in a ruleset, doing it once and reusing the cleaned up version in subsequent calls to the sniff will be sufficient. For test runs, this may need to be done more often, but that can be handled by storing the previous value of `$excluded_files_strict_check` property and checking if it has changed before doing the validation. This commit implements this. Additionally it adds some extra safeguards for incorrectly passed "excluded files" paths. Includes removing the `$phpcsFile` parameter from the `is_file_excluded()` method as it is no longer needed by that method. Includes additional unit tests. Includes updating a pre-existing test to pass duplicate excluded files in different ways.
No need to throw this for every single file. Throwing it once should be sufficient. Includes moving the check for the missing basepath to earlier in the process flow.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Files/FileName: minor documentation improvements
Files/FileName: efficiency fix
Always return a stack pointer at the end of the file, even when the file passed comes from
STDIN
, as we ignoreSTDIN
files for this sniff, so there is no need to continue listening for tokens in the file.Includes ignoring this line for code coverage checks as the line is untestable in a unit test situation.
Files/FileName: use PHPCSUtils in more places
Files/FileName: make non-interface methods private
As the sniff class is now
final
(since PR #319), there is no need for anyprotected
methods, so let's make theseprivate
.Files/FileName: minor code tweaks
Lower nesting levels and remove
elseif
when not needed.👉 This commit will be easiest to review while ignoring whitespace changes.
Files/FileName: rename a local variable
$name
is very generic, so let's rename this variable to$oo_name
to make it more descriptive.Files/FileName: more defensive coding for parse errors/live coding
Prevents a PHP notice in case of a IDE check during live coding.
Includes test.
👉 this change will be easiest to review while ignoring whitespace changes.
Files/FileName: improve error message consistency
Always have quotes around the expected + found file names.
As things were, this was not the case for the generic "lowercase/hyphenated" message + the class file name message.
Fixed now.
Files/FileName: examine views using only short echo open tags
Files using only short open tags were never examined. Fixed now.
Includes additional tests.
Files/FileName: allow for the inline PHPCS ignore annotations
These type of selective ignore annotations were introduced in PHPCS 3.2.0.
This commit now allows for disabling this sniff through an annotation, though error code based ignores are still not supported.
Includes unit tests.
Files/FileName: improve handling of files using non-underscore, non-dash word separators
This mirrors the same change as made upstream in the WPCS native FileName sniff in PR WordPress/WordPress-Coding-Standards#2285.
Files using non-underscore punctuation characters as word separators were previously not being flagged by the sniff.
Fixed now. Includes extra tests.
Files/FileName: refactor some near duplicate code
The switch cases for
interface
s andtrait
s are basically duplicates of each other with the term being variable.This refactors the code to remove the duplicate code and use the term as a variable.
Includes updated unit tests to ensure the message creation is stable.
Files/FileName: add handling of files containing PHP 8.1+ enums
Enums should be treated the same as interfaces/traits, i.e.:
enum
.-enum
.The sniff has been updated to handle enums in this way.
Includes additional unit tests.
Includes updated documentation.
Files/FileName: bug fix - excluded paths were not always normalized correctly
The
is_file_excluded()
method would first call theclean_custom_array_property()
method with the$flip
parameter set totrue
, resulting in an array containing the excluded files as the keys.After that it would call the
normalize_directory_separators()
method on all values via anarray_map()
.In effect, this means that the actual values, which are stored in the array keys would still not be normalized, meaning the "is file excluded" check could return the wrong result for ruleset provided paths containing a backslash directory separator.
Even though, in practice, this didn't lead to problems as the paths in the Yoast rulesets are set using forward slashes, it is still a bug.
Fixed now by no longer "flipping" the array.
And as the
$flip
parameter of theclean_custom_array_property()
method is now no longer used, the parameter and its handling has been removed from the function.Includes test.
Files/FileName: simplification in excluded file comparison
There is no need to lowercase the "excluded files" + the file name for the
is_file_excluded()
check.Checking these in their original, proper case should be just fine.
And as the
$to_lower
parameter of theclean_custom_array_property()
method is now no longer used, the parameter and its handling has been removed from the function.Includes additional test safeguarding that the comparison is done case-sensitively.
Files/FileName: only clean up the OO prefixes when needed
Efficiency tweak. Cleaning the ruleset passed OO prefixes doesn't need to be done time and again every single time this sniff is called.
For a normal PHPCS run, where the property is set in a ruleset, doing it once and reusing the cleaned up version in subsequent calls to the sniff will be sufficient.
For test runs, this may need to be done more often, but that can be handled by storing the previous value of
$oo_prefixes
property and checking if it has changed before doing the validation.This commit implements this.
Files/FileName: only clean up the excluded files when needed
Efficiency tweak. Cleaning the ruleset passed "excluded files" doesn't need to be done time and again every single time this sniff is called.
For a normal PHPCS run, where the property is set in a ruleset, doing it once and reusing the cleaned up version in subsequent calls to the sniff will be sufficient.
For test runs, this may need to be done more often, but that can be handled by storing the previous value of
$excluded_files_strict_check
property and checking if it has changed before doing the validation.This commit implements this.
Additionally it adds some extra safeguards for incorrectly passed "excluded files" paths.
Includes removing the
$phpcsFile
parameter from theis_file_excluded()
method as it is no longer needed by that method.Includes additional unit tests.
Includes updating a pre-existing test to pass duplicate excluded files in different ways.
Files/FileName: only throw the "missing basepath" warning once
No need to throw this for every single file. Throwing it once should be sufficient.
Includes moving the check for the missing basepath to earlier in the process flow.
Files/FileName: implement use of new PathHelper and PathValidationHelper classes