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

Fix escaping problem in write_literal and print_literal lint suggestion #13990

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lapla-cogito
Copy link
Contributor

@lapla-cogito lapla-cogito commented Jan 12, 2025

fix #13959

The current implementation of the write_literal and print_literal lint performs escaping for the second argument of write! ,writeln!, print! and println! of the suggestion by first replacing " with \", and then replacing \ with \\. Performing these replacements in this order may lead to unnecessary backslashes being added if the original code contains " (e.g. " -> \\"), potentially resulting in a suggestion that causes the code to fail to compile.
In the issue mentioned above, it’s suggested to use raw strings as raw strings, but implementing this would require an ad-hoc change to the current implementation, so it has been deferred. (I'll implement this in another PR)

changelog: [write_literal]: fix incorrect escaping of suggestions
changelog: [print_literal]: fix incorrect escaping of suggestions

@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2025

r? @Centri3

rustbot has assigned @Centri3.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 12, 2025
@lapla-cogito lapla-cogito force-pushed the issue_13959 branch 2 times, most recently from e67bad1 to 05125db Compare January 12, 2025 21:06
@samueltardieu
Copy link
Contributor

Maybe you could also include a \ in the tests as well.

@lapla-cogito lapla-cogito force-pushed the issue_13959 branch 2 times, most recently from 2e92d3a to 3d7b1ee Compare January 12, 2025 21:55
@lapla-cogito
Copy link
Contributor Author

Indeed, I've updated the test code.

@lapla-cogito lapla-cogito changed the title Fix escaping problem in write_literal lint suggestion Fix escaping problem in write_literal and print_literal lint suggestion Jan 13, 2025
@ChrisJefferson
Copy link

Just to say, this fixed my issue, and I tried poking it a bit more and couldn't break it.

I'd still like my raw strings leaving as raw strings in the long term (as I wrote them they way in purpose), but this definately fixes the issue of producing invalid code!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

write_literal breaks raw strings
5 participants