feat(natu): detect returned let bindings
This commit is contained in:
@@ -69,7 +69,7 @@ fn natu_reports_out_of_order_let_bindings() -> googletest::Result<()> {
|
|||||||
fn natu_preserves_suppressed_bindings() -> googletest::Result<()> {
|
fn natu_preserves_suppressed_bindings() -> googletest::Result<()> {
|
||||||
let report = natu::future_sight(vec![SourceFile {
|
let report = natu::future_sight(vec![SourceFile {
|
||||||
path: PathBuf::from("testdata/nolint.nix"),
|
path: PathBuf::from("testdata/nolint.nix"),
|
||||||
source: "{ config, ... }: let\n cfg = config.example; # NOLINT(let_order)\n value = cfg;\nin value\n".into(),
|
source: "{ config, ... }: let # NOLINT(redundant_let)\n cfg = config.example; # NOLINT(let_order)\n value = cfg;\nin value\n".into(),
|
||||||
}])
|
}])
|
||||||
.or_fail()?;
|
.or_fail()?;
|
||||||
|
|
||||||
@@ -107,7 +107,8 @@ fn natu_reports_redundant_let() -> googletest::Result<()> {
|
|||||||
path: PathBuf::from("testdata/redundant_let.nix"),
|
path: PathBuf::from("testdata/redundant_let.nix"),
|
||||||
source: r#"
|
source: r#"
|
||||||
let
|
let
|
||||||
value = 1;
|
helper = 1;
|
||||||
|
value = helper;
|
||||||
in
|
in
|
||||||
value
|
value
|
||||||
"#
|
"#
|
||||||
@@ -126,16 +127,29 @@ value
|
|||||||
|
|
||||||
#[gtest]
|
#[gtest]
|
||||||
fn natu_accepts_nonredundant_let() -> googletest::Result<()> {
|
fn natu_accepts_nonredundant_let() -> googletest::Result<()> {
|
||||||
let report = natu::future_sight(vec![SourceFile {
|
let report = natu::future_sight(vec![
|
||||||
path: PathBuf::from("testdata/nonredundant_let.nix"),
|
SourceFile {
|
||||||
source: r#"
|
path: PathBuf::from("testdata/recursive_let.nix"),
|
||||||
|
source: r#"
|
||||||
let
|
let
|
||||||
recursive = recursive;
|
recursive = recursive;
|
||||||
in
|
in
|
||||||
recursive
|
recursive
|
||||||
"#
|
"#
|
||||||
.into(),
|
.into(),
|
||||||
}])
|
},
|
||||||
|
SourceFile {
|
||||||
|
path: PathBuf::from("testdata/dependent_let.nix"),
|
||||||
|
source: r#"
|
||||||
|
let
|
||||||
|
value = 1;
|
||||||
|
dependent = value;
|
||||||
|
in
|
||||||
|
value
|
||||||
|
"#
|
||||||
|
.into(),
|
||||||
|
},
|
||||||
|
])
|
||||||
.or_fail()?;
|
.or_fail()?;
|
||||||
|
|
||||||
let count = report
|
let count = report
|
||||||
|
|||||||
@@ -97,7 +97,7 @@ pub fn run_rules(file: &ast::ParsedFile, graph: &ScopeGraph, source: &str) -> Ve
|
|||||||
.map(|finding| Finding {
|
.map(|finding| Finding {
|
||||||
rule_id: "redundant_let",
|
rule_id: "redundant_let",
|
||||||
range: finding.range,
|
range: finding.range,
|
||||||
message: "Inline this single-binding let expression",
|
message: "Inline this returned let binding",
|
||||||
binding: None,
|
binding: None,
|
||||||
expected_order: None,
|
expected_order: None,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -45,11 +45,18 @@ fn check_expr(expr: &ast::Expr, findings: &mut Vec<Finding>) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn is_redundant(bindings: &[ast::Binding], body: &ast::Expr) -> bool {
|
fn is_redundant(bindings: &[ast::Binding], body: &ast::Expr) -> bool {
|
||||||
let [binding] = bindings else {
|
let Some((binding_index, binding)) = bindings
|
||||||
|
.iter()
|
||||||
|
.enumerate()
|
||||||
|
.find(|(_, binding)| is_ident(body, &binding.key.name))
|
||||||
|
else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
|
|
||||||
is_ident(body, &binding.key.name) && !references_name(&binding.value, &binding.key.name)
|
!references_name(&binding.value, &binding.key.name)
|
||||||
|
&& !bindings.iter().enumerate().any(|(candidate_index, candidate)| {
|
||||||
|
candidate_index != binding_index && references_name(&candidate.value, &binding.key.name)
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
fn is_ident(expr: &ast::Expr, name: &str) -> bool {
|
fn is_ident(expr: &ast::Expr, name: &str) -> bool {
|
||||||
|
|||||||
Reference in New Issue
Block a user