Config: can use custom LocalFile object

[forrest79]

Description

I'm the author of package PHPCS-Ignores that allows to ignore PHPCS errors/warning in the similar way as PHPStan - you have a file with listed ignores.

To make this happen, I need some extra logic in LocalFile class. I'm using my own LocalFile that extends the original one.

LocalFile is hardcoded in FileList. To force PHP_CodeSniffer to use my own class, I do some magic via stream wrapper that update FileList.php on the fly. I was thinking if it would be possible to allow this without a hacking through Config class.

I prepared a small example that works for me. I'm able to update config entry in bootstrap file, that you must use if you want to use my package.

This is just proof of work, I will add tests later if this change will be approved.

Suggested changelog entry

Internals: can use own LocalFile class.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.
• [Required for new files] I have added any new files to the package.xml file.

[jrfnl]

Thanks for this PR and your willingness to contribute to PHPCS.

I'm not keen on this PR as I really don't want to have to deal with support requests/bug reports from end-users using a standard using this feature. That's going to be very messy and will waste everyone's time.

Also, as per the CONTRIBUTING guide, please open an issue to discuss proposals instead of opening POC PRs. Those are not welcome.

Marking this as close candidate.

Also see #137

[jrfnl]

I'm not keen on this PR as I really don't want to have to deal with support requests/bug reports from end-users using a standard using this feature. That's going to be very messy and will waste everyone's time.

To clarify the above (as I realize I didn't give an example) - with a config setting like proposed in this PR:

• The use of a custom localfile is non-transparent for the end-user, meaning support requests will be posted here, while they don't belong in this repo.
• Multiple standards/rulesets could each try to set their own localfile, but only one "custom" localfile will be registered, so now any standard whose localfile did not get registered will potentially cause errors.
• Those errors will lead to unexpected behaviour for the end-user in a non-transparent manner.
• And even when the use of a custom localfile does not lead to errors, it may still cause unexpected behaviour for the end-user as the end-user may not be aware of the use of the custom localfile.

[forrest79]

Also, as per the CONTRIBUTING guide, please open an issue to discuss proposals instead of opening POC PRs. Those are not welcome.

Sorry, I thought that see the code will be easier to describe the proposal.

I totally understand - I can live without this PR, just some thoughts.

• The use of a custom localfile is non-transparent for the end-user, meaning support requests will be posted here, while they don't belong in this repo.

I have some external sniffs that also give me just some internal error (in some corner cases) during the file analyses without any context what sniff did this and I have to debug this and disable the concrete sniff for the file. Badly written bootstrap could also fail and you don't know what is wrong.

• Multiple standards/rulesets could each try to set their own localfile, but only one "custom" localfile will be registered, so now any standard whose localfile did not get registered will potentially cause errors.

Ok, I thought about this as an internal thing to use only when you really know what you're doing, that's why it's not possible to change this via command line.

• Those errors will lead to unexpected behaviour for the end-user in a non-transparent manner.
• And even when the use of a custom localfile does not lead to errors, it may still cause unexpected behaviour for the end-user as the end-user may not be aware of the use of the custom localfile.

Again, I understand, but I think this could be said also for badly written sniffs, bootstraps, reports etc...

[jrfnl]

Also, as per the CONTRIBUTING guide, please open an issue to discuss proposals instead of opening POC PRs. Those are not welcome.

Sorry, I thought that see the code will be easier to describe the proposal.

Actually no. The PR is a potential solution. But it doesn't explain the actual problem.

Yes, I know your usecase, but what I still don't know is why that usecase would require this solution. There may be numerous other solutions for your problem, but I can't even begin to imagine those as I still don't know the problem and submitting a PR basically says "this is the only solution I'm interested in" (independently of whether it is the right solution).

I think this could be said also for badly written sniffs, bootstraps, reports etc...

All the more reason not to make the potential surface for errors larger by adding a niche feature which is error prone.

[forrest79]

Yes, I know your usecase, but what I still don't know is why that usecase would require this solution. There may be numerous other solutions for your problem, but I can't even begin to imagine those as I still don't know the problem and submitting a PR basically says "this is the only solution I'm interested in" (independently of whether it is the right solution).

Fair enough :-) You're right. I wrote my package when there was no active development in the original PHPCS package and I prepare my package with an information from this PR squizlabs/PHP_CodeSniffer#3387. Using own LocalFile wasn't originally my idea. I saw it in the PR where there was a package that make similar update right in the vendor directory which I don't want to.

I will try to summarise all of this to an issue, maybe there is some better way to achieve this.

[jrfnl]

Seeing the referenced issue in the old repo, I again refer you to issue #137 in this repo...

[forrest79]

Sorry, I miss that in your first comment :-( Now I feel silly I didn't find this before open an PR. Thanks :-)