Standardize try message for code suggestion

selene-lib/src/lints.rs

@@ -129,14 +129,20 @@ impl Diagnostic {
         fixed_code: Option<String>,
         applicability: Applicability,
     ) -> Self {
+        let notes = if let Some(ref fixed_code_str) = fixed_code {
+            vec![format!("try: `{}`", fixed_code_str)]
+        } else {
+            Vec::new()
+        };
+
         Self {
             code,
             message,
             primary_label,
             fixed_code,
             applicability,
+            notes,
 
-            notes: Vec::new(),
             secondary_labels: Vec::new(),
         }
     }

selene-lib/src/lints/almost_swapped.rs

@@ -31,22 +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(),
-                    None,
-                    Applicability::Unspecified,
+                    )),
+                    // FIXME: Ideally `MaybeIncorrect`, but cannot because the end range of swapping `t[1]` and `t[2]`
+                    // doesn't include the closing `]`
+                    Applicability::HasPlaceholders,
                 )
             })
             .collect()

selene-lib/src/lints/compare_nan.rs

@@ -36,12 +36,10 @@ impl Lint for CompareNanLint {
                     operator = comparisons.operator,
                 );
 
-                Diagnostic::new_complete(
+                Diagnostic::new(
                     "compare_nan",
                     "comparing things to nan directly is not allowed".to_owned(),
                     Label::new(comparisons.range),
-                    vec![format!("try: `{}` instead", fixed_code.clone())],
-                    Vec::new(),
                     Some(fixed_code),
                     Applicability::MaybeIncorrect,
                 )

selene-lib/src/lints/constant_table_comparison.rs

@@ -32,35 +32,37 @@ impl Lint for ConstantTableComparisonLint {
             .comparisons
             .iter()
             .map(|comparison| {
-                Diagnostic::new_complete(
+                let suggestion = comparison.empty_side.as_ref().map(|empty_side| {
+                    format!(
+                        "next({}) {} nil",
+                        match empty_side {
+                            EmptyComparison::CheckEmpty(side) => match side {
+                                EmptyComparisonSide::Left => &comparison.rhs,
+                                EmptyComparisonSide::Right => &comparison.lhs,
+                            },
+
+                            EmptyComparison::CheckNotEmpty(side) => match side {
+                                EmptyComparisonSide::Left => &comparison.rhs,
+                                EmptyComparisonSide::Right => &comparison.lhs,
+                            },
+                        },
+                        match empty_side {
+                            EmptyComparison::CheckEmpty(_) => "==",
+                            EmptyComparison::CheckNotEmpty(_) => "~=",
+                        }
+                    )
+                });
+
+                Diagnostic::new(
                     "constant_table_comparison",
                     "comparing to a constant table will always fail".to_owned(),
                     Label::new(comparison.range),
-                    if let Some(empty_side) = comparison.empty_side {
-                        vec![format!(
-                            "try: `next({}) {} nil`",
-                            match empty_side {
-                                EmptyComparison::CheckEmpty(side) => match side {
-                                    EmptyComparisonSide::Left => &comparison.rhs,
-                                    EmptyComparisonSide::Right => &comparison.lhs,
-                                },
-
-                                EmptyComparison::CheckNotEmpty(side) => match side {
-                                    EmptyComparisonSide::Left => &comparison.rhs,
-                                    EmptyComparisonSide::Right => &comparison.lhs,
-                                },
-                            },
-                            match empty_side {
-                                EmptyComparison::CheckEmpty(_) => "==",
-                                EmptyComparison::CheckNotEmpty(_) => "~=",
-                            }
-                        )]
+                    suggestion.clone(),
+                    if suggestion.is_some() {
+                        Applicability::MaybeIncorrect
                     } else {
-                        Vec::new()
+                        Applicability::Unspecified
                     },
-                    Vec::new(),
-                    None,
-                    Applicability::Unspecified,
                 )
             })
             .collect()

selene-lib/src/lints/standard_library.rs

@@ -463,7 +463,7 @@ impl Visitor for StandardLibraryVisitor<'_> {
 
             let name = name_path.pop().unwrap();
 
-            self.diagnostics.push(Diagnostic::new_complete(
+            self.diagnostics.push(Diagnostic::new(
                 "incorrect_standard_library_use",
                 format!(
                     "standard library function `{}{}{}` {}",
@@ -473,15 +473,13 @@ impl Visitor for StandardLibraryVisitor<'_> {
                     problem,
                 ),
                 Label::from_node(call, None),
-                vec![format!(
-                    "try: {}{}{}(...)",
+                Some(format!(
+                    "{}{}{}(...)",
                     name_path.join("."),
                     use_instead,
                     name
-                )],
-                Vec::new(),
-                None,
-                Applicability::Unspecified,
+                )),
+                Applicability::HasPlaceholders,
             ));
 
             return;

selene-lib/src/lints/test_util.rs

@@ -1,4 +1,4 @@
-use super::{AstContext, Context, Diagnostic, Lint};
+use super::{Applicability, AstContext, Context, Diagnostic, Lint};
 use crate::{
     test_util::{get_standard_library, PrettyString},
     StandardLibrary,
@@ -48,7 +48,7 @@ fn replace_code_range(code: &str, start: usize, end: usize, replacement: &str) -
 /// 1. Applies all disjoint fixes
 /// 1. If a fix is completely contained inside another fix, uses the inner one and discard the outer one
 /// 2. If fixes partially overlap, chooses the fix that starts first and discard the other one
-fn apply_diagnostics_fixes(code: &str, diagnostics: &Vec<&Diagnostic>) -> String {
+fn apply_diagnostics_fixes(code: &str, diagnostics: &[&Diagnostic]) -> String {
     let mut chosen_diagnostics = Vec::new();
 
     let mut candidate = diagnostics[0];
@@ -77,9 +77,9 @@ fn apply_diagnostics_fixes(code: &str, diagnostics: &Vec<&Diagnostic>) -> String
                 let (start, end) = diagnostic.primary_label.range;
                 let new_code = replace_code_range(
                     code.as_str(),
-                    (start as isize + bytes_offset as isize) as usize,
-                    (end as isize + bytes_offset as isize) as usize,
-                    &fixed.as_str(),
+                    (start as isize + bytes_offset) as usize,
+                    (end as isize + bytes_offset) as usize,
+                    fixed.as_str(),
                 );
 
                 bytes_offset += fixed.len() as isize - (end - start) as isize;
@@ -148,25 +148,32 @@ pub fn test_lint_config_with_output<
     let mut fixed_code = lua_source.to_string();
     let mut fixed_diagnostics = diagnostics
         .iter()
-        .filter(|diagnostic| diagnostic.fixed_code.is_some())
+        .filter(|diagnostic| {
+            diagnostic.fixed_code.is_some()
+                && (diagnostic.applicability == Applicability::MachineApplicable
+                    || diagnostic.applicability == Applicability::MaybeIncorrect)
+        })
         .collect::<Vec<_>>();
     let mut lint_results;
 
     // To handle potential conflicts with different lint suggestions, we apply conflicting fixes one at a time.
     // Then we re-evaluate the lints with the new code until there are no more fixes to apply
-    while fixed_diagnostics
-        .iter()
-        .any(|diagnostic| diagnostic.fixed_code.is_some())
-    {
+    while fixed_diagnostics.iter().any(|diagnostic| {
+        diagnostic.fixed_code.is_some()
+            && (diagnostic.applicability == Applicability::MachineApplicable
+                || diagnostic.applicability == Applicability::MaybeIncorrect)
+    }) {
         fixed_code = apply_diagnostics_fixes(fixed_code.as_str(), &fixed_diagnostics);
 
-        let fixed_ast = full_moon::parse(&fixed_code).expect(&format!(
-            "Fixer generated invalid code:\n\
-            ----------------\n\
-            {}\n\
-            ----------------\n",
-            fixed_code
-        ));
+        let fixed_ast = full_moon::parse(&fixed_code).unwrap_or_else(|_| {
+            panic!(
+                "Fixer generated invalid code:\n\
+                ----------------\n\
+                {}\n\
+                ----------------\n",
+                fixed_code
+            )
+        });
         lint_results = lint.pass(
             &fixed_ast,
             &context,

selene-lib/tests/lints/almost_swapped/almost_swapped.fixed.diff

@@ -0,0 +1,16 @@
+ x, y = y, x
+
+ a = b
+ b = a
+
+ t[1], t[2] = t[2], t[1]
+
+ t[1] = t[2]
+ t[2] = t[1]
+
+ foo().a = foo().b
+ foo().b = foo().a
+
+ -- We use a weird hack so this comment might break something, oh no!
+ a = b
+ b = a

selene-lib/tests/lints/almost_swapped/panic.fixed.diff

@@ -0,0 +1 @@
+ foo = "bar"

selene-lib/tests/lints/compare_nan/compare_nan_if.stderr

@@ -4,13 +4,13 @@ error[compare_nan]: comparing things to nan directly is not allowed
 1 │ if x == 0/0 then
   │    ^^^^^^^^
   │
-  = try: `x ~= x` instead
+  = try: `x ~= x`
 
 error[compare_nan]: comparing things to nan directly is not allowed
   ┌─ compare_nan_if.lua:4:4
   │
 4 │ if x ~= 0/0 then
   │    ^^^^^^^^
   │
-  = try: `x == x` instead
+  = try: `x == x`
 

selene-lib/tests/lints/compare_nan/compare_nan_variables.stderr

@@ -4,13 +4,13 @@ error[compare_nan]: comparing things to nan directly is not allowed
 6 │ local _ = x ~= 0/0
   │           ^^^^^^^^
   │
-  = try: `x == x` instead
+  = try: `x == x`
 
 error[compare_nan]: comparing things to nan directly is not allowed
   ┌─ compare_nan_variables.lua:7:11
   │
 7 │ local _ = x == 0/0
   │           ^^^^^^^^
   │
-  = try: `x ~= x` instead
+  = try: `x ~= x`
 

selene-lib/tests/lints/constant_table_comparison/constant_table_comparison.fixed.diff

@@ -0,0 +1,20 @@
+ print(x == { "a", "b", "c" })
+ print({ "a", "b", "c" } == x)
+
+-print(x == {})
+-print({} == x)
+-print(x ~= {})
+-print({} ~= x)
++print(next(x) == nil)
++print(next(x) == nil)
++print(next(x) ~= nil)
++print(next(x) ~= nil)
+
+ print({ "a", "b", "c" } == { "a", "b", "c" })
+ print({ "a", "b", "c" } == {})
+ print({} == {})
+
+ print( -- my cool table
+-	t == {}
++	next(t) == nil
+ )

selene-lib/tests/lints/roblox_incorrect_roact_usage/old_roblox_std.fixed.diff

@@ -0,0 +1 @@
+ Roact.createElement("Frame")

selene-lib/tests/lints/standard_library/any.fixed.diff

@@ -0,0 +1,6 @@
+ print(foo)
+ print(foo.x)
+ print(foo.x.y.z)
+ print(foo:x())
+ foo.x = 1
+ foo.x.y = 2

selene-lib/tests/lints/standard_library/assert.fixed.diff

@@ -0,0 +1,9 @@
+ assert(true, "message")
+ assert(call())
+ assert(call(), "this is ok")
+ assert(...)
+
+ assert(true)
+ assert(true, "message", call())
+ assert(true, "message", ...)
+ assert()

selene-lib/tests/lints/standard_library/bad_call_signatures.fixed.diff

@@ -0,0 +1,20 @@
+ print()
+ print("a", 2, "c")
+ math.sin(3)
+ math.sin(1 + 1)
+ math.random()
+ math.random(1)
+ print "hello"
+ --
+ math.sin("i'd like the sine of three, please")
+ math.floor(3.5, "remember to make sure it's 3 :)")
+ math.sin "pi"
+ --
+ local variable = 3
+ math.sin(variable)
+ math.sin(variable, variable)
+ --
+ math.max(2)
+ math.pi()
+ string.format(-2, "foo")
+ string.format(1 + 1)

selene-lib/tests/lints/standard_library/callable_metatables.fixed.diff

@@ -0,0 +1,2 @@
+ expect.extend({})
+ expect(1).to.be.ok()

selene-lib/tests/lints/standard_library/complex.fixed.diff

@@ -0,0 +1,3 @@
+ print(require("foo").bar)
+ print(coroutine.wrap(print)())
+ getmetatable({}).__index = function() end

selene-lib/tests/lints/standard_library/constants.fixed.diff

@@ -0,0 +1,9 @@
+ collectgarbage("count")
+ collectgarbage "count"
+
+ collectgarbage("doge")
+ collectgarbage(1)
+
+ collectgarbage("coun" .. "t")
+
+ collectgarbage()

selene-lib/tests/lints/standard_library/if_expressions.fixed.diff

@@ -0,0 +1,5 @@
+ math.floor(if x then 1 else 2)
+ math.floor(if x then "a" else "b")
+
+ -- TODO: Let's try to capture this, right now the entire expression just resolves to no type
+ math.floor(if x then 1 else "a")

selene-lib/tests/lints/standard_library/lua52.fixed.diff

@@ -0,0 +1,2 @@
+ table.unpack({})
+ setfenv() -- This would error if it was defined, since setfenv requires an argument

selene-lib/tests/lints/standard_library/math_on_types.fixed.diff

@@ -0,0 +1,2 @@
+ usesType(x * 0.5)
+ usesType(0.5 * x)

selene-lib/tests/lints/standard_library/method_call.fixed.diff

@@ -0,0 +1,4 @@
+ foo:bar(1)
+ foo:bar("a")
+ foo.bar()
+ foo:baz()

selene-lib/tests/lints/standard_library/method_call.stderr

@@ -10,7 +10,7 @@ error[incorrect_standard_library_use]: standard library function `foo.bar` is a
 3 │ foo.bar()
   │ ^^^^^^^^^
   │
-  = try: foo:bar(...)
+  = try: `foo:bar(...)`
 
 error[incorrect_standard_library_use]: standard library global `foo` does not contain the field `baz`
   ┌─ method_call.lua:4:1

selene-lib/tests/lints/standard_library/required.fixed.diff

@@ -0,0 +1,11 @@
+ -- ipairs requires an argument--this should fail.
+ ipairs()
+
+ -- setmetatable requires a first argument, but not a second one. this should still fail.
+ setmetatable()
+
+ -- This, however, should not.
+ setmetatable({})
+
+ -- nil counts as filling in an unrequired argument.
+ setmetatable({}, nil)