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

Give original filename to phpcs #921

Closed
choma opened this issue Mar 24, 2016 · 7 comments

Comments

@choma
Copy link

commented Mar 24, 2016

As explained here, using file path instead of STDIN allows phpcs to infer which rules should be excluded.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2016

Please summarise the discussion in the issue you've referenced as far as it's relevant to Flycheck and elaborate why we need to drop standard input for phpcs. Ideally, try to produce a minimal example where phpcs behaves different as expected with Flycheck.

@choma

This comment has been minimized.

Copy link
Author

commented Mar 24, 2016

There are phpcs rules that depends on the file location.
As an example take this excerpt from CakePHP code sniffer (taked from here):

<rule ref="Squiz.NamingConventions.ValidFunctionName">
  <exclude-pattern>*/src/*</exclude-pattern>
  <exclude-pattern>*/tests/*</exclude-pattern>
</rule>

it tells to phpcs to exclude that rule when the path of the file being checked match the patterns.
So, if you are checking files under src/ (which are almost all in a cakephp project and most other frameworks) and you call phpcs with file path it won't apply that rule, and you'll see less errors.
If you call phpcs with STDIN it can't know where the file is, so it applies all the rules, resulting in an incorrect list of errors.
I hope it to be understandable, my english is really bad.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2016

@choma Other syntax checkers (e.g. rubocop, eslint, etc.) solve this by accepting the name of the file as argument. Is that no option for phpcs?

@choma

This comment has been minimized.

Copy link
Author

commented Mar 25, 2016

I copy my comment on flycheck issue, so you can find it easily:

As I understand from this issue, and what I can understand from this code, you have to add phpcs_input_file: [file path] as the first line of content.
I've just tested it and it works great!

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2016

@choma phpcs 2.6 will provide an option --stdin-path which allows us to set the path, see squizlabs/PHP_CodeSniffer#934 (comment)

When phpcs 2.6 is released will use this option to fix this issue.

@lunaryorn lunaryorn changed the title Stop using STDIN for phpcs Give original filename to phpcs Apr 4, 2016

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2016

phpcs 2.6 is out now, I'll update the syntax checker definition soon. Note that this will implicitly drop support for any older phpcs release.

@lunaryorn lunaryorn self-assigned this Apr 4, 2016

@lunaryorn lunaryorn changed the title Give original filename to phpcs Give original filename to phpcs Apr 4, 2016

@lunaryorn lunaryorn closed this in ca8401f Apr 30, 2016

@lunaryorn lunaryorn removed the S-ready label Apr 30, 2016

@lunaryorn

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2016

@choma I've updated phpcs accordingly, the issue should be fixed now.

Thanks for reporting it, and for your help with finding a proper solution! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.