Skip to content

Commit

Permalink
Preserve suggestion formatting or safely unescape in write_literal
Browse files Browse the repository at this point in the history
  • Loading branch information
@Alexendoo
Alexendoo committed Mar 30, 2022
1 parent a56a0d9 commit 2f51fd3
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 45 deletions.
131 changes: 98 additions & 33 deletions clippy_lints/src/write.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ impl<'tcx> LateLintPass<'tcx> for Write {
}

let Some(args) = format_args.args(cx) else { return };
check_literal(cx, &args, name, format_args.is_raw(cx));
check_literal(cx, &format_args, &args, name);

if !self.in_debug_impl {
for arg in args {
Expand Down Expand Up @@ -426,7 +426,7 @@ fn check_empty_string(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, ma
}
}

fn check_literal(cx: &LateContext<'_>, args: &[FormatArgsArg<'_>], name: &str, raw: bool) {
fn check_literal(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, args: &[FormatArgsArg<'_>], name: &str) {
let mut counts = HirIdMap::<usize>::default();
for arg in args {
*counts.entry(arg.value.hir_id).or_default() += 1;
Expand All @@ -436,14 +436,24 @@ fn check_literal(cx: &LateContext<'_>, args: &[FormatArgsArg<'_>], name: &str, r
if_chain! {
if counts[&arg.value.hir_id] == 1;
if arg.format_trait == sym::Display;
if let ExprKind::Lit(lit) = &arg.value.kind;
if !arg.has_primitive_formatting();
if !arg.value.span.from_expansion();
if let ExprKind::Lit(lit) = &arg.value.kind;
if let Some(value_string) = snippet_opt(cx, arg.value.span);
if let Some(format_string) = snippet_opt(cx, format_args.format_string_span);
then {
let replacement = match lit.node {
LitKind::Str(s, _) => s.to_string(),
LitKind::Char(c) => c.to_string(),
LitKind::Bool(b) => b.to_string(),
let (replacement, replace_raw) = match lit.node {
LitKind::Str(..) => extract_str_literal(&value_string),
LitKind::Char(ch) => (
match ch {
'"' => "\\\"",
'\'' => "'",
_ => &value_string[1..value_string.len() - 1],
}
.to_string(),
false,
),
LitKind::Bool(b) => (b.to_string(), false),
_ => continue,
};

Expand All @@ -453,40 +463,95 @@ fn check_literal(cx: &LateContext<'_>, args: &[FormatArgsArg<'_>], name: &str, r
PRINT_LITERAL
};

let replacement = match (format_string.starts_with('r'), replace_raw) {
(false, false) => Some(replacement),
(false, true) => Some(replacement.replace('"', "\\\"").replace('\\', "\\\\")),
(true, false) => match conservative_unescape(&replacement) {
Ok(unescaped) => Some(unescaped),
Err(UnescapeErr::Lint) => None,
Err(UnescapeErr::Ignore) => continue,
},
(true, true) => {
if replacement.contains(['#', '"']) {
None
} else {
Some(replacement)
}
},
};

span_lint_and_then(cx, lint, arg.value.span, "literal with an empty format string", |diag| {
if raw && replacement.contains(&['"', '#']) {
return;
}
if let Some(replacement) = replacement {
// `format!("{}", "a")`, `format!("{named}", named = "b")
// ~~~~~ ~~~~~~~~~~~~~
let value_span = expand_past_previous_comma(cx, arg.value.span);

let backslash = if raw {
r"\"
} else {
r"\\"
};
let replacement = replacement
.replace('{', "{{")
.replace('}', "}}")
.replace('"', "\\\"")
.replace('\\', backslash);

// `format!("{}", "a")`, `format!("{named}", named = "b")
// ~~~~~ ~~~~~~~~~~~~~
let value_span = expand_past_previous_comma(cx, arg.value.span);

diag.multipart_suggestion(
"try this",
vec![
(arg.span, replacement),
(value_span, String::new()),
],
Applicability::MachineApplicable,
);
let replacement = replacement.replace('{', "{{").replace('}', "}}");
diag.multipart_suggestion(
"try this",
vec![
(arg.span, replacement),
(value_span, String::new()),
],
Applicability::MachineApplicable,
);
}
});
}
}
}
}

/// Removes the raw marker, `#`s and quotes from a str, and returns if the literal is raw
///
/// `r#"a"#` -> (`a`, true)
///
/// `"b"` -> (`b`, false)
fn extract_str_literal(literal: &str) -> (String, bool) {
let (literal, raw) = match literal.strip_prefix('r') {
Some(stripped) => (stripped.trim_matches('#'), true),
None => (literal, false),
};

(literal[1..literal.len() - 1].to_string(), raw)
}

enum UnescapeErr {
/// Should still be linted, can be manually resolved by author, e.g.
///
/// ```ignore
/// print!(r"{}", '"');
/// ```
Lint,
/// Should not be linted, e.g.
///
/// ```ignore
/// print!(r"{}", '\r');
/// ```
Ignore,
}

/// Unescape a normal string into a raw string
fn conservative_unescape(literal: &str) -> Result<String, UnescapeErr> {
let mut unescaped = String::with_capacity(literal.len());
let mut chars = literal.chars();
let mut err = false;

while let Some(ch) = chars.next() {
match ch {
'#' => err = true,
'\\' => match chars.next() {
Some('\\') => unescaped.push('\\'),
Some('"') => err = true,
_ => return Err(UnescapeErr::Ignore),
},
_ => unescaped.push(ch),
}
}

if err { Err(UnescapeErr::Lint) } else { Ok(unescaped) }
}

// Expand from `writeln!(o, "")` to `writeln!(o, "")`
// ^^ ^^^^
fn expand_past_previous_comma(cx: &LateContext<'_>, span: Span) -> Span {
Expand Down
8 changes: 0 additions & 8 deletions clippy_utils/src/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,14 +511,6 @@ impl<'tcx> FormatArgsExpn<'tcx> {
.collect()
}

pub fn is_raw(&self, cx: &LateContext<'tcx>) -> bool {
if let Some(format_string) = snippet_opt(cx, self.format_string_span) {
format_string.starts_with('r')
} else {
false
}
}

/// Source callsite span of all inputs
pub fn inputs_span(&self) -> Span {
match *self.value_args {
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/write_literal_2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@ fn main() {
writeln!(v, r"{}", "\\");
writeln!(v, r#"{}"#, "\\");
writeln!(v, "{}", r"\");
writeln!(v, "{}", "\r");
writeln!(v, r#"{}{}"#, '#', '"'); // hard mode
writeln!(v, r"{}", "\r"); // should not lint
}
32 changes: 28 additions & 4 deletions tests/ui/write_literal_2.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ LL | | world!"
|
help: try this
|
LL - "some {}",
LL + "some hello world!"
|
LL ~ "some hello /
LL ~ world!"
|

error: literal with an empty format string
--> $DIR/write_literal_2.rs:25:9
Expand Down Expand Up @@ -162,5 +162,29 @@ LL - writeln!(v, "{}", r"/");
LL + writeln!(v, "/");
|

error: aborting due to 14 previous errors
error: literal with an empty format string
--> $DIR/write_literal_2.rs:31:23
|
LL | writeln!(v, "{}", "/r");
| ^^^^
|
help: try this
|
LL - writeln!(v, "{}", "/r");
LL + writeln!(v, "/r");
|

error: literal with an empty format string
--> $DIR/write_literal_2.rs:32:28
|
LL | writeln!(v, r#"{}{}"#, '#', '"'); // hard mode
| ^^^

error: literal with an empty format string
--> $DIR/write_literal_2.rs:32:33
|
LL | writeln!(v, r#"{}{}"#, '#', '"'); // hard mode
| ^^^

error: aborting due to 17 previous errors

0 comments on commit 2f51fd3

Please sign in to comment.