Skip to content

Commit f36a1fd

Browse files
committed
Minor cleanup of map_entry
1 parent c6feb9b commit f36a1fd

File tree

1 file changed

+77
-92
lines changed

1 file changed

+77
-92
lines changed

clippy_lints/src/entry.rs

+77-92
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use rustc_hir::{
1313
use rustc_lint::{LateContext, LateLintPass};
1414
use rustc_session::{declare_lint_pass, declare_tool_lint};
1515
use rustc_span::{Span, SyntaxContext, DUMMY_SP};
16+
use std::fmt::Write;
1617

1718
declare_clippy_lint! {
1819
/// **What it does:** Checks for uses of `contains_key` + `insert` on `HashMap`
@@ -67,19 +68,23 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
6768
None => return,
6869
};
6970

71+
let then_search = match find_insert_calls(cx, &contains_expr, then_expr) {
72+
Some(x) => x,
73+
None => return,
74+
};
75+
7076
let mut app = Applicability::MachineApplicable;
7177
let map_str = snippet_with_context(cx, contains_expr.map.span, contains_expr.call_ctxt, "..", &mut app).0;
7278
let key_str = snippet_with_context(cx, contains_expr.key.span, contains_expr.call_ctxt, "..", &mut app).0;
7379
let sugg = if let Some(else_expr) = else_expr {
74-
let (then_search, else_search) = match (
75-
find_insert_calls(cx, &contains_expr, then_expr),
76-
find_insert_calls(cx, &contains_expr, else_expr),
77-
) {
78-
(Some(then), Some(els)) if !(then.edits.is_empty() && els.edits.is_empty()) => (then, els),
80+
// if .. { .. } else { .. }
81+
let else_search = match find_insert_calls(cx, &contains_expr, else_expr) {
82+
Some(search) if !(then_search.edits.is_empty() && search.edits.is_empty()) => search,
7983
_ => return,
8084
};
8185

8286
if then_search.edits.is_empty() || else_search.edits.is_empty() {
87+
// if .. { insert } else { .. } or if .. { .. } else { then } of
8388
let (then_str, else_str, entry_kind) = if else_search.edits.is_empty() {
8489
if contains_expr.negated {
8590
(
@@ -107,7 +112,6 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
107112
"Vacant(e)",
108113
)
109114
};
110-
111115
format!(
112116
"if let {}::{} = {}.entry({}) {} else {}",
113117
map_ty.entry_path(),
@@ -118,87 +122,63 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
118122
else_str,
119123
)
120124
} else {
121-
let indent_str = snippet_indent(cx, expr.span);
122-
let indent_str = indent_str.as_deref().unwrap_or("");
123-
if contains_expr.negated {
124-
format!(
125-
"match {}.entry({}) {{\n\
126-
{indent} {entry}::Vacant(e) => {}\n\
127-
{indent} {entry}::Occupied(mut e) => {}\n\
128-
{indent}}}",
129-
map_str,
130-
key_str,
131-
reindent_multiline(
132-
then_search.snippet_vacant(cx, then_expr.span, &mut app).into(),
133-
true,
134-
Some(4 + indent_str.len())
135-
),
136-
reindent_multiline(
137-
else_search.snippet_occupied(cx, else_expr.span, &mut app).into(),
138-
true,
139-
Some(4 + indent_str.len())
140-
),
141-
entry = map_ty.entry_path(),
142-
indent = indent_str,
125+
// if .. { insert } else { insert }
126+
let (then_str, else_str, then_entry, else_entry) = if contains_expr.negated {
127+
(
128+
then_search.snippet_vacant(cx, then_expr.span, &mut app),
129+
else_search.snippet_occupied(cx, else_expr.span, &mut app),
130+
"Vacant(e)",
131+
"Occupied(mut e)",
143132
)
144133
} else {
145-
format!(
146-
"match {}.entry({}) {{\n\
147-
{indent} {entry}::Occupied(mut e) => {}\n\
148-
{indent} {entry}::Vacant(e) => {}\n\
149-
{indent}}}",
150-
map_str,
151-
key_str,
152-
reindent_multiline(
153-
then_search.snippet_occupied(cx, then_expr.span, &mut app).into(),
154-
true,
155-
Some(4 + indent_str.len())
156-
),
157-
reindent_multiline(
158-
else_search.snippet_vacant(cx, else_expr.span, &mut app).into(),
159-
true,
160-
Some(4 + indent_str.len())
161-
),
162-
entry = map_ty.entry_path(),
163-
indent = indent_str,
134+
(
135+
then_search.snippet_occupied(cx, then_expr.span, &mut app),
136+
else_search.snippet_vacant(cx, else_expr.span, &mut app),
137+
"Occupied(mut e)",
138+
"Vacant(e)",
164139
)
165-
}
140+
};
141+
let indent_str = snippet_indent(cx, expr.span);
142+
let indent_str = indent_str.as_deref().unwrap_or("");
143+
format!(
144+
"match {}.entry({}) {{\n{indent} {entry}::{} => {}\n\
145+
{indent} {entry}::{} => {}\n{indent}}}",
146+
map_str,
147+
key_str,
148+
then_entry,
149+
reindent_multiline(then_str.into(), true, Some(4 + indent_str.len())),
150+
else_entry,
151+
reindent_multiline(else_str.into(), true, Some(4 + indent_str.len())),
152+
entry = map_ty.entry_path(),
153+
indent = indent_str,
154+
)
166155
}
156+
} else if then_search.edits.is_empty() {
157+
// no insertions
158+
return;
167159
} else {
168-
let search = match find_insert_calls(cx, &contains_expr, then_expr) {
169-
Some(x) if !x.edits.is_empty() => x,
170-
_ => return,
171-
};
172-
173-
if !search.allow_insert_closure {
174-
if contains_expr.negated {
175-
format!(
176-
"if let {}::Vacant(e) = {}.entry({}) {}",
177-
map_ty.entry_path(),
178-
map_str,
179-
key_str,
180-
search.snippet_vacant(cx, then_expr.span, &mut app),
181-
)
160+
// if .. { insert }
161+
if !then_search.allow_insert_closure {
162+
let (body_str, entry_kind) = if contains_expr.negated {
163+
(then_search.snippet_vacant(cx, then_expr.span, &mut app), "Vacant(e)")
182164
} else {
183-
format!(
184-
"if let {}::Occupied(mut e) = {}.entry({}) {}",
185-
map_ty.entry_path(),
186-
map_str,
187-
key_str,
188-
search.snippet_occupied(cx, then_expr.span, &mut app),
165+
(
166+
then_search.snippet_occupied(cx, then_expr.span, &mut app),
167+
"Occupied(mut e)",
189168
)
190-
}
191-
} else if search.is_single_insert {
192-
let value_str = snippet_with_context(
193-
cx,
194-
search.first_insertion().value.span,
195-
then_expr.span.ctxt(),
196-
"..",
197-
&mut app,
169+
};
170+
format!(
171+
"if let {}::{} = {}.entry({}) {}",
172+
map_ty.entry_path(),
173+
entry_kind,
174+
map_str,
175+
key_str,
176+
body_str,
198177
)
199-
.0;
178+
} else if let Some(insertion) = then_search.as_single_insertion() {
179+
let value_str = snippet_with_context(cx, insertion.value.span, then_expr.span.ctxt(), "..", &mut app).0;
200180
if contains_expr.negated {
201-
if search.first_insertion().value.can_have_side_effects() {
181+
if insertion.value.can_have_side_effects() {
202182
format!("{}.entry({}).or_insert_with(|| {});", map_str, key_str, value_str)
203183
} else {
204184
format!("{}.entry({}).or_insert({});", map_str, key_str, value_str)
@@ -208,7 +188,7 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
208188
return;
209189
}
210190
} else {
211-
let block_str = search.snippet_closure(cx, then_expr.span, &mut app);
191+
let block_str = then_search.snippet_closure(cx, then_expr.span, &mut app);
212192
if contains_expr.negated {
213193
format!("{}.entry({}).or_insert_with(|| {});", map_str, key_str, block_str)
214194
} else {
@@ -535,8 +515,8 @@ struct InsertSearchResults<'tcx> {
535515
is_single_insert: bool,
536516
}
537517
impl InsertSearchResults<'tcx> {
538-
fn first_insertion(&self) -> Insertion<'tcx> {
539-
self.edits[0].as_insertion().unwrap()
518+
fn as_single_insertion(&self) -> Option<Insertion<'tcx>> {
519+
self.is_single_insert.then(|| self.edits[0].as_insertion().unwrap())
540520
}
541521

542522
fn snippet_occupied(&self, cx: &LateContext<'_>, mut span: Span, app: &mut Applicability) -> String {
@@ -576,20 +556,25 @@ impl InsertSearchResults<'tcx> {
576556
));
577557
if is_expr_used_or_unified(cx.tcx, insertion.call) {
578558
if is_expr_final_block_expr(cx.tcx, insertion.call) {
579-
res.push_str("e.insert(");
580-
res.push_str(&snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0);
581-
res.push_str(");\n");
582-
res.push_str(snippet_indent(cx, insertion.call.span).as_deref().unwrap_or(""));
583-
res.push_str("None");
559+
let _ = write!(
560+
res,
561+
"e.insert({});\n{}None",
562+
snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0,
563+
snippet_indent(cx, insertion.call.span).as_deref().unwrap_or(""),
564+
);
584565
} else {
585-
res.push_str("{ e.insert(");
586-
res.push_str(&snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0);
587-
res.push_str("); None }");
566+
let _ = write!(
567+
res,
568+
"{{ e.insert({}); None }}",
569+
snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0,
570+
);
588571
}
589572
} else {
590-
res.push_str("e.insert(");
591-
res.push_str(&snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0);
592-
res.push(')');
573+
let _ = write!(
574+
res,
575+
"e.insert({})",
576+
snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0,
577+
);
593578
}
594579
span = span.trim_start(insertion.call.span).unwrap_or(DUMMY_SP);
595580
}

0 commit comments

Comments
 (0)