Add fixers

CHANGELOG.md

@@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Added `bit32.byteswap` to Luau standard library
 - Added `buffer` library to Luau standard library
 - Added `SharedTable` to Roblox standard library
+- Added new [`--fix`](https://kampfkarren.github.io/selene/cli/usage.html#fix) flag, which will automatically apply lint suggestions.
 
 ### Changed
 - Updated internal parser, which includes floor division (`//`), more correct parsing of string interpolation with double braces, and better parsing of `\z` escapes.

Cargo.toml

@@ -1,18 +1,18 @@
-[workspace]
-members = ["selene", "selene-lib"]
-resolver = "2"
-
-[workspace.package]
-version = "0.25.0"
-authors = ["Kampfkarren <kampfkarren@gmail.com>"]
-edition = "2021"
-homepage = "https://kampfkarren.github.io/selene/"
-license = "MPL-2.0"
-repository = "https://github.com/Kampfkarren/selene"
-
-[workspace.dependencies]
-full_moon = "0.19.0"
-toml = "0.7.2"
-
-# Do not update this without confirming profiling uses the same version of tracy-client as selene
-profiling = "1.0.7"
+[workspace]
+members = ["selene", "selene-lib"]
+resolver = "2"
+
+[workspace.package]
+version = "0.25.0"
+authors = ["Kampfkarren <kampfkarren@gmail.com>"]
+edition = "2021"
+homepage = "https://kampfkarren.github.io/selene/"
+license = "MPL-2.0"
+repository = "https://github.com/Kampfkarren/selene"
+
+[workspace.dependencies]
+full_moon = "0.19.0"
+toml = "0.7.2"
+
+# Do not update this without confirming profiling uses the same version of tracy-client as selene
+profiling = "1.0.7"

docs/src/cli/usage.md

@@ -9,6 +9,7 @@ USAGE:
 FLAGS:
         --allow-warnings    Pass when only warnings occur
         --no-exclude        Ignore excludes defined in config
+        --fix               Automatically fix applicable lint warnings
     -h, --help              Prints help information
     -n, --no-summary        Suppress summary information
     -q, --quiet             Display only the necessary information. Equivalent to --display-style="quiet"
@@ -76,6 +77,10 @@ Results:
 0 parse errors
 ```
 
+**--fix** *fix*
+
+Automatically applies lint suggestions. Since this can be potentially destructive, it's not allowed when there are uncommitted changes. This safety mechanism can be ignored by including `--allow-dirty` to allow unstaged changes or `--allow-staged` to allow only staged changes.
+
 **--num-threads** *num-threads*
 
 Specifies the number of threads for selene to use. Defaults to however many cores your CPU has. If you type `selene --help`, you can see this number because it will show as the default for you.

selene-lib/Cargo.toml

@@ -22,6 +22,7 @@ profiling.workspace = true
 regex = "1.7.1"
 serde = "1.0.152"
 serde_yaml = "0.9.16"
+similar = "2.3.0"
 toml.workspace = true
 
 [dev-dependencies]

selene-lib/src/lib.rs

@@ -229,10 +229,10 @@ macro_rules! use_lints {
                 })
             }
 
-            pub fn test_on(&self, ast: &Ast) -> Vec<CheckerDiagnostic> {
+            pub fn test_on(&self, ast: &Ast, code: &String) -> Vec<CheckerDiagnostic> {
                 let mut diagnostics = Vec::new();
 
-                let ast_context = AstContext::from_ast(ast);
+                let ast_context = AstContext::from_ast(ast, &code.to_string());
 
                 macro_rules! check_lint {
                     ($name:ident) => {

selene-lib/src/lint_filtering.rs

@@ -4,7 +4,7 @@ use crate::{
         visit_nodes::{NodeVisitor, VisitorType},
     },
     lint_exists,
-    lints::{Diagnostic, Label, Severity},
+    lints::{Applicability, Diagnostic, Label, Severity},
     CheckerDiagnostic, LintVariation,
 };
 use full_moon::{ast::Ast, node::Node, tokenizer::TokenType};
@@ -149,6 +149,8 @@ impl NodeVisitor for FilterVisitor {
                                     trivia_start_position.bytes(),
                                     trivia_end_position.bytes(),
                                 )),
+                                None,
+                                Applicability::Unspecified,
                             ))
                         }
                     }));
@@ -223,6 +225,8 @@ pub fn filter_diagnostics(
                                 (first_code.0.bytes(), first_code.1.bytes()),
                                 "global filter must be before this".to_owned(),
                             )],
+                            None,
+                            Applicability::Unspecified,
                         ));
 
                         continue;
@@ -244,6 +248,8 @@ pub fn filter_diagnostics(
                                     possibly_conflicting.comment_range,
                                     "conflicts with this".to_owned(),
                                 )],
+                                None,
+                                Applicability::Unspecified,
                             ));
                         }
                     }

selene-lib/src/lints.rs

@@ -89,21 +89,66 @@ pub enum Severity {
     Warning,
 }
 
+/// Indicates the confidence in the correctness of a suggestion
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum Applicability {
+    /// The suggestion is definitely what the user intended, or maintains the exact meaning of the code
+    ///
+    /// The suggestion is safe to be automatically applied
+    MachineApplicable,
+
+    /// The suggestion is probably what the user intended, but may not maintain the exact meaning of the code
+    ///
+    /// The suggestion generates valid code when applied
+    MaybeIncorrect,
+
+    /// The suggestion is probably what the user intended, but may not maintain the exact meaning of the code
+    ///
+    /// The suggestion does not generate valid code when applied
+    HasPlaceholders,
+
+    Unspecified,
+}
+
 #[derive(Debug)]
 pub struct Diagnostic {
     pub code: &'static str,
     pub message: String,
     pub notes: Vec<String>,
     pub primary_label: Label,
     pub secondary_labels: Vec<Label>,
+    pub suggestion: Option<String>,
+    pub applicability: Applicability,
 }
 
 impl Diagnostic {
-    pub fn new(code: &'static str, message: String, primary_label: Label) -> Self {
+    pub fn new(
+        code: &'static str,
+        message: String,
+        primary_label: Label,
+        suggestion: Option<String>,
+        applicability: Applicability,
+    ) -> Self {
+        let mut label = primary_label;
+        if let Some(ref suggestion_str) = suggestion {
+            if label.message.is_none() {
+                label = Label::new_with_message(
+                    label.range,
+                    if suggestion_str.is_empty() {
+                        "consider removing this".to_string()
+                    } else {
+                        format!("try: `{suggestion_str}`")
+                    },
+                );
+            }
+        };
+
         Self {
             code,
             message,
-            primary_label,
+            primary_label: label,
+            suggestion,
+            applicability,
 
             notes: Vec::new(),
             secondary_labels: Vec::new(),
@@ -116,13 +161,17 @@ impl Diagnostic {
         primary_label: Label,
         notes: Vec<String>,
         secondary_labels: Vec<Label>,
+        suggestion: Option<String>,
+        applicability: Applicability,
     ) -> Self {
         Self {
             code,
             message,
             notes,
             primary_label,
             secondary_labels,
+            suggestion,
+            applicability,
         }
     }
 
@@ -147,9 +196,107 @@ impl Diagnostic {
         }
     }
 
+    #[must_use]
     pub fn start_position(&self) -> u32 {
         self.primary_label.range.0
     }
+
+    #[must_use]
+    pub fn has_machine_applicable_fix(&self) -> bool {
+        self.suggestion.is_some() && self.applicability == Applicability::MachineApplicable
+    }
+
+    #[must_use]
+    pub fn has_maybe_incorrect_fix(&self) -> bool {
+        self.suggestion.is_some() && self.applicability == Applicability::MaybeIncorrect
+    }
+
+    /// After applying suggestions, calls `get_new_diagnostics` and reruns to ensure fixes didn't produce new errors
+    pub fn get_applied_suggestions_code<F>(
+        code: &str,
+        diagnostics: &[&Diagnostic],
+        get_new_diagnostics: F,
+    ) -> String
+    where
+        F: Fn(&str) -> Vec<Diagnostic>,
+    {
+        let mut chosen_diagnostics = Self::get_independent_suggestions(diagnostics);
+        let mut owned_diagnostics_ref;
+        let mut owned_diagnostics;
+
+        let mut fixed_code = code.to_string();
+
+        while !chosen_diagnostics.is_empty() {
+            let mut bytes_offset = 0;
+
+            for diagnostic in chosen_diagnostics {
+                if let Some(suggestion) = &diagnostic.suggestion {
+                    let (start, end) = diagnostic.primary_label.range;
+
+                    // This conversion can theoretically overflow, but it's tied to string length so the user
+                    // would face memory constraints of such large strings long before this is an issue
+                    let start_with_offset = start as isize + bytes_offset;
+                    let end_with_offset = end as isize + bytes_offset;
+
+                    fixed_code = format!(
+                        "{}{}{}",
+                        // Conversion is safe as this algorithm guarantees range + offset will never be negative
+                        &fixed_code[..start_with_offset as usize],
+                        suggestion.as_str(),
+                        &fixed_code[(end_with_offset as usize)..]
+                    );
+
+                    bytes_offset +=
+                        suggestion.len() as isize - (end_with_offset - start_with_offset);
+                }
+            }
+
+            owned_diagnostics = get_new_diagnostics(&fixed_code);
+            owned_diagnostics_ref = owned_diagnostics.iter().collect::<Vec<_>>();
+            chosen_diagnostics = Self::get_independent_suggestions(&owned_diagnostics_ref);
+        }
+
+        fixed_code
+    }
+
+    /// * Chooses all disjoint suggestions
+    /// * If a suggestion is completely contained inside others, uses the innermost one and discards the outer ones
+    /// * If suggestions partially overlap, chooses the one that starts first and discard the other ones
+    fn get_independent_suggestions<'a>(diagnostics: &'a [&'a Diagnostic]) -> Vec<&'a Diagnostic> {
+        let mut sorted_diagnostics = diagnostics
+            .iter()
+            .filter(|&&diagnostic| {
+                diagnostic.has_machine_applicable_fix() || diagnostic.has_maybe_incorrect_fix()
+            })
+            .copied()
+            .collect::<Vec<_>>();
+        sorted_diagnostics.sort_by_key(|diagnostic| diagnostic.start_position());
+
+        let mut chosen_diagnostics = Vec::new();
+
+        let mut candidate = match sorted_diagnostics.first() {
+            Some(first) => *first,
+            None => return Vec::new(),
+        };
+
+        for &diagnostic in sorted_diagnostics.iter().skip(1) {
+            let this_range = diagnostic.primary_label.range;
+
+            if this_range.1 <= candidate.primary_label.range.1 {
+                // Completely contained inside
+                candidate = diagnostic;
+            } else if this_range.0 <= candidate.primary_label.range.1 {
+                // Partially overlapping
+                continue;
+            } else {
+                chosen_diagnostics.push(candidate);
+                candidate = diagnostic;
+            }
+        }
+        chosen_diagnostics.push(candidate);
+
+        chosen_diagnostics
+    }
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -234,12 +381,17 @@ impl Context {
 #[derive(Debug)]
 pub struct AstContext {
     pub scope_manager: ScopeManager,
+
+    // Converting ast -> code should probably come from the parser, but this should be ok as a temporary solution
+    pub code: String,
 }
 
 impl AstContext {
-    pub fn from_ast(ast: &Ast) -> Self {
+    #[must_use]
+    pub fn from_ast(ast: &Ast, code: &String) -> Self {
         Self {
             scope_manager: ScopeManager::new(ast),
+            code: code.to_string(),
         }
     }
 }

selene-lib/src/lints/almost_swapped.rs

@@ -31,20 +31,22 @@ impl Lint for AlmostSwappedLint {
             .almost_swaps
             .iter()
             .map(|almost_swap| {
-                Diagnostic::new_complete(
+                Diagnostic::new(
                     "almost_swapped",
                     format!(
                         "this looks like you are trying to swap `{}` and `{}`",
                         (almost_swap.names.0),
                         (almost_swap.names.1),
                     ),
                     Label::new(almost_swap.range),
-                    vec![format!(
-                        "try: `{name1}, {name2} = {name2}, {name1}`",
+                    Some(format!(
+                        "{name1}, {name2} = {name2}, {name1}",
                         name1 = almost_swap.names.0,
                         name2 = almost_swap.names.1,
-                    )],
-                    Vec::new(),
+                    )),
+                    // FIXME: Ideally `MaybeIncorrect`, but the end range for swapping `t[1]` and `t[2]`
+                    // currently doesn't include the closing `]`
+                    Applicability::HasPlaceholders,
                 )
             })
             .collect()