Add rule to remove unused variables (#172)

Author: jeparlefrancais

.github/workflows/test.yml

@@ -44,7 +44,6 @@ jobs:
         run: |
           luarocks install luafilesystem
           luarocks install busted
-          luarocks install luacheck
 
       - name: Run end-to-end tests
         run: lua ./scripts/test-commands.lua

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+* add rule to remove unused variables (`remove_unused_variable`). Fix issue with `rename_variables` where `self` variables and some cases of variable shadowing were not correctly renamed ([#172](https://github.com/seaofvoices/darklua/pull/172))
+
 ## 0.12.1
 
 * fix `append_text_comment` rule to support multiline comments ([#167](https://github.com/seaofvoices/darklua/pull/167))

scripts/install-lua.sh

@@ -6,4 +6,3 @@ export PATH=$PATH:$PWD/lua_install/bin
 
 luarocks install luafilesystem
 luarocks install busted
-luarocks install luacheck

site/content/rules/append_text_comment.md

@@ -9,7 +9,7 @@ parameters:
     type: string
     description: A path to a file to be used as the comment content (required if `text` is not defined)
   - name: location
-    default: "start"
+    default: start
     type: '"start" or "end"'
     description: The location where to add the comment
 examples:

site/content/rules/remove_assertions.md

@@ -5,7 +5,7 @@ parameters:
   - name: preserve_arguments_side_effects
     type: boolean
     description: Defines how darklua handle arguments passed to the function. If true, darklua will inspect each argument and preserve any potential side effects. When false, darklua will not perform any verification and simply erase any arguments passed.
-    default: true
+    default: "true"
 examples:
   - content: assert(condition, 'condition is incorrect!')
 ---

site/content/rules/remove_debug_profiling.md

@@ -5,7 +5,7 @@ parameters:
   - name: preserve_arguments_side_effects
     type: boolean
     description: Defines how darklua handle arguments passed to the functions. If true, darklua will inspect each argument and preserve any potential side effects. When false, darklua will not perform any verification and simply erase any arguments passed.
-    default: true
+    default: "true"
 examples:
   - content: |
       debug.profilebegin('function name')

site/content/rules/remove_empty_do.md

@@ -12,3 +12,5 @@ examples:
       end
       return {}
 ---
+
+This simple rule removes all empty do blocks found.

site/content/rules/remove_unused_variable.md

@@ -0,0 +1,17 @@
+---
+description: Removes unused variable declarations
+added_in: "unreleased"
+parameters: []
+examples:
+  - content: "local var"
+  - content: |
+      local var1 = true
+      local var2 = var1
+  - content: "local var = call()"
+  - content: "local function fn() print('unused') end"
+  - content: |
+      local a, b, c = 1, 2, 3
+      return a
+---
+
+This rule removes unused variables from code. It also removes unused local function definitions.

site/gatsby-node.js

@@ -217,8 +217,8 @@ exports.createSchemaCustomization = ({ actions }) => {
     }
 
     type RuleParameter {
-      name: String
-      type: String
+      name: String!
+      type: String!
       default: String
       description: String
       added_in: String

src/nodes/block.rs

@@ -136,11 +136,21 @@ impl Block {
         self.last_statement.is_none() && self.statements.is_empty()
     }
 
+    #[inline]
+    pub fn statements_len(&self) -> usize {
+        self.statements.len()
+    }
+
     #[inline]
     pub fn iter_statements(&self) -> impl Iterator<Item = &Statement> {
         self.statements.iter()
     }
 
+    #[inline]
+    pub fn reverse_iter_statements(&self) -> impl Iterator<Item = &Statement> {
+        self.statements.iter().rev()
+    }
+
     #[inline]
     pub fn get_last_statement(&self) -> Option<&LastStatement> {
         self.last_statement.as_ref()

src/nodes/statements/function.rs

@@ -114,6 +114,11 @@ impl FunctionName {
         self.method.as_ref()
     }
 
+    #[inline]
+    pub fn has_method(&self) -> bool {
+        self.method.is_some()
+    }
+
     #[inline]
     pub fn get_name(&self) -> &Identifier {
         &self.name

src/process/processors/find_identifier.rs

@@ -11,8 +11,8 @@ use crate::{nodes::Identifier, process::NodeProcessor};
 /// # use darklua_core::nodes::Expression;
 /// # use darklua_core::process::processors::FindVariables;
 /// # use darklua_core::process::{DefaultVisitor, NodeProcessor, NodeVisitor};
-/// let variables = vec!["foo".to_owned()];
-/// let mut find_foo: FindVariables = variables.iter().collect();
+/// let variables = vec!["foo"];
+/// let mut find_foo: FindVariables = variables.into_iter().collect();
 ///
 /// let mut foo_expression = Expression::identifier("foo");
 /// DefaultVisitor::visit_expression(&mut foo_expression, &mut find_foo);
@@ -25,27 +25,34 @@ use crate::{nodes::Identifier, process::NodeProcessor};
 /// # use darklua_core::nodes::Expression;
 /// # use darklua_core::process::processors::FindVariables;
 /// # use darklua_core::process::{DefaultVisitor, NodeProcessor, NodeVisitor};
-/// # let variables = vec!["foo".to_owned()];
-/// # let mut find_foo: FindVariables = variables.iter().collect();
+/// # let variables = vec!["foo"];
+/// # let mut find_foo: FindVariables = variables.into_iter().collect();
 /// let mut bar_expression = Expression::identifier("bar");
 /// DefaultVisitor::visit_expression(&mut bar_expression, &mut find_foo);
 ///
 /// assert!(!find_foo.has_found_usage());
 /// ```
 pub struct FindVariables<'a> {
-    variables: Vec<&'a String>,
+    variables: Vec<&'a str>,
     usage_found: bool,
 }
 
 impl<'a> FindVariables<'a> {
+    pub fn new(variable: &'a str) -> Self {
+        Self {
+            variables: vec![variable],
+            usage_found: false,
+        }
+    }
+
     #[inline]
     pub fn has_found_usage(&self) -> bool {
         self.usage_found
     }
 }
 
-impl<'a> FromIterator<&'a String> for FindVariables<'a> {
-    fn from_iter<T: IntoIterator<Item = &'a String>>(iter: T) -> Self {
+impl<'a> FromIterator<&'a str> for FindVariables<'a> {
+    fn from_iter<T: IntoIterator<Item = &'a str>>(iter: T) -> Self {
         Self {
             variables: iter.into_iter().collect(),
             usage_found: false,

src/process/processors/find_usage.rs

@@ -0,0 +1,50 @@
+use std::ops;
+
+use crate::{
+    nodes::Identifier,
+    process::{IdentifierTracker, NodeProcessor},
+};
+
+/// A processor to find usage of a given variable.
+pub(crate) struct FindUsage<'a> {
+    variable: &'a str,
+    usage_found: bool,
+    identifier_tracker: IdentifierTracker,
+}
+
+impl<'a> ops::Deref for FindUsage<'a> {
+    type Target = IdentifierTracker;
+
+    fn deref(&self) -> &Self::Target {
+        &self.identifier_tracker
+    }
+}
+
+impl<'a> ops::DerefMut for FindUsage<'a> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.identifier_tracker
+    }
+}
+
+impl<'a> FindUsage<'a> {
+    pub fn new(variable: &'a str) -> Self {
+        Self {
+            variable,
+            usage_found: false,
+            identifier_tracker: Default::default(),
+        }
+    }
+
+    #[inline]
+    pub fn has_found_usage(&self) -> bool {
+        self.usage_found
+    }
+}
+
+impl<'a> NodeProcessor for FindUsage<'a> {
+    fn process_variable_expression(&mut self, variable: &mut Identifier) {
+        if !self.usage_found && variable.get_name() == self.variable {
+            self.usage_found = !self.is_identifier_used(self.variable);
+        }
+    }
+}

src/process/processors/mod.rs

@@ -1,5 +1,7 @@
 //! A collection of utility processors that can be used when creating rules.
 
 mod find_identifier;
+mod find_usage;
 
 pub use find_identifier::*;
+pub(crate) use find_usage::*;

src/process/scope_visitor.rs

@@ -18,6 +18,8 @@ pub trait Scope {
     /// Called when entering a function block (with each parameters of the function), with the
     /// identifiers from a generic for statement or the identifier from a numeric for loop.
     fn insert(&mut self, identifier: &mut String);
+    /// Called when entering a function defined with a method
+    fn insert_self(&mut self);
     /// Called when a new local variable is initialized.
     fn insert_local(&mut self, identifier: &mut String, value: Option<&mut Expression>);
     /// Called when a new local function is initialized.
@@ -36,13 +38,7 @@ impl ScopeVisitor {
             .for_each(|statement| Self::visit_statement(statement, scope));
 
         if let Some(last_statement) = block.mutate_last_statement() {
-            scope.process_last_statement(last_statement);
-
-            if let LastStatement::Return(expressions) = last_statement {
-                expressions
-                    .iter_mut_expressions()
-                    .for_each(|expression| Self::visit_expression(expression, scope));
-            };
+            Self::visit_last_statement(last_statement, scope);
         };
     }
 }
@@ -61,6 +57,13 @@ impl<T: NodeProcessor + Scope> NodeVisitor<T> for ScopeVisitor {
             .iter_mut_values()
             .for_each(|value| Self::visit_expression(value, scope));
 
+        for r#type in statement
+            .iter_mut_variables()
+            .filter_map(TypedIdentifier::mutate_type)
+        {
+            Self::visit_type(r#type, scope);
+        }
+
         statement.for_each_assignment(|variable, expression| {
             scope.insert_local(variable.mutate_name(), expression)
         });
@@ -69,6 +72,21 @@ impl<T: NodeProcessor + Scope> NodeVisitor<T> for ScopeVisitor {
     fn visit_function_expression(function: &mut FunctionExpression, scope: &mut T) {
         scope.process_function_expression(function);
 
+        for r#type in function
+            .iter_mut_parameters()
+            .filter_map(TypedIdentifier::mutate_type)
+        {
+            Self::visit_type(r#type, scope);
+        }
+
+        if let Some(variadic_type) = function.mutate_variadic_type() {
+            Self::visit_function_variadic_type(variadic_type, scope);
+        }
+
+        if let Some(return_type) = function.mutate_return_type() {
+            Self::visit_function_return_type(return_type, scope);
+        }
+
         scope.push();
         function
             .mutate_parameters()
@@ -83,7 +101,25 @@ impl<T: NodeProcessor + Scope> NodeVisitor<T> for ScopeVisitor {
         scope.process_function_statement(statement);
         scope.process_variable_expression(statement.mutate_function_name().mutate_identifier());
 
+        for r#type in statement
+            .iter_mut_parameters()
+            .filter_map(TypedIdentifier::mutate_type)
+        {
+            Self::visit_type(r#type, scope);
+        }
+
+        if let Some(variadic_type) = statement.mutate_variadic_type() {
+            Self::visit_function_variadic_type(variadic_type, scope);
+        }
+
+        if let Some(return_type) = statement.mutate_return_type() {
+            Self::visit_function_return_type(return_type, scope);
+        }
+
         scope.push();
+        if statement.get_name().has_method() {
+            scope.insert_self();
+        }
         statement
             .mutate_parameters()
             .iter_mut()
@@ -98,6 +134,21 @@ impl<T: NodeProcessor + Scope> NodeVisitor<T> for ScopeVisitor {
 
         scope.insert_local_function(statement);
 
+        for r#type in statement
+            .iter_mut_parameters()
+            .filter_map(TypedIdentifier::mutate_type)
+        {
+            Self::visit_type(r#type, scope);
+        }
+
+        if let Some(variadic_type) = statement.mutate_variadic_type() {
+            Self::visit_function_variadic_type(variadic_type, scope);
+        }
+
+        if let Some(return_type) = statement.mutate_return_type() {
+            Self::visit_function_return_type(return_type, scope);
+        }
+
         scope.push();
         statement
             .mutate_parameters()
@@ -119,6 +170,13 @@ impl<T: NodeProcessor + Scope> NodeVisitor<T> for ScopeVisitor {
             .iter_mut_identifiers()
             .for_each(|identifier| scope.insert(identifier.mutate_name()));
 
+        for r#type in statement
+            .iter_mut_identifiers()
+            .filter_map(TypedIdentifier::mutate_type)
+        {
+            Self::visit_type(r#type, scope);
+        }
+
         Self::visit_block(statement.mutate_block(), scope);
     }
 
@@ -132,6 +190,10 @@ impl<T: NodeProcessor + Scope> NodeVisitor<T> for ScopeVisitor {
             Self::visit_expression(step, scope);
         };
 
+        if let Some(r#type) = statement.mutate_identifier().mutate_type() {
+            Self::visit_type(r#type, scope);
+        }
+
         scope.push();
         scope.insert(statement.mutate_identifier().mutate_name());
 
@@ -220,6 +282,10 @@ impl Scope for IdentifierTracker {
         self.insert_identifier(identifier);
     }
 
+    fn insert_self(&mut self) {
+        self.insert_identifier("self");
+    }
+
     fn insert_local(&mut self, identifier: &mut String, _value: Option<&mut Expression>) {
         self.insert_identifier(identifier);
     }
@@ -250,6 +316,11 @@ where
         self.deref_mut().insert(identifier);
     }
 
+    #[inline]
+    fn insert_self(&mut self) {
+        self.deref_mut().insert_self();
+    }
+
     #[inline]
     fn insert_local(&mut self, identifier: &mut String, value: Option<&mut Expression>) {
         self.deref_mut().insert_local(identifier, value)