diff --git a/CHANGELOG.md b/CHANGELOG.md index bd5f5c35..b6015620 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Moved `math.log` second argument addition from Lua 5.3 std lib to 5.2 std lib - `undefined_variable` now correctly errors when defining multiple methods in undefined tables - Corrected `os.exit` definition in Lua 5.2 standard library +- Fixed `manual_table_clone` incorrectly warning when loop and table are defined at different depths ## [0.25.0](https://github.com/Kampfkarren/selene/releases/tag/0.25.0) - 2023-03-12 ### Added diff --git a/docs/src/lints/manual_table_clone.md b/docs/src/lints/manual_table_clone.md index 52559832..0c88d5da 100644 --- a/docs/src/lints/manual_table_clone.md +++ b/docs/src/lints/manual_table_clone.md @@ -27,6 +27,7 @@ Very little outside this exact pattern is matched. This is the list of circumsta - Any usage of the output variable in between the definition and the loop (as determined by position in code). - If the input variable is not a plain locally initialized variable. For example, `self.state[key] = value` will not lint. - If the input variable is not defined as a completely empty table. +- If the loop and input variable are defined at different depths. --- diff --git a/selene-lib/src/ast_util/loop_tracker.rs b/selene-lib/src/ast_util/loop_tracker.rs index de35f4b3..dde70c92 100644 --- a/selene-lib/src/ast_util/loop_tracker.rs +++ b/selene-lib/src/ast_util/loop_tracker.rs @@ -1,3 +1,6 @@ +// Remove this once loop_tracker is used again +#![allow(dead_code)] + use full_moon::{ast, node::Node, visitors::Visitor}; #[derive(Debug, Clone, Copy)] diff --git a/selene-lib/src/lints/manual_table_clone.rs b/selene-lib/src/lints/manual_table_clone.rs index 731fa8ca..73b0b198 100644 --- a/selene-lib/src/lints/manual_table_clone.rs +++ b/selene-lib/src/lints/manual_table_clone.rs @@ -3,12 +3,10 @@ use full_moon::{ visitors::Visitor, }; -use crate::ast_util::{ - expression_to_ident, range, scopes::AssignedValue, strip_parentheses, LoopTracker, -}; +use crate::ast_util::{expression_to_ident, range, scopes::AssignedValue, strip_parentheses}; use super::*; -use std::convert::Infallible; +use std::{collections::HashSet, convert::Infallible}; pub struct ManualTableCloneLint; @@ -34,9 +32,9 @@ impl Lint for ManualTableCloneLint { let mut visitor = ManualTableCloneVisitor { matches: Vec::new(), - loop_tracker: LoopTracker::new(ast), scope_manager: &ast_context.scope_manager, - stmt_begins: Vec::new(), + completed_stmt_begins: Vec::new(), + inside_stmt_begins: HashSet::new(), }; visitor.visit_ast(ast); @@ -92,9 +90,9 @@ impl ManualTableCloneMatch { struct ManualTableCloneVisitor<'ast> { matches: Vec, - loop_tracker: LoopTracker, scope_manager: &'ast ScopeManager, - stmt_begins: Vec, + completed_stmt_begins: Vec, + inside_stmt_begins: HashSet, } #[derive(Debug)] @@ -214,7 +212,7 @@ impl ManualTableCloneVisitor<'_> { ) -> bool { debug_assert!(assigment_start > definition_end); - for &stmt_begin in self.stmt_begins.iter() { + for &stmt_begin in self.completed_stmt_begins.iter() { if stmt_begin > definition_end { return true; } else if stmt_begin > assigment_start { @@ -224,6 +222,13 @@ impl ManualTableCloneVisitor<'_> { false } + + fn get_depth_at_byte(&self, byte: usize) -> usize { + self.inside_stmt_begins + .iter() + .filter(|&&start| start < byte) + .count() + } } fn has_filter_comment(for_loop: &ast::GenericFor) -> bool { @@ -374,9 +379,7 @@ impl Visitor for ManualTableCloneVisitor<'_> { let (position_start, position_end) = range(node); - if self.loop_tracker.depth_at_byte(position_start) - != self.loop_tracker.depth_at_byte(*definition_start) - { + if self.get_depth_at_byte(*definition_start) != self.get_depth_at_byte(position_start) { return; } @@ -401,8 +404,13 @@ impl Visitor for ManualTableCloneVisitor<'_> { }); } + fn visit_stmt(&mut self, stmt: &ast::Stmt) { + self.inside_stmt_begins.insert(range(stmt).0); + } + fn visit_stmt_end(&mut self, stmt: &ast::Stmt) { - self.stmt_begins.push(range(stmt).0); + self.completed_stmt_begins.push(range(stmt).0); + self.inside_stmt_begins.remove(&range(stmt).0); } } diff --git a/selene-lib/tests/lints/manual_table_clone/false_positive.lua b/selene-lib/tests/lints/manual_table_clone/false_positive.lua index 52961fd4..200e62a6 100644 --- a/selene-lib/tests/lints/manual_table_clone/false_positive.lua +++ b/selene-lib/tests/lints/manual_table_clone/false_positive.lua @@ -39,6 +39,34 @@ local function falsePositive3(t) return result end +local result4 = {} +local function falsePositive4(t) + for key, value in t do + result3[key] = value + end +end + +local result5 = {} +local function falsePositive5(t) + local function f() end + + for key, value in t do + result4[key] = value + end +end + +local function falsePositive6(t) + local result = {} + + if b then return end + + if a then + for key, value in t do + result4[key] = value + end + end +end + local function notFalsePositive1(t) local result = {} diff --git a/selene-lib/tests/lints/manual_table_clone/false_positive.stderr b/selene-lib/tests/lints/manual_table_clone/false_positive.stderr index 356c9224..1af67e80 100644 --- a/selene-lib/tests/lints/manual_table_clone/false_positive.stderr +++ b/selene-lib/tests/lints/manual_table_clone/false_positive.stderr @@ -1,24 +1,24 @@ error[manual_table_clone]: manual implementation of table.clone - ┌─ false_positive.lua:49:2 + ┌─ false_positive.lua:77:2 │ -43 │ local result = {} +71 │ local result = {} │ ----------------- remove this definition · -49 │ ╭ for key, value in pairs(t) do -50 │ │ result[key] = value -51 │ │ end +77 │ ╭ for key, value in pairs(t) do +78 │ │ result[key] = value +79 │ │ end │ ╰───────^ │ = try `local result = table.clone(t)` error[manual_table_clone]: manual implementation of table.clone - ┌─ false_positive.lua:58:3 + ┌─ false_positive.lua:86:3 │ -58 │ ╭ local result = {} -59 │ │ -60 │ │ for key, value in pairs(t) do -61 │ │ result[key] = value -62 │ │ end +86 │ ╭ local result = {} +87 │ │ +88 │ │ for key, value in pairs(t) do +89 │ │ result[key] = value +90 │ │ end │ ╰───────────^ │ = try `local result = table.clone(t)`