Merge pull request #32 from RelationalAI/improving-method-call-error

Improving method call error

src/interface.jl

@@ -172,11 +172,17 @@ function filter_and_print_hint(hint_as_string::String, io::IO=stdout, filters::V
     # +1 is because CSTParser gives offset starting at 0.
     offset = Base.parse(Int64, offset_as_string) + 1
 
-    line_number, column, annotation_line = convert_offset_to_line_from_filename(offset, filename)
-
-    if isnothing(annotation_line)
-        print_hint(formatter, io, "Line $(line_number), column $(column):", hint_as_string)
-        return true
+    # Remove the offset from the result. No need for this.
+    cleaned_hint = replace(hint_as_string, (" at offset $offset_as_string of" => ""))
+    try
+        line_number, column, annotation_line = convert_offset_to_line_from_filename(offset, filename)
+
+        if isnothing(annotation_line)
+            print_hint(formatter, io, "Line $(line_number), column $(column):", cleaned_hint)
+            return true
+        end
+    catch
+        @error "Cannot retreive offset=$offset in file $filename"
     end
     return false
 end

src/linting/checks.jl

@@ -113,6 +113,10 @@ LintOptions(options::Vararg{Union{Bool,Nothing},length(default_options)}) =
     LintOptions(something.(options, default_options)...)
 
 
+function fetch_value(x::SymbolServer.VarRef, tag::Symbol)
+    return x.name
+end
+
 function fetch_value(x::EXPR, tag::Symbol)
     if headof(x) == tag
         return x.val
@@ -341,7 +345,18 @@ function check_call(x, env::ExternalEnv)
         tls = retrieve_toplevel_scope(x)
         tls === nothing && return @warn "Couldn't get top-level scope." # General check, this means something has gone wrong.
         func_ref === nothing && return
-        !sig_match_any(func_ref, x, call_counts, tls, env) && seterror!(x, IncorrectCallArgs)
+        if !sig_match_any(func_ref, x, call_counts, tls, env)
+            if func_ref.name isa SymbolServer.VarRef &&
+                !isnothing(func_ref.name.parent) &&
+                func_ref.name.parent.name == :Base &&
+                !isnothing(func_ref.name.name)
+
+                func_ref.name.name in [:copy] && return
+            end
+            function_name = fetch_value(func_ref.name, :IDENTIFIER)
+            function_name in ["delete!", "copy", "copy!", "write", "hash", "iterate"] && return
+            seterror!(x, "Possible method call error: $(function_name).")
+        end
     end
 end
 

test/rai_rules_tests.jl

@@ -644,8 +644,8 @@ end
 
 @testset "Should be filtered" begin
     filters = StaticLint.LintCodes[StaticLint.MissingReference, StaticLint.IncorrectCallArgs]
-    hint_as_string1 = "Missing reference at offset 24104 of /Users/alexandrebergel/Documents/RAI/raicode11/src/DataExporter/export_csv.jl"
-    hint_as_string2 = "Line 254, column 19: Possible method call error. at offset 8430 of /Users/alexandrebergel/Documents/RAI/raicode11/src/Compiler/Front/problems.jl"
+    hint_as_string1 = "Missing reference /Users/alexandrebergel/Documents/RAI/raicode11/src/DataExporter/export_csv.jl"
+    hint_as_string2 = "Line 254, column 19: Possible method call error: foo /Users/alexandrebergel/Documents/RAI/raicode11/src/Compiler/Front/problems.jl"
     @test should_be_filtered(hint_as_string1, filters)
     @test !should_be_filtered(hint_as_string2, filters)
 
@@ -687,8 +687,8 @@ end
 
         expected = r"""
             ---------- \H+
-            Line 1, column 11: `Threads.nthreads\(\)` should not be used in a constant variable\. at offset 10 of \H+
-            Line 1, column 11: Missing reference at offset 10 of \H+
+            Line 1, column 11: `Threads.nthreads\(\)` should not be used in a constant variable\. \H+
+            Line 1, column 11: Missing reference \H+
             2 potential threats are found
             ----------
             """
@@ -702,7 +702,7 @@ end
 
         expected = r"""
             ---------- \H+
-            Line 1, column 11: `Threads.nthreads\(\)` should not be used in a constant variable\. at offset 10 of \H+
+            Line 1, column 11: `Threads.nthreads\(\)` should not be used in a constant variable\. \H+
             1 potential threat is found
             ----------
             """
@@ -715,8 +715,8 @@ end
         result = String(take!(io))
 
         expected = r"""
-             - \*\*Line 1, column 11:\*\* `Threads.nthreads\(\)` should not be used in a constant variable\. at offset 10 of \H+
-             - \*\*Line 1, column 11:\*\* Missing reference at offset 10 of \H+
+             - \*\*Line 1, column 11:\*\* `Threads.nthreads\(\)` should not be used in a constant variable\. \H+
+             - \*\*Line 1, column 11:\*\* Missing reference \H+
             """
         @test !isnothing(match(expected, result))
     end
@@ -727,7 +727,7 @@ end
         result = String(take!(io))
 
         expected = r"""
-             - \*\*Line 1, column 11:\*\* `Threads.nthreads\(\)` should not be used in a constant variable\. at offset 10 of \H+
+             - \*\*Line 1, column 11:\*\* `Threads.nthreads\(\)` should not be used in a constant variable\. \H+
             """
         @test !isnothing(match(expected, result))
     end
@@ -745,9 +745,8 @@ end
         result = String(take!(io))
 
         expected = r"""
-             - \*\*\[Line 1, column 11:\]\(https://github\.com/RelationalAI/raicode/blob/axb-example-with-lint-errors/\H+/src/Compiler/tmp_julia_file\.jl#L1\)\*\* `Threads.nthreads\(\)` should not be used in a constant variable\. at offset 10 of \H+
+             - \*\*\[Line 1, column 11:\]\(https://github\.com/RelationalAI/raicode/blob/axb-example-with-lint-errors/\H+/src/Compiler/tmp_julia_file\.jl#L1\)\*\* `Threads.nthreads\(\)` should not be used in a constant variable\. \H+
             """
-        println("DEBUG: $result")
         @test !isnothing(match(expected, result))
     end
 
@@ -762,7 +761,7 @@ end
             directory="src/Compiler/")
         result = String(take!(io))
         expected = r"""
-             - \*\*\[Line 1, column 11:\]\(https://github\.com/RelationalAI/raicode/blob/axb-example-with-lint-errors/\H+/src/Compiler/tmp_julia_file\.jl#L1\)\*\* `Threads.nthreads\(\)` should not be used in a constant variable\. at offset 10 of \H+
+             - \*\*\[Line 1, column 11:\]\(https://github\.com/RelationalAI/raicode/blob/axb-example-with-lint-errors/\H+/src/Compiler/tmp_julia_file\.jl#L1\)\*\* `Threads.nthreads\(\)` should not be used in a constant variable\. \H+
             """
         @test !isnothing(match(expected, result))
     end
@@ -820,8 +819,8 @@ end
                     result = String(take!(str))
 
                     expected = r"""
-                         - \*\*Line 2, column 3:\*\* Macro `@spawn` should be used instead of `@async`\. at offset 15 of \H+
-                         - \*\*Line 2, column 3:\*\* Macro `@spawn` should be used instead of `@async`\. at offset 15 of \H+
+                         - \*\*Line 2, column 3:\*\* Macro `@spawn` should be used instead of `@async`\. \H+
+                         - \*\*Line 2, column 3:\*\* Macro `@spawn` should be used instead of `@async`\. \H+
                         """
                     result_matching = !isnothing(match(expected, result))
                 end
@@ -861,9 +860,9 @@ end
                         ## Static code analyzer report
                         \*\*Output of the \[StaticLint\.jl code analyzer\]\(https://github\.com/RelationalAI/StaticLint\.jl\)\*\*
                         Report creation time \(UTC\): \H+
-                         - \*\*Line 2, column 3:\*\* Macro `@spawn` should be used instead of `@async`\. at offset 15 of \H+
-                         - \*\*Line 2, column 3:\*\* `finalizer\(_,_\)` should not be used\. at offset 15 of \H+
-                         - \*\*Line 2, column 25:\*\* Variable has been assigned but not used\. If you want to keep this variable unused then prefix it with `_`. at offset 37 of \H+
+                         - \*\*Line 2, column 3:\*\* Macro `@spawn` should be used instead of `@async`\. \H+
+                         - \*\*Line 2, column 3:\*\* `finalizer\(_,_\)` should not be used\. \H+
+                         - \*\*Line 2, column 25:\*\* Variable has been assigned but not used\. If you want to keep this variable unused then prefix it with `_`. \H+
                         🚨\*\*In total, 3 potential threats are found over 2 Julia files\*\*🚨
                         """
                     result_matching = !isnothing(match(expected, result))
@@ -1034,7 +1033,7 @@ end
                 end
 
                 expected = r"""
-                     - \*\*\[Line 2, column 3:\]\(https://github\.com/RelationalAI/raicode/blob/axb-foo-bar/folders/\H+/foo\.jl#L2\)\*\* Macro `@spawn` should be used instead of `@async`. at offset 15 of \H+
+                     - \*\*\[Line 2, column 3:\]\(https://github\.com/RelationalAI/raicode/blob/axb-foo-bar/folders/\H+/foo\.jl#L2\)\*\* Macro `@spawn` should be used instead of `@async`. \H+
                     """
                 result_matching = !isnothing(match(expected, result))
             end
@@ -1174,3 +1173,11 @@ end
            end
            """)
 end
+
+@testset "IncorrectCallArgs" begin
+    source = """
+               f(x) = 1
+               f(1, 2)
+               """
+    @test lint_test(source, "Line 2, column 1: Possible method call error: f.")
+end

test/runtests.jl

@@ -292,7 +292,7 @@ f(arg) = arg
             let cst = parse_and_pass("""
             sin(1,2,3)
             """)
-                @test errorof(cst.args[1]) === StaticLint.IncorrectCallArgs
+                @test startswith(errorof(cst.args[1]), "Possible method call error")
             end
             let cst = parse_and_pass("""
                 for i in length(1) end
@@ -522,7 +522,9 @@ f(arg) = arg
         sin(1,2)
         """)
             @test StaticLint.errorof(cst.args[1]) === nothing
-            @test StaticLint.errorof(cst.args[2]) == StaticLint.IncorrectCallArgs
+            # @test StaticLint.errorof(cst.args[2]) == StaticLint.IncorrectCallArgs
+            @test startswith(errorof(cst[2]), "Possible method call error")
+
         end
 
         let cst = parse_and_pass("""
@@ -539,7 +541,8 @@ f(arg) = arg
         f(x) = 1
         f(1, 2)
         """)
-            @test StaticLint.errorof(cst.args[2]) === StaticLint.IncorrectCallArgs
+            # @test StaticLint.errorof(cst.args[2]) === StaticLint.IncorrectCallArgs
+            @test startswith(errorof(cst[2]), "Possible method call error")
         end
 
         let cst = parse_and_pass("""
@@ -1041,7 +1044,8 @@ f(arg) = arg
         """)
                 @test StaticLint.scopehasbinding(scopeof(cst), "adf")
                 @test !StaticLint.scopehasbinding(scopeof(cst.args[1]), "adf")
-                @test errorof(cst.args[2]) === StaticLint.IncorrectCallArgs
+                # @test errorof(cst.args[2]) === StaticLint.IncorrectCallArgs
+                @test startswith(errorof(cst[2]), "Possible method call error")
             end
             let cst = parse_and_pass("""
         for name in (:sdf, :asdf)
@@ -1051,7 +1055,9 @@ f(arg) = arg
         """)
                 @test StaticLint.scopehasbinding(scopeof(cst), "sdf")
                 @test !StaticLint.scopehasbinding(scopeof(cst.args[1]), "asdf")
-                @test errorof(cst[2]) === StaticLint.IncorrectCallArgs
+                # @test errorof(cst[2]) === StaticLint.IncorrectCallArgs
+                @test startswith(errorof(cst[2]), "Possible method call error")
+
             end
         end
     end