From 6749c77145c32bc26e1293dc28dfee2da1153601 Mon Sep 17 00:00:00 2001
From: Christopher Chang <51393127+chriscerie@users.noreply.github.com>
Date: Wed, 18 Oct 2023 17:19:52 -0700
Subject: [PATCH] Add unused self fix

---
 selene-lib/src/lints/test_util.rs             |  2 ++
 selene-lib/src/lints/unused_variable.rs       | 20 +++++++++++++------
 .../lints/unused_variable/self.fixed.diff     |  9 +++++++++
 .../tests/lints/unused_variable/self.lua      |  1 +
 .../tests/lints/unused_variable/self.stderr   | 13 ++++++++++--
 5 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/selene-lib/src/lints/test_util.rs b/selene-lib/src/lints/test_util.rs
index 4c5640eb..a6a2ab07 100644
--- a/selene-lib/src/lints/test_util.rs
+++ b/selene-lib/src/lints/test_util.rs
@@ -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")
         );
diff --git a/selene-lib/src/lints/unused_variable.rs b/selene-lib/src/lints/unused_variable.rs
index 067c48ff..e3f40431 100644
--- a/selene-lib/src/lints/unused_variable.rs
+++ b/selene-lib/src/lints/unused_variable.rs
@@ -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 {
@@ -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
@@ -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()
@@ -206,7 +214,7 @@ impl Lint for UnusedVariableLint {
                         })
                         .collect(),
                     fixed_code,
-                    Applicability::MachineApplicable,
+                    applicability,
                 ));
             };
         }
diff --git a/selene-lib/tests/lints/unused_variable/self.fixed.diff b/selene-lib/tests/lints/unused_variable/self.fixed.diff
index e69de29b..122a9dbe 100644
--- a/selene-lib/tests/lints/unused_variable/self.fixed.diff
+++ b/selene-lib/tests/lints/unused_variable/self.fixed.diff
@@ -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
diff --git a/selene-lib/tests/lints/unused_variable/self.lua b/selene-lib/tests/lints/unused_variable/self.lua
index 15a3ca8a..2bb2b976 100644
--- a/selene-lib/tests/lints/unused_variable/self.lua
+++ b/selene-lib/tests/lints/unused_variable/self.lua
@@ -2,5 +2,6 @@ local Foo = {}
 
 function Foo:A() end
 function Foo.B() end
+function Foo    :    C() end
 
 return Foo
diff --git a/selene-lib/tests/lints/unused_variable/self.stderr b/selene-lib/tests/lints/unused_variable/self.stderr
index 7f7eda99..40f81522 100644
--- a/selene-lib/tests/lints/unused_variable/self.stderr
+++ b/selene-lib/tests/lints/unused_variable/self.stderr
@@ -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 `:`