rustfix: Support inserting new lines. #13226
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is just some minor code cleanup for the parse_and_replace test, there should not be any functional differences.
This adds an environment variable to make it easier to add new tests.
If rustfix received a suggestion which inserts new lines without replacing existing lines, it would ignore the suggestion. This is because `parse_snippet` would immediately return if the `lines` to replace was empty. The solution here is to just drop the code which messes with the original text line. `cargo fix` (and compiletest) currently do not use this. This was originally added back in the days when rustfix supported an interactive UI which showed color highlighting of what it looks like with the replacement. My feeling is that when we add something like this back in, I would prefer to instead use a real diff library and display instead of trying to do various text manipulation for display. This particular code has generally been buggy, and has been a problem several times. The included test fails without this fix because the changes do not apply, and the code cannot compile.
|
r? @weihanglo (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
Command-fix
S-waiting-on-review
Suggestions that come from rustc that are multi-line only use LF line endings. But if the file is checked out on windows with CRLF line-endings, then you end up with a mix of line endings that don't match the "fixed.rs" file. Tracking this at rust-lang/rust#119482.
weihanglo
approved these changes
Jan 2, 2024
| @@ -20,6 +42,7 @@ mod settings { | |||
| pub const CHECK_JSON: &str = "RUSTFIX_TEST_CHECK_JSON"; | |||
| pub const RECORD_JSON: &str = "RUSTFIX_TEST_RECORD_JSON"; | |||
| pub const RECORD_FIXED_RUST: &str = "RUSTFIX_TEST_RECORD_FIXED_RUST"; | |||
| pub const BLESS: &str = "RUSTFIX_TEST_BLESS"; | |||
There was a problem hiding this comment.
This makes me wonder if we should use snapbox to minimize the custom logic here.
|
Thank you. Going to merge this soon. @bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 3, 2024
Update cargo 11 commits in ac6bbb33293d8d424c17ecdb42af3aac25fb7295..add15366eaf3f3eb84717d3b8b71902ca36a7c84 2023-12-26 23:22:08 +0000 to 2024-01-02 03:24:42 +0000 - chore(deps): update gix (rust-lang/cargo#13230) - chore(deps): update alpine docker tag to v3.19 (rust-lang/cargo#13228) - rustfix: Support inserting new lines. (rust-lang/cargo#13226) - Fix fix::fix_in_dependency to not rely on rustc (rust-lang/cargo#13220) - cleanup: Remove error-format special-case in `cargo fix` (rust-lang/cargo#13224) - `cargo fix`: always inherit the jobserver (rust-lang/cargo#13225) - Bump cargo-credential to 0.4.3 (rust-lang/cargo#13221) - `cargo add` - fix for adding features from repository with multiple packages. (rust-lang/cargo#13213) - Remove repetitive words (rust-lang/cargo#13216) - Add cargo:rustc-cdylib-link-arg into RESERVED_PREFIXES list (rust-lang/cargo#13212) - chore(doc): doc for custom subcommands look up. (rust-lang/cargo#13203)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
If rustfix received a suggestion which inserts new lines without replacing existing lines, it would ignore the suggestion. This is because
parse_snippetwould immediately return if thelinesto replace was empty.The solution here is to just drop the code which messes with the original text line.
cargo fix(and compiletest) currently do not use this. This was originally added back in the days when rustfix supported an interactive UI which showed color highlighting of what it looks like with the replacement. My feeling is that when we add something like this back in, I would prefer to instead use a real diff library and display instead of trying to do various text manipulation for display. This particular code has generally been buggy, and has been a problem several times.The included test fails without this fix because the changes do not apply, and the code cannot compile.
This also includes a little bit of cleanup for the
parse_and_replacetest. My feeling is that this test could use further improvements.