fix: map_entry: don't emit lint before checks have been performed (#14568)
Fixes rust-lang/rust-clippy#14449, introduced in #14314 changelog: [`map_entry`]: fix a false positive where the lint would trigger without any insert calls present
This commit is contained in:
parent
7d83c8c543
commit
79df6ac1a7
3 changed files with 62 additions and 15 deletions
|
|
@ -95,14 +95,13 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
|
|||
return;
|
||||
};
|
||||
|
||||
if then_search.is_key_used_and_no_copy || else_search.is_key_used_and_no_copy {
|
||||
span_lint(cx, MAP_ENTRY, expr.span, lint_msg);
|
||||
return;
|
||||
}
|
||||
|
||||
if then_search.edits.is_empty() && else_search.edits.is_empty() {
|
||||
// No insertions
|
||||
return;
|
||||
} else if then_search.is_key_used_and_no_copy || else_search.is_key_used_and_no_copy {
|
||||
// If there are other uses of the key, and the key is not copy,
|
||||
// we cannot perform a fix automatically, but continue to emit a lint.
|
||||
None
|
||||
} else if then_search.edits.is_empty() || else_search.edits.is_empty() {
|
||||
// if .. { insert } else { .. } or if .. { .. } else { insert }
|
||||
let ((then_str, entry_kind), else_str) = match (else_search.edits.is_empty(), contains_expr.negated) {
|
||||
|
|
@ -123,10 +122,10 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
|
|||
snippet_with_applicability(cx, then_expr.span, "{ .. }", &mut app),
|
||||
),
|
||||
};
|
||||
format!(
|
||||
Some(format!(
|
||||
"if let {}::{entry_kind} = {map_str}.entry({key_str}) {then_str} else {else_str}",
|
||||
map_ty.entry_path(),
|
||||
)
|
||||
))
|
||||
} else {
|
||||
// if .. { insert } else { insert }
|
||||
let ((then_str, then_entry), (else_str, else_entry)) = if contains_expr.negated {
|
||||
|
|
@ -142,13 +141,13 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
|
|||
};
|
||||
let indent_str = snippet_indent(cx, expr.span);
|
||||
let indent_str = indent_str.as_deref().unwrap_or("");
|
||||
format!(
|
||||
Some(format!(
|
||||
"match {map_str}.entry({key_str}) {{\n{indent_str} {entry}::{then_entry} => {}\n\
|
||||
{indent_str} {entry}::{else_entry} => {}\n{indent_str}}}",
|
||||
reindent_multiline(&then_str, true, Some(4 + indent_str.len())),
|
||||
reindent_multiline(&else_str, true, Some(4 + indent_str.len())),
|
||||
entry = map_ty.entry_path(),
|
||||
)
|
||||
))
|
||||
}
|
||||
} else {
|
||||
if then_search.edits.is_empty() {
|
||||
|
|
@ -163,17 +162,17 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
|
|||
} else {
|
||||
then_search.snippet_occupied(cx, then_expr.span, &mut app)
|
||||
};
|
||||
format!(
|
||||
Some(format!(
|
||||
"if let {}::{entry_kind} = {map_str}.entry({key_str}) {body_str}",
|
||||
map_ty.entry_path(),
|
||||
)
|
||||
))
|
||||
} else if let Some(insertion) = then_search.as_single_insertion() {
|
||||
let value_str = snippet_with_context(cx, insertion.value.span, then_expr.span.ctxt(), "..", &mut app).0;
|
||||
if contains_expr.negated {
|
||||
if insertion.value.can_have_side_effects() {
|
||||
format!("{map_str}.entry({key_str}).or_insert_with(|| {value_str});")
|
||||
Some(format!("{map_str}.entry({key_str}).or_insert_with(|| {value_str});"))
|
||||
} else {
|
||||
format!("{map_str}.entry({key_str}).or_insert({value_str});")
|
||||
Some(format!("{map_str}.entry({key_str}).or_insert({value_str});"))
|
||||
}
|
||||
} else {
|
||||
// TODO: suggest using `if let Some(v) = map.get_mut(k) { .. }` here.
|
||||
|
|
@ -183,7 +182,7 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
|
|||
} else {
|
||||
let block_str = then_search.snippet_closure(cx, then_expr.span, &mut app);
|
||||
if contains_expr.negated {
|
||||
format!("{map_str}.entry({key_str}).or_insert_with(|| {block_str});")
|
||||
Some(format!("{map_str}.entry({key_str}).or_insert_with(|| {block_str});"))
|
||||
} else {
|
||||
// TODO: suggest using `if let Some(v) = map.get_mut(k) { .. }` here.
|
||||
// This would need to be a different lint.
|
||||
|
|
@ -192,7 +191,11 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
|
|||
}
|
||||
};
|
||||
|
||||
span_lint_and_sugg(cx, MAP_ENTRY, expr.span, lint_msg, "try", sugg, app);
|
||||
if let Some(sugg) = sugg {
|
||||
span_lint_and_sugg(cx, MAP_ENTRY, expr.span, lint_msg, "try", sugg, app);
|
||||
} else {
|
||||
span_lint(cx, MAP_ENTRY, expr.span, lint_msg);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -226,4 +226,26 @@ fn issue11976() {
|
|||
}
|
||||
}
|
||||
|
||||
mod issue14449 {
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
pub struct Meow {
|
||||
map: BTreeMap<String, String>,
|
||||
}
|
||||
|
||||
impl Meow {
|
||||
fn pet(&self, _key: &str, _v: u32) -> u32 {
|
||||
42
|
||||
}
|
||||
}
|
||||
|
||||
pub fn f(meow: &Meow, x: String) {
|
||||
if meow.map.contains_key(&x) {
|
||||
let _ = meow.pet(&x, 1);
|
||||
} else {
|
||||
let _ = meow.pet(&x, 0);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
|
|||
|
|
@ -232,4 +232,26 @@ fn issue11976() {
|
|||
}
|
||||
}
|
||||
|
||||
mod issue14449 {
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
pub struct Meow {
|
||||
map: BTreeMap<String, String>,
|
||||
}
|
||||
|
||||
impl Meow {
|
||||
fn pet(&self, _key: &str, _v: u32) -> u32 {
|
||||
42
|
||||
}
|
||||
}
|
||||
|
||||
pub fn f(meow: &Meow, x: String) {
|
||||
if meow.map.contains_key(&x) {
|
||||
let _ = meow.pet(&x, 1);
|
||||
} else {
|
||||
let _ = meow.pet(&x, 0);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue