Skip to content

Commit

Permalink
Skip fix with unused self in method call
Browse files Browse the repository at this point in the history
  • Loading branch information
chriscerie committed Oct 14, 2023
1 parent 5451778 commit ae7555e
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 48 deletions.
37 changes: 8 additions & 29 deletions selene-lib/src/lints/unused_variable.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
use crate::{
ast_util::{range, scopes::AssignedValue},
ast_util::scopes::AssignedValue,
standard_library::{Field, FieldKind, Observes},
};

use super::*;

use full_moon::{
ast::Ast,
node::Node,
tokenizer::{Symbol, TokenType},
};
use full_moon::ast::Ast;
use regex::Regex;
use serde::Deserialize;

Expand Down Expand Up @@ -55,7 +51,7 @@ impl Lint for UnusedVariableLint {
})
}

fn pass(&self, ast: &Ast, context: &Context, ast_context: &AstContext) -> Vec<Diagnostic> {
fn pass(&self, _: &Ast, context: &Context, ast_context: &AstContext) -> Vec<Diagnostic> {
let mut diagnostics = Vec::new();

for (_, variable) in ast_context
Expand Down Expand Up @@ -161,8 +157,7 @@ impl Lint for UnusedVariableLint {
{
let mut notes = Vec::new();

let mut variable_range = variable.identifiers[0];
let mut fixed_code = format!("_{}", variable.name);
let mut fixed_code = Some(format!("_{}", variable.name));

if variable.is_self {
if self.allow_unused_self {
Expand All @@ -173,24 +168,8 @@ impl Lint for UnusedVariableLint {
notes
.push("if you don't need it, consider using `.` instead of `:`".to_owned());

// This is a very hacky and fragile way to get the colon in the function call. Can we do better?
// We can't just use variable start - 1 due to cases like `function a: b() end`
let mut last_colon_start = 0;
for token in ast.tokens() {
let start_position: usize = range(token).0;
if start_position >= variable_range.0 {
break;
}

if let TokenType::Symbol { symbol } = token.token().token_type() {
if *symbol == Symbol::Colon {
last_colon_start = range(token).0;
}
}
}

variable_range = (last_colon_start, last_colon_start + 1);
fixed_code = ".".to_string();
// Automatically changing `:` to `.` would break any existing methods calls
fixed_code = None;
}

let write_only = !analyzed_references.is_empty();
Expand All @@ -202,7 +181,7 @@ impl Lint for UnusedVariableLint {
} else {
format!("{} is defined, but never used", variable.name)
},
Label::new(variable_range),
Label::new(variable.identifiers[0]),
notes,
analyzed_references
.into_iter()
Expand All @@ -214,7 +193,7 @@ impl Lint for UnusedVariableLint {
}
})
.collect(),
Some(fixed_code),
fixed_code,
));
};
}
Expand Down
9 changes: 3 additions & 6 deletions selene-lib/tests/lints/unused_variable/self.fixed.diff
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
local Foo = {}

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

return Foo
return Foo
3 changes: 1 addition & 2 deletions selene-lib/tests/lints/unused_variable/self.lua
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
local Foo = {}

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

return Foo
13 changes: 2 additions & 11 deletions selene-lib/tests/lints/unused_variable/self.stderr
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
error[unused_variable]: self is defined, but never used
┌─ self.lua:3:13
┌─ self.lua:3:14
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:4:16
4 │ function Foo : B() 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 ae7555e

Please sign in to comment.