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

Fix PHP notices when file extensions defaults are not present #1032

Merged
merged 1 commit into from
Jun 24, 2016
Merged

Fix PHP notices when file extensions defaults are not present #1032

merged 1 commit into from
Jun 24, 2016

Conversation

klausi
Copy link
Contributor

@klausi klausi commented Jun 11, 2016

Sometimes the internal values of CLI.php are not initialized. This happens on test runs in Drupal's coder. I'm trying to add

<arg name="extensions" value="php,module,inc,install,test,profile,theme,css,info,txt,md,yml"/>

to ruleset.xml to have some extension default values.

The commit is my best guess at fixing the notice, or is there another place where $this->values should be initialized?

@gsherwood
Copy link
Member

I can't figure out why you are having problems just yet, but I can replicate your issues with coder and it does appear to just be a unit testing thing given how a normal run works:

PHPCS sets default values for all settings here: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/CLI.php#L266

When CLI values are set, the first thing that happens is that these defaults are loading into the values member var here: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/CLI.php#L423

I'm not sure how command line values would be processed without the default values being set first, given that the processLongArgument method is only ever called from setCommandLineValues and that's the method that loads in the defaults. So I'll need to look more into what is happening during the test run.

@gsherwood
Copy link
Member

Gosh I'm an idiot sometimes. During unit testing, the defaults are intentionally not loaded. The code is right there in my second link: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/CLI.php#L420

@gsherwood gsherwood merged commit d418df7 into squizlabs:master Jun 24, 2016
gsherwood added a commit that referenced this pull request Jun 24, 2016
@gsherwood
Copy link
Member

I think your fix is in the right spot. Thanks for the PR.

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.

2 participants