Skip to content

Commit

Permalink
Add unused self fix
Browse files Browse the repository at this point in the history
  • Loading branch information
chriscerie committed Oct 19, 2023
1 parent 22b2fee commit 6749c77
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 8 deletions.
2 changes: 2 additions & 0 deletions selene-lib/src/lints/test_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
);
Expand Down
20 changes: 14 additions & 6 deletions selene-lib/src/lints/unused_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -206,7 +214,7 @@ impl Lint for UnusedVariableLint {
})
.collect(),
fixed_code,
Applicability::MachineApplicable,
applicability,
));
};
}
Expand Down
9 changes: 9 additions & 0 deletions selene-lib/tests/lints/unused_variable/self.fixed.diff
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions selene-lib/tests/lints/unused_variable/self.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ local Foo = {}

function Foo:A() end
function Foo.B() end
function Foo : C() end

return Foo
13 changes: 11 additions & 2 deletions selene-lib/tests/lints/unused_variable/self.stderr
Original file line number Diff line number Diff line change
@@ -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 `:`
Expand Down

0 comments on commit 6749c77

Please sign in to comment.