Add suggestion to quote inlined format argument as string literal #114507
Conversation
|
r? @jackh726 (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| (unexpanded_fmt_span.shrink_to_hi(), "\"".to_string()), | ||
| (unexpanded_fmt_span.shrink_to_lo(), "\"".to_string()), | ||
| ], | ||
| Applicability::MaybeIncorrect, |
There was a problem hiding this comment.
i can't think of any situations where this suggestion would ever produce code that wouldn't compile. i'm not exactly sure of the exact use cases for Applicability::MachineApplicable and how rigorously provable the suggestions must be, but due to the relatively simplicity of the suggestion and the general unambiguity surrounding the purpose of println! and associated macros, I think its fair to annotate this suggestion as MachineApplicable. let me know if i'm overlooking something obvious though
There was a problem hiding this comment.
For the original you might be missing a string literal to format with suggestion, I can't think of cases where the suggestion would produce code that wouldn't compile too. Yet, it uses Applicability::MaybeIncorrect. This is why I used Applicability::MaybeIncorrect in this new suggestion. Do you think I should change both to Applicability::MachineApplicable?
There was a problem hiding this comment.
I think keeping MaybeIncorrect is probably best for now.
| | | ||
| help: you might be missing a string literal to format with | ||
| | | ||
| LL | eprintln!("{}", {1}); |
There was a problem hiding this comment.
nit: removing the braces when there is only one distinct 'statement' would be nice, if you're up for it
There was a problem hiding this comment.
thanks for the suggestion! will look into it outside of this PR 😄
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (e286f25): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 631.684s -> 632.934s (0.20%) |
Fixes #114235