Skip to content

Commit

Permalink
Fix explicit self variables not marking as unused -- Closes #215 (#264)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kampfkarren authored Jun 21, 2021
1 parent fd4b884 commit 4436847
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Fixed
- Fixed internal selene panics exiting with a zero code. Panics will now exit with status code 1, allowing it to be picked up by CI.
- Fixed variables named `self` not showing as unused if `allow_unused_self` was enabled. The implicit `self` variable being unused will still respect this configuration. [(#215)](https://github.com/Kampfkarren/selene/issues/215)

## [0.12.1] - 2021-05-26
### Fixed
Expand Down
25 changes: 22 additions & 3 deletions selene-lib/src/ast_util/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ pub struct Variable {
pub name: String,
pub references: Vec<Id<Reference>>,
pub shadowed: Option<Id<Variable>>,
pub is_self: bool,
}

#[derive(Default)]
Expand Down Expand Up @@ -312,18 +313,19 @@ impl ScopeVisitor {
self.define_name_full(&token.token().to_string(), range(token), definition_range);
}

fn define_name_full(
fn define_name_full_with_variable(
&mut self,
name: &str,
range: Range,
definition_range: Range,
variable: Variable,
) -> Id<Variable> {
let shadowed = self.find_variable(name).map(|(var, _)| var);

let id = self.scope_manager.variables.alloc(Variable {
name: name.to_owned(),
shadowed,
..Variable::default()
..variable
});

self.current_scope().variables.push(id);
Expand All @@ -336,6 +338,15 @@ impl ScopeVisitor {
id
}

fn define_name_full(
&mut self,
name: &str,
range: Range,
definition_range: Range,
) -> Id<Variable> {
self.define_name_full_with_variable(name, range, definition_range, Variable::default())
}

fn try_hoist(&mut self) {
let latest_reference_id = *self.current_scope().references.last().unwrap();
let (name, identifier, write_expr) = {
Expand Down Expand Up @@ -570,7 +581,15 @@ impl Visitor<'_> for ScopeVisitor {

if let Some(name) = name.method_name() {
self.open_scope(declaration.body());
self.define_name_full("self", range(name), range(name));
self.define_name_full_with_variable(
"self",
range(name),
range(name),
Variable {
is_self: true,
..Variable::default()
},
);
}
}

Expand Down
29 changes: 19 additions & 10 deletions selene-lib/src/rules/unused_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl Rule for UnusedVariableLint {
if !references.clone().any(|reference| reference.read) {
let mut notes = Vec::new();

if variable.name == "self" {
if variable.is_self {
if self.allow_unused_self {
continue;
}
Expand Down Expand Up @@ -122,6 +122,15 @@ mod tests {
);
}

#[test]
fn test_explicit_self() {
test_lint(
UnusedVariableLint::new(UnusedVariableConfig::default()).unwrap(),
"unused_variable",
"explicit_self",
);
}

#[test]
fn test_generic_for_shadowing() {
test_lint(
Expand Down Expand Up @@ -149,6 +158,15 @@ mod tests {
);
}

#[test]
fn test_invalid_regex() {
assert!(UnusedVariableLint::new(UnusedVariableConfig {
ignore_pattern: "(".to_owned(),
..UnusedVariableConfig::default()
})
.is_err());
}

#[test]
fn test_mutating_functions() {
test_lint(
Expand Down Expand Up @@ -215,13 +233,4 @@ mod tests {
"varargs",
);
}

#[test]
fn test_invalid_regex() {
assert!(UnusedVariableLint::new(UnusedVariableConfig {
ignore_pattern: "(".to_owned(),
..UnusedVariableConfig::default()
})
.is_err());
}
}
4 changes: 4 additions & 0 deletions selene-lib/tests/lints/unused_variable/explicit_self.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
local function foo(self)
end

foo()
6 changes: 6 additions & 0 deletions selene-lib/tests/lints/unused_variable/explicit_self.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error[unused_variable]: self is defined, but never used
┌─ explicit_self.lua:1:20
1 │ local function foo(self)
│ ^^^^

0 comments on commit 4436847

Please sign in to comment.