Add unscoped_variables

CHANGELOG.md

@@ -5,6 +5,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Added
 - Added `--color` option to specify whether colors could be used on the output.
 - Added [`incorrect_roact_usage`](https://kampfkarren.github.io/selene/lints/incorrect_roact_usage.html) lint to verify correct usage of Roact.createElement.
+- Added [`unscoped_variables`](https://kampfkarren.github.io/selene/lints/unscoped_variables.html) lint to disallow usage of unscoped (global) variables.
 
 ### Changed
 - Colors will no longer be on by default when being piped. [(#32)](https://github.com/Kampfkarren/selene/issues/32)

docs/src/SUMMARY.md

@@ -24,4 +24,5 @@
   - [suspicious_reverse_loop](./lints/suspicious_reverse_loop.md)
   - [unbalanced_assignments](./lints/unbalanced_assignments.md)
   - [undefined_variable](./lints/undefined_variable.md)
+  - [unscoped_variables](./lints/unscoped_variables.md)
   - [unused_variable](./lints/unused_variable.md)

docs/src/lints/unscoped_variables.md

@@ -0,0 +1,14 @@
+# unused_variable
+## What it does
+Checks for variables that are unscoped (don't have a local variable attached).
+
+## Why this is bad
+Unscoped variables make code harder to read and debug, as well as making it harder for selene to analyze.
+
+## Configuration
+`ignore_pattern` (default: `"^_"`) - A [regular expression](https://en.wikipedia.org/wiki/Regular_expression) for variables that are allowed to be unscoped. The default allows for variables like `_` to be unscoped, as they shouldn't be used anyway.
+
+## Example
+```lua
+baz = 3
+```

selene-lib/src/lib.rs

@@ -226,6 +226,7 @@ use_rules! {
     suspicious_reverse_loop: rules::suspicious_reverse_loop::SuspiciousReverseLoopLint,
     unbalanced_assignments: rules::unbalanced_assignments::UnbalancedAssignmentsLint,
     undefined_variable: rules::undefined_variable::UndefinedVariableLint,
+    unscoped_variables: rules::unscoped_variables::UnscopedVariablesLint,
     unused_variable: rules::unused_variable::UnusedVariableLint,
 
     #[cfg(feature = "roblox")]

selene-lib/src/rules.rs

@@ -20,6 +20,7 @@ pub mod standard_library;
 pub mod suspicious_reverse_loop;
 pub mod unbalanced_assignments;
 pub mod undefined_variable;
+pub mod unscoped_variables;
 pub mod unused_variable;
 
 #[cfg(feature = "roblox")]

selene-lib/src/rules/unscoped_variables.rs

@@ -0,0 +1,88 @@
+use super::*;
+use crate::ast_util::scopes::ScopeManager;
+use std::collections::HashSet;
+
+use full_moon::ast::Ast;
+use regex::Regex;
+use serde::Deserialize;
+
+#[derive(Clone, Deserialize)]
+#[serde(default)]
+pub struct UnscopedVariablesConfig {
+    ignore_pattern: String,
+}
+
+impl Default for UnscopedVariablesConfig {
+    fn default() -> Self {
+        Self {
+            ignore_pattern: "^_".to_owned(),
+        }
+    }
+}
+
+pub struct UnscopedVariablesLint {
+    ignore_pattern: Regex,
+}
+
+impl Rule for UnscopedVariablesLint {
+    type Config = UnscopedVariablesConfig;
+    type Error = regex::Error;
+
+    fn new(config: Self::Config) -> Result<Self, Self::Error> {
+        Ok(UnscopedVariablesLint {
+            ignore_pattern: Regex::new(&config.ignore_pattern)?,
+        })
+    }
+
+    fn pass(&self, ast: &Ast, context: &Context) -> Vec<Diagnostic> {
+        // ScopeManager repeats references, and I just don't want to fix it right now
+        let mut read = HashSet::new();
+
+        let mut diagnostics = Vec::new();
+        let scope_manager = ScopeManager::new(ast);
+
+        for (_, reference) in &scope_manager.references {
+            if reference.resolved.is_none()
+                && reference.write
+                && !read.contains(&reference.identifier)
+                && !self.ignore_pattern.is_match(&reference.name)
+                && !context
+                    .standard_library
+                    .globals
+                    .contains_key(&reference.name)
+            {
+                read.insert(reference.identifier);
+
+                diagnostics.push(Diagnostic::new(
+                    "unscoped_variables",
+                    format!("`{}` is not declared locally, and will be available in every scope", reference.name),
+                    Label::new(reference.identifier),
+                ));
+            }
+        }
+
+        diagnostics
+    }
+
+    fn severity(&self) -> Severity {
+        Severity::Warning
+    }
+
+    fn rule_type(&self) -> RuleType {
+        RuleType::Complexity
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{super::test_util::*, *};
+
+    #[test]
+    fn test_unscoped_variables() {
+        test_lint(
+            UnscopedVariablesLint::new(UnscopedVariablesConfig::default()).unwrap(),
+            "unscoped_variables",
+            "unscoped_variables",
+        );
+    }
+}

selene-lib/tests/lints/unscoped_variables/unscoped_variables.lua

@@ -0,0 +1,14 @@
+bar = true
+
+local foo = 1
+foo = 2
+
+local bar = 1
+bar = 2
+
+pcall(function()
+    foo = 3
+    baz = 1
+end)
+
+_ = 3

selene-lib/tests/lints/unscoped_variables/unscoped_variables.stderr

@@ -0,0 +1,16 @@
+error[unscoped_variables]: `bar` is not declared locally, and will be available in every scope
+
+   ┌── unscoped_variables.lua:1:1 ───
+   │
+ 1 │ bar = true
+   │ ^^^
+   │
+
+error[unscoped_variables]: `baz` is not declared locally, and will be available in every scope
+
+    ┌── unscoped_variables.lua:11:5 ───
+    │
+ 11 │     baz = 1
+    │     ^^^
+    │
+