diff --git a/selene-lib/src/lints/test_util.rs b/selene-lib/src/lints/test_util.rs index 4c5640eb..a6a2ab07 100644 --- a/selene-lib/src/lints/test_util.rs +++ b/selene-lib/src/lints/test_util.rs @@ -187,6 +187,8 @@ pub fn test_lint_config_with_output< if let Ok(expected) = fs::read_to_string(&diff_output_path) { pretty_assertions::assert_str_eq!( + // Must normalize newline characters otherwise testing on windows locally passes but fails + // in github actions environment &expected.replace("\r\n", "\n"), &fixed_diff.replace("\r\n", "\n") ); diff --git a/selene-lib/src/lints/unused_variable.rs b/selene-lib/src/lints/unused_variable.rs index 067c48ff..e3f40431 100644 --- a/selene-lib/src/lints/unused_variable.rs +++ b/selene-lib/src/lints/unused_variable.rs @@ -156,8 +156,10 @@ impl Lint for UnusedVariableLint { .any(|reference| reference == &AnalyzedReference::Read) { let mut notes = Vec::new(); + let mut report_range = variable.identifiers[0]; let mut fixed_code = Some(format!("_{}", variable.name)); + let mut applicability = Applicability::MachineApplicable; if variable.is_self { if self.allow_unused_self { @@ -168,13 +170,19 @@ impl Lint for UnusedVariableLint { notes .push("if you don't need it, consider using `.` instead of `:`".to_owned()); - // Applying fix by changing `:` to `.` would break any existing methods calls - fixed_code = None; + if let Some(colon_position) = + ast_context.code[..variable.identifiers[0].0].rfind(':') + { + report_range = (colon_position, colon_position + 1); + + // Applying fix by changing `:` to `.` would break any existing methods calls + applicability = Applicability::MaybeIncorrect; + fixed_code = Some(".".to_string()); + } } // Applying fix would cause existing references to reference a nonexistent variable. It's possible to - // also rename those references as well, but we'd need to check each of their scopes for potentially - // colliding variables. Clippy just doesn't apply a fix at all in these cases. + // also rename those references as well. Clippy just doesn't apply a fix at all in these cases. if variable.references.iter().any(|reference| { ast_context.scope_manager.references[*reference] .identifier @@ -193,7 +201,7 @@ impl Lint for UnusedVariableLint { } else { format!("{} is defined, but never used", variable.name) }, - Label::new(variable.identifiers[0]), + Label::new(report_range), notes, analyzed_references .into_iter() @@ -206,7 +214,7 @@ impl Lint for UnusedVariableLint { }) .collect(), fixed_code, - Applicability::MachineApplicable, + applicability, )); }; } diff --git a/selene-lib/tests/lints/unused_variable/self.fixed.diff b/selene-lib/tests/lints/unused_variable/self.fixed.diff index e69de29b..122a9dbe 100644 --- a/selene-lib/tests/lints/unused_variable/self.fixed.diff +++ b/selene-lib/tests/lints/unused_variable/self.fixed.diff @@ -0,0 +1,9 @@ + local Foo = {} + +-[MI] function Foo:A() end ++[MI] function Foo.A() end + function Foo.B() end +-[MI] function Foo : C() end ++[MI] function Foo . C() end + + return Foo diff --git a/selene-lib/tests/lints/unused_variable/self.lua b/selene-lib/tests/lints/unused_variable/self.lua index 15a3ca8a..2bb2b976 100644 --- a/selene-lib/tests/lints/unused_variable/self.lua +++ b/selene-lib/tests/lints/unused_variable/self.lua @@ -2,5 +2,6 @@ local Foo = {} function Foo:A() end function Foo.B() end +function Foo : C() end return Foo diff --git a/selene-lib/tests/lints/unused_variable/self.stderr b/selene-lib/tests/lints/unused_variable/self.stderr index 7f7eda99..40f81522 100644 --- a/selene-lib/tests/lints/unused_variable/self.stderr +++ b/selene-lib/tests/lints/unused_variable/self.stderr @@ -1,8 +1,17 @@ error[unused_variable]: self is defined, but never used - ┌─ self.lua:3:14 + ┌─ self.lua:3:13 │ 3 │ function Foo:A() end - │ ^ + │ ^ + │ + = `self` is implicitly defined when defining a method + = if you don't need it, consider using `.` instead of `:` + +error[unused_variable]: self is defined, but never used + ┌─ self.lua:5:17 + │ +5 │ function Foo : C() end + │ ^ │ = `self` is implicitly defined when defining a method = if you don't need it, consider using `.` instead of `:`