Bug fix: loadInstalledPaths() very very broken since PHPCS 3.1.0
#98
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
jrfnl
force-pushed
the
feature/bug-fix-loadInstalledPaths
branch
from
5e8a561
to
a5f1082
Compare
Potherca
requested changes
Jan 19, 2020
The PHPCS `--config-show` command shows **all** config settings set, not just the `installed_paths` and since PHPCS 3.1.0, it also includes a line to indicate which configuration file is being used to help with debugging configuration issues. In PHPCS 2.0 - 3.0.1, if any [other configuration options](https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options) were set, the result of the `loadInstalledPaths()` command would be broken. In PHPCS 3.1.0+, the result would always be broken, no matter what. In effect this means that the `installed_paths` would (nearly) always be set as if the config was previously non-existent as the `cleanInstalledPaths()` method would empty out the array as the values contained therein would be invalid anyway. It would be lovely if it were possible to add unit tests, but as we're talking about a `private` method setting a `private` property... So, for now, to proof the bug and validate the fix, I've set up a "fake" unit test to demonstrate what the previous behaviour was and what the behaviour will be with this fix in place: https://3v4l.org/XKMIf
jrfnl
force-pushed
the
feature/bug-fix-loadInstalledPaths
branch
from
a5f1082
to
b09c763
Compare
Potherca
approved these changes
Jan 19, 2020
3 tasks
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.
Proposed Changes
Discovered while debugging the build failure of #97
The PHPCS
--config-showcommand shows all config settings set, not just theinstalled_pathsand since PHPCS 3.1.0, it also includes a line to indicate which configuration file is being used to help with debugging configuration issues.Example:
In PHPCS 2.0 - 3.0.1, if any other configuration options were set, aside from
installed_paths, the result of theloadInstalledPaths()command would be broken.In PHPCS 3.1.0+, the result would always be broken, no matter what.
In effect this means that the
installed_pathswould (nearly) always be set as if the config was previously non-existent as thecleanInstalledPaths()method would empty out the array as the values contained therein would be invalid anyway.It would be lovely if it were possible to add unit tests, but as we're talking about a
privatemethod setting aprivateproperty...So, for now, to proof the bug and validate the fix, I've set up a "fake" unit test to demonstrate what the previous behaviour was and what the behaviour will be with this fix in place: https://3v4l.org/brrNT
I suspect that this may fix some mystery issues previously reported.