ObliviousHarmony / vscode-php-codesniffer

A VS Code extension for integrating PHP_CodeSniffer.
https://marketplace.visualstudio.com/items?itemName=obliviousharmony.vscode-php-codesniffer
Other
40 stars 2 forks source link

Now the plugin will try to find the .phpcs file in the parent folders #57

Closed leonardola closed 1 year ago

leonardola commented 1 year ago

All Submissions:

Changes proposed in this Pull Request:

This PR makes the plugin look into the parent folders for the .phpcs files so it can use the project settings instead of only the standard settings.

How to test the changes in this Pull Request:

  1. Create a blank project
  2. Inside that project add a subfolder with a .phpcs.xml file
  3. Add a php file to the subfolder
  4. trigger a lint error that is present in the .phpcs.xml file you created and not in the standard
  5. move the .phpcs.xml file to the root folder
  6. trigger the lint error again
  7. remove the .phpcs.xml file and try to trigger the error again
ObliviousHarmony commented 1 year ago

Hey @leonardola,

I appreciate you taking the time to submit this pull request! It's great to see someone other than me opening one to be honest :smile:.

Unfortunately, I'm not entirely convinced that this functionality belongs in this extension. The main goal of this was to provide an interface for deeply integrating PHP_CodeSniffer into VS Code. I made it a point to avoid adding any features to the extension that change the behavior of phpcs:

When you run phpcs without a --standard option it will use the default standard that is set using phpcs --config-set default_standard <standard>. If none is set, it will infer one based on your current working directory. When you use the Default standard in this extension, it relies on phpcs to select the standard that should be applied.

Coming back to the feature proposed in this PR; my concern is whether or not it changes the typical expected behavior of phpcs. Although ESLint cascades configuration files the way that you're proposing we do here, PHPCS does not. In order for you to do what you're asking via the CLI you would need to either call the file with a different --standard or call it with no standard from within the same directory as the file. I don't think that this extension should do the former, but, maybe there's an argument for doing the latter.

Could you share a little bit more about the use-case that led you to open this PR?

Thanks again for starting this discussion!

leonardola commented 1 year ago

I usually work with multiple repos in the same workspace. Each repo has it's own set of rules. If I use the standards of one of the projects, loads of lint errors will show on the other project.

If none is set, it will infer one based on your current working directory

Well that looks a lot like my code. Implementing the no standard option would make more sense in this case.

I'll close this PR and work on the no standard idea

ObliviousHarmony commented 1 year ago

That's actually how the "Default" setting already works. It uses the workspace root as the working directory. When the autoExecutable setting is enabled, it will instead use the directory that the nearest composer.json file is located in. You can find that code here.

It might make sense to add an "Automatic" standard option that uses the document's fsPath's directory instead of the working directory in the Worker. This option would get you what you the behavior that you're looking for. I don't think I would be opposed to this option, but, we should make sure the option has a good description and is documented in the README.md so that there's no ambiguity.

If you'd prefer, I could go ahead and make this change myself? I wanted to make a few other changes for discoverability in the extension marketplace, so, it seems like a perfectly reasonable time to do it now.

leonardola commented 1 year ago

Sure go ahead.