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

--stdin-path with not existed path will ignore code checking #1488

Closed
withwind8 opened this issue May 27, 2017 · 4 comments
Closed

--stdin-path with not existed path will ignore code checking #1488

withwind8 opened this issue May 27, 2017 · 4 comments

Comments

@withwind8
Copy link

I'm using PHPCS to process data from STDIN.

When using --stdin-path to specify a existed path, everything is OK.
cat tmp.php | phpcs --stdin-path=existed/path/to/php

However, when a non-existent path is specified,phpcs will ignore the content from STDIN.
cat tmp.php | phpcs --stdin-path=not/existed/path/to/php

My requirement is to check the code that does not have real files.

In my opinion, it is not necessary to check the existence of stdin-path when processing data from STDIN. If you want, just specify the file instead of using STDIN.

I will submit a PR for it.

@gsherwood
Copy link
Member

Having a fake path ignored by the file filtering process is pretty easy, but it will then not apply any of the filters to it. And by not calling realpath on it, the path will still (potentially) be a relative path, which may or may not matter to the sniffs running over it.

I mentioned in the linked PR that I do not want to maintain code that attempts to turn a relative path into an absolute path without using the built-in realpath function, so it probably doesn't leave too many options.

The most obvious one is to say that if you specify a STDIN path, either on the CLI or inside the file itself using the phpcs_input_file: syntax, then:

  1. Specify an absolute path if you need one because a relative path will not be converted to absolute
  2. None of the filters (extensions, gitmodified etc) will apply, so the STDIN content will be processed regardless of your settings

I actually think this is fine because why would you specify STDIN content, and use a fake path, only to apply filters that exclude the content? It doesn't make a lot of sense to me.

Do you think those conditions are fair enough for your use case?

@MichelHartmann
Copy link

I have the same problem and in my use case your conditions are totally acceptable.

@gsherwood
Copy link
Member

I ended up removing the realpath check from the filter because:

  1. I don't think it is needed because all paths passed in would have already had realpath applied elsewhere.
  2. This was causing ignore rules to be checked for symlink source paths instead of the target path, which is something that looks to have changed in 3.0
  3. Local files were having their contents loaded up without realpath anyway, so the filter check felt inconsistent

The only good reason I could see for the realpath was that it ensured that the is_dir call was being made on a fully resolved file path, but that looks to be working fine without it anyway and was not the reason realpath was originally introduced.

@gsherwood
Copy link
Member

The end result of this change is that the filters you have set will be applied to whatever file path you have told PHPCS to use for STDIN. So STDIN will not be processed regardless - it will still have ignore patterns applied - but it can now use a fake file path.

morozov added a commit to diff-sniffer/core that referenced this issue Aug 17, 2018
As of squizlabs/PHP_CodeSniffer#1488, Filter::accept() doesn't strictly check the real path of the files, so intstead of using reflection and calling its protected methods, we can use the filter itself.

Closes #4.
morozov added a commit to diff-sniffer/core that referenced this issue Aug 28, 2018
As of squizlabs/PHP_CodeSniffer#1488, Filter::accept() doesn't strictly check the real path of the files, so intstead of using reflection and calling its protected methods, we can use the filter itself.

Closes #4.
morozov added a commit to diff-sniffer/core that referenced this issue Aug 29, 2018
As of squizlabs/PHP_CodeSniffer#1488, Filter::accept() doesn't strictly check the real path of the files, so intstead of using reflection and calling its protected methods, we can use the filter itself.

Closes #4.
morozov added a commit to diff-sniffer/core that referenced this issue Mar 1, 2019
As of squizlabs/PHP_CodeSniffer#1488, Filter::accept() doesn't strictly check the real path of the files, so intstead of using reflection and calling its protected methods, we can use the filter itself.

Closes #4.
morozov added a commit to diff-sniffer/core that referenced this issue Mar 1, 2019
As of squizlabs/PHP_CodeSniffer#1488, Filter::accept() doesn't strictly check the real path of the files, so intstead of using reflection and calling its protected methods, we can use the filter itself.

Closes #4.
morozov added a commit to diff-sniffer/core that referenced this issue Mar 2, 2019
As of squizlabs/PHP_CodeSniffer#1488, Filter::accept() doesn't strictly check the real path of the files, so intstead of using reflection and calling its protected methods, we can use the filter itself.

Closes #4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants