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

Feature Request: ignore/exclude parts of a file in the coding standard xml file #3874

Open
finn-matti opened this issue Aug 11, 2023 Discussed in #3873 · 4 comments
Open

Comments

@finn-matti
Copy link

finn-matti commented Aug 11, 2023

Discussed in #3873

Originally posted by finn-matti August 11, 2023
Context: We do not have a linter in use in our project, but none the less an informal standard is enforced. To make my life easier in MRs and make sure the code fits that informal standard, I use phpcs to point out and fix things. As we all know: You can't always fix all problems and have to use phpcs:ignore or similar. The problem is, of course, that comments like this shouldn't be part of the code base, since no one else is using phpcs (yet, I hope. But that's a different discussion). So I would like a way to ignore certain parts of files from the coding standard file (the xml file). I tried to search in issues and discussions and checked the wiki for this feature (to no avail)- but I thought I'd ask here, just to be sure.

It seems to me that this could be an interesting feature since I'm surely not the only one suffering from this. Another use case: Avoiding long phpcs:ignore comments like this one:

@phpcsSuppress SlevomatCodingStandard.TypeHints.PropertyTypeHint.MissingNativeTypeHint

You can imagine if you need to suppress more rules lines can become very long. Why not allow partial exclusions of files in the xml file, too?

@jrfnl
Copy link
Contributor

jrfnl commented Aug 11, 2023

First off, the @phpcsSuppress annotation syntax is specific to the Slevomat Coding Standard and is not a PHPCS feature.

The PHPCS native supported annotations allow for selective ignoring, on a modular level - i.e. // phpcs:ignore = ignore everything, // phpcs:ignore Stnd,Cat,Sniff,ErrorCode = ignore a specific error code; and every variation in between is possible.

As for the feature request: how do you envision this ? I mean, would this be based on line nrs ? If so, that is highly unstable as just updating a docblock could already cause an "off-by-one" (or more) issue.
If not by line nrs, then how ?

@fredden
Copy link
Contributor

fredden commented Aug 15, 2023

This sounds similar to the 'baseline' feature in PHPStan. Perhaps a similar format could be used here: store the path, count, and code as a tuple.

I've not yet looked into this in any real detail; my initial thoughts are that this could perhaps be a implemented as a custom report.

@jrfnl
Copy link
Contributor

jrfnl commented Aug 15, 2023

There are a number of feature requests which are loosely related to the baseline feature in PHPStan. I've started tagging those with the Focus: Ignore Annotations/Excludes label to make them easier to find, though there are probably more which I haven't tagged yet.

@finn-matti
Copy link
Author

First off, the @phpcsSuppress annotation syntax is specific to the Slevomat Coding Standard and is not a PHPCS feature.

Ooff. Sorry about that. I meant to write // phpcs:ignore etc.

I think the idea of @fredden could work - as we know that it works in phpstan. To be honest I was thinking about it more naively: And was just thinking about line numbers and ranges at first. These would be easy enough to adjust, remove or add - at least in my use case, since I would mainly need this in the end of my work, when committing to a codebase that doesn't have an official standard (only an implicit one) and no agreed upon tools to enforce it.

Furthermore I use the CodeSniffer exlusively through vs code. Which means that I deal with one file at a time, so its not too overwhelming even in the case where line numbers would be the only way to mark ignored parts of files.

With that said: A baseline would of course be smarter for use within pipelines and such.

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