Coverage decreased (-0.01%) to 92.898% when pulling 9f5c7da on keradus:mono into 22804e8 on FriendsOfPHP:master.

I like the idea, but +0 for implementation. Maybe we can change the default AbstractFixer's isCandidate implementation to skip non-monolithic and has alternative syntax files, and than we go through all fixers to see if these actually handle these cases correctly.

I would suggest we focus first on non-monolithic, and if we are aligned, we try to align on alternative syntax (the later is doing way less issues, but I would be OK to apply that logic too!)

not sure about AbstractFixer -> custom fixers don't use it and therefor I put check directly in runner. We already had similar concern with supports/isCandidate, when we incorporated it into our abstractFixer, and then still had to check it for 3rd party fixers ref

With that, would you suggest to run check in 2 places?

Very good idea, if one has non-monolithic files the priority should not be coding standards, but getting rid of them, it's not 1995 anymore.

I wonder it the change would count as BC break though.

I see the use case of people reporting that non-monolithic are touched in a way they do not expect.
For that the proposal is great 👍 I see why this is good to address.

However, I also see non-monolithic file being fixed in a way people like. For example the header rule, or no bom rule or many other rules. This proposal stops all this fixing.

That is why I suggest not to block this on a tool level. We can fix non-monolithic files, however it is hard. Hard in the same way of fixing alternative syntax; it is not impossible, it is just really hard.
Impossible I consider things like needing scope beyond a single file, or needing some AST.

With that, would you suggest to run check in 2 places?

I think we are looking at different things here. I'm not in favor of do things in 2 places. The linked reference is about the order of how methods, defined by an interface, are called by the tool. What I'm trying to suggest here is not about the order of or the (lack of) heuristic behavior of methods, but about allowing (3rd party) rules to fix non-monolithic if they can.

Changing all the current core rules to no longer fix non-monolithic rules (or not touch alternative syntax) I really like, we can than refine by checking each rule to see if it actually can handle these cases correctly and than still allow the rule to do it.
However the proposal now is that it will be impossible to fix any non-monolithic by any rule, not because of some technical limitation but because of some tool scope or design decision, and I'm not convinced of that just yet.

the proposal now is that it will be impossible to fix any non-monolithic by any rule, not because of some technical limitation but because of some tool scope or design decision, and I'm not convinced of that just yet.

that's exactly my proposal. and if it wouldn't get support, I'm OK to downgrade it to "only our built-in rules".

We aim to encourage good code, and supporting files of mixed content (PHP, HTML, ...) is not doing so (even if we can improve the code a bit there)

as discussed during check-in, please go-ahead @keradus :)

I'm not sure I see value in ignoring non-monolithic files altogether. First, I don't think using PHP as a template language is inherently bad. Secondly, as @SpacePossum said, some rules can apply to those files without issue.

Recently, the main issue reports relate to indentation. Maybe we can make some rules skip those file (because e.g. indentation can be tricky when mixed with HTML)? Maybe those issue aren't that hard to fix?

I like the idea in general, but I think there should be opt-in path for people who want fix non-monolithic files anyway (but it should be stated explicitly we don't provide active support for these files).

Let's reopen and give us some additional time to process it.

@keradus it needs a solid rebase, there's Fabbot in the workflow 🙈.

Since this pull request has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 14 days.

Please keep your branch up-to-date by rebasing it when main branch is ahead of it, thanks in advance!

coverage: 94.733% (-0.03%) from 94.765%
when pulling 62f5feb on keradus:mono
into 125449b on PHP-CS-Fixer:master.

Sorry for the late feedback.

I'm not sure I see value in ignoring non-monolithic files altogether. First, I don't think using PHP as a template language is inherently bad.

I agree here, especially since you can use the same static analysis tools for these template files (php-cs-fixer, phpstan, rector etc.). You can be more strict in php templates than in, e.g., twig templates.
(Of course there are also many advantages for real template languages and other approaches. But still I agree that php templates are not inherently bad.)

some rules can apply to those files without issue.

Yes, there is no reason, why e.g. the binary_operator fixer (any many others) should not be used for non-monolithic files.

Recently, the main issue reports relate to indentation. Maybe we can make some rules skip those file (because e.g. indentation can be tricky when mixed with HTML)?

I agree, fixing indentation in non-monolithic files is too hard. That's why I think this fixer should be disabled for them: #7864

So my approach would be:
Don't disable non-monolithic files by default.
When issues for non-monolithic files are reported, wait a short time, if someone is opening a pr with an (easy) fix.
When this is not happening, disable that fixer for non-monolithic files (without configuration options etc.).
(but it can be re-enabled when someone is providing an easy fix.)

Don't disable non-monolithic files by default.

i disagree. I do not want Fixer to have explicit support for mixed files. Important to say, we never explicitly claimed such support, yet ppl assumed it as tool was running on such file implicitly.

For long time, nobody bothered to fix the issues for non-monolithic files and i have no interest in that, nor promoting usage of html mixed into php files. i was simply closing such issues and also nobody bothered to raise a pr to fix those.

i see limited reasoning to run the Fixer for such files, so i'm ok to let ppl opt-in on their own, as discussed earlier.
if someone will raise a fix to improve handling of non-monolitic files, i'm ok with that. yet not putting my own effort into it nor claim official support.

So let's agree to disagree on the general topic "php templates".

Given your stance on it, I understand that you don't want to work on that topic and you want to make this more clear. And of course I don't want to expect any extra work from you anyway. I have great respect for your ongoing work here!

And of course it is no big deal to set the env variable to re-enable the non-monolithic files.

thanks.
if we will somehow see more demand for it, i will be open to move env var into config property