Skip to content

Commit f050c6c

Browse files
TD5meta-codesync[bot]
authored andcommitted
Support calls to lists:nth/2
Summary: Fixes a query as to why we report on `hd(lists:reverse(L))`, but not `lists:nth(1, lists:reverse(L))`. For the purposes of this diagnostic, `lists:nth(1, lists:reverse(L))` is equivalent to `hd(lists:reverse(L))`, so we trigger on that too. Reviewed By: alanz Differential Revision: D86969766 fbshipit-source-id: e6a022a10267d04c6700b3ee5fc9fa31a02b75ac
1 parent 395db3d commit f050c6c

File tree

1 file changed

+65
-0
lines changed

1 file changed

+65
-0
lines changed

crates/ide/src/diagnostics/inefficient_last.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ pub(crate) static DESCRIPTOR: DiagnosticDescriptor = DiagnosticDescriptor {
3838
},
3939
checker: &|acc, sema, file_id, _ext| {
4040
inefficient_last_hd_ssr(acc, sema, file_id);
41+
inefficient_last_nth_ssr(acc, sema, file_id);
4142
inefficient_last_pat_ssr(acc, sema, file_id);
4243
},
4344
};
@@ -62,6 +63,24 @@ fn inefficient_last_hd_ssr(diags: &mut Vec<Diagnostic>, sema: &Semantic, file_id
6263
});
6364
}
6465

66+
// lists:nth(1, L) is functionally equivalent to hd(L)
67+
fn inefficient_last_nth_ssr(diags: &mut Vec<Diagnostic>, sema: &Semantic, file_id: FileId) {
68+
let matches = match_pattern_in_file_functions(
69+
sema,
70+
Strategy {
71+
macros: MacroStrategy::Expand,
72+
parens: ParenStrategy::InvisibleParens,
73+
},
74+
file_id,
75+
format!("ssr: lists:nth(1, lists:reverse({LIST_VAR})).").as_str(),
76+
);
77+
matches.matches.iter().for_each(|m| {
78+
if let Some(diagnostic) = make_diagnostic_hd(sema, file_id, m) {
79+
diags.push(diagnostic)
80+
}
81+
});
82+
}
83+
6584
fn inefficient_last_pat_ssr(diags: &mut Vec<Diagnostic>, sema: &Semantic, file_id: FileId) {
6685
let matches = match_pattern_in_file_functions(
6786
sema,
@@ -248,4 +267,50 @@ mod tests {
248267
"#]],
249268
)
250269
}
270+
271+
#[test]
272+
fn ignores_inefficient_last_via_nth_when_index_is_not_one() {
273+
check_diagnostics(
274+
r#"
275+
//- /src/inefficient_last.erl
276+
-module(inefficient_last).
277+
278+
% elp:ignore W0017 (undefined_function)
279+
fn(List) -> lists:nth(3, lists:reverse(List)).
280+
"#,
281+
)
282+
}
283+
284+
#[test]
285+
fn detects_inefficient_last_via_nth() {
286+
check_diagnostics(
287+
r#"
288+
//- /src/inefficient_last.erl
289+
-module(inefficient_last).
290+
291+
% elp:ignore W0017 (undefined_function)
292+
fn(List) -> lists:nth(1, lists:reverse(List)).
293+
%% ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 💡 warning: W0029: Unnecessary intermediate reverse list allocated.
294+
"#,
295+
)
296+
}
297+
298+
#[test]
299+
fn fixes_inefficient_last_via_nth() {
300+
check_fix(
301+
r#"
302+
//- /src/inefficient_last.erl
303+
-module(inefficient_last).
304+
305+
% elp:ignore W0017 (undefined_function)
306+
fn(List) -> lists:nth(1, lists:re~verse(List)).
307+
"#,
308+
expect![[r#"
309+
-module(inefficient_last).
310+
311+
% elp:ignore W0017 (undefined_function)
312+
fn(List) -> lists:last(List).
313+
"#]],
314+
)
315+
}
251316
}

0 commit comments

Comments
 (0)