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

Rector makes whitespace changes even when no rules are run #7258

Closed
edsrzf opened this issue Jun 26, 2022 · 7 comments
Closed

Rector makes whitespace changes even when no rules are run #7258

edsrzf opened this issue Jun 26, 2022 · 7 comments
Labels

Comments

@edsrzf
Copy link

edsrzf commented Jun 26, 2022

Bug Report

Subject Details
Rector version 0.13.6

Rector sometimes modifies whitespace in files that are not affected by any rules.

Minimal PHP Code Causing Issue

https://getrector.org/demo/f49f5775-2295-4708-8e8c-c4ac1c4b4123

There are no rules configured at all, but still a whitespace modification is made.

Expected Behaviour

A configuration with no rules added should be a no-op. Rector should not touch code that is not affected by any rules. This behavior adds extra noise to diffs that should be preventable.

Ideally it should not change whitespace or formatting at all unless asked, but I recognize that this is a very difficult problem in general. This particular case seems solvable though.

@edsrzf edsrzf added the bug label Jun 26, 2022
@samsonasik
Copy link
Member

That is expected right now, see e2e test for it:

https://github.com/rectorphp/rector-src/blob/0a644b148b39ee4adfde45519622b7cd9e4ef0d6/e2e/plain-views/src/view.php

https://github.com/rectorphp/rector-src/blob/0a644b148b39ee4adfde45519622b7cd9e4ef0d6/e2e/plain-views/expected-output.diff

That's possibly require many tweak in case want to be applied.

On other note, actually, having a space before any code can cause some issue, eg: buffer output, issue on session header, etc.

You can exclude the file from touched by rector via $rectorConfig->skip() in case the space before code is actually needed.

$rectorConfig->skip([
   __DIR__ . '/views',
]);

@edsrzf
Copy link
Author

edsrzf commented Jun 27, 2022

That makes some sense. I probably chose a poor way to minimize this case.

In the actual code I'm dealing with, there is space in the middle of the file which Rector modifies. It's more like this: https://getrector.org/demo/39070d3f-84dd-41a2-a260-21e2d67f13bb

The comment seems to be relevant. The code is pretty weird, for sure; I won't defend it.

@samsonasik
Copy link
Member

That's due to mixing with inline html, which php-parser limitation https://github.com/rectorphp/rector#how-to-apply-coding-standards .

You possibly need to:

  • skip it
  • or run cs tools againt it after run rector to it.

@edsrzf
Copy link
Author

edsrzf commented Jun 27, 2022

Again, I think the main thing is I'm surprised Rector would make any changes to a file when there's no rule that applies to it. I completely understand not preserving style when a file needs to be modified, as that's a very hard problem to solve.

If it's by design that Rector always writes to files, even when rules don't apply or don't need to change anything, that's okay, but I don't understand why it would be designed that way. Is there a reason? It seems to me that Rector could be much more efficient if it didn't always write out ASTs to files when there are no changes.

@TomasVotruba
Copy link
Member

TomasVotruba commented Jun 27, 2022

@edsrzf Could you create a minimal reproducible repository? There might be some rule that tries to make change and then file is reprinted.

@TomasVotruba
Copy link
Member

@samsonasik I see.

@edsrzf In this case, we already cover with test and list it as known issue. The problem is the php-parser, that cannot handle all the edge cases of spacing properly, see: nikic/PHP-Parser#344
HTML and PHP spacing in AST tree is hard :)

In your case it would be easier to skip it as @samsonasik suggest or use cs tool to keep your expcting formatting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants