fix: api:upgrade-resource output formatting #5624
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello, fixed 2 issues affecting the output of
bin/console api:upgrade-resource
. I know the command is removed in v3, but I think it's an important part of the upgrade path, and it is really hard to use right now because almost the whole file appears in diff, see below.PhpParser\PrettyPrinter\Standard::printFormatPreserving
method has 3 requirements to preserve formatting, and one of them was not met.The AST needs to be traversed by
CloningVisitor
before making changes to the AST. This is adding a property on the AST nodes, keeping a reference to the original node, and especially original tokens positions.This is documented here and here
This is obviously still working on a best effort basis, but fixing this makes a huge difference in the output, but one issue remains with docblocks tags, which is point 2
RemoveAnnotationTrait::removeAnnotationByTag
method is creating a diff for every line containing a tag in docblocks.This occurs because we actually rebuild the whole docblock, and tags are outputed with an extra space, no matter the initial formatting
At the end, because we just need to delete a specific annotation in a string here, and annotation format is pretty predictible, I ended up using a good old regex.
I tested this patched version in my own application where the command needs to alter ~200 files, and I am super happy with the result (see example below), you still need to run cs-fixer to eventually cleanup a few remaining blank lines, but those blank lines are legitimate, i.e next to a deleted one, it's not random blank lines & spaces everywhere
Example diff after fix for a User entity
Example diff before fix (same User entity)