Skip to content

Commit

Permalink
Improved method call error.
Browse files Browse the repository at this point in the history
  • Loading branch information
bergel committed Mar 11, 2024
1 parent ddce961 commit 548103b
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 32 deletions.
16 changes: 11 additions & 5 deletions src/interface.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 10 additions & 5 deletions src/linting/checks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,16 @@ function check_call(x, env::ExternalEnv)
tls === nothing && return @warn "Couldn't get top-level scope." # General check, this means something has gone wrong.
func_ref === nothing && return
if !sig_match_any(func_ref, x, call_counts, tls, env)
# isdefined(Main, :Infiltrator) && Main.infiltrate(@__MODULE__, Base.@locals, @__FILE__, @__LINE__)
# if func_ref.name isa SymbolServer.VarRef
# isdefined(Main, :Infiltrator) && Main.infiltrate(@__MODULE__, Base.@locals, @__FILE__, @__LINE__)
# end
seterror!(x, "Possible method call error. Call of: $(fetch_value(func_ref.name, :IDENTIFIER)).")
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
Expand Down
34 changes: 17 additions & 17 deletions test/rai_rules_tests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,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)

Expand Down Expand Up @@ -680,8 +680,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
----------
"""
Expand All @@ -695,7 +695,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
----------
"""
Expand All @@ -708,8 +708,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
Expand All @@ -720,7 +720,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
Expand All @@ -738,7 +738,7 @@ 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+
"""
@test !isnothing(match(expected, result))
end
Expand All @@ -754,7 +754,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
Expand Down Expand Up @@ -812,8 +812,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
Expand Down Expand Up @@ -853,9 +853,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))
Expand Down Expand Up @@ -1026,7 +1026,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
Expand Down Expand Up @@ -1172,5 +1172,5 @@ end
f(x) = 1
f(1, 2)
"""
@test lint_test(source, "Line 2, column 1: Possible method call error. Call of: f.")
@test lint_test(source, "Line 2, column 1: Possible method call error: f.")
end
10 changes: 5 additions & 5 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ f(arg) = arg
let cst = parse_and_pass("""
sin(1,2,3)
""")
@test startswith(errorof(cst.args[1]), "Possible method call error.")
@test startswith(errorof(cst.args[1]), "Possible method call error")
end
let cst = parse_and_pass("""
for i in length(1) end
Expand Down Expand Up @@ -523,7 +523,7 @@ f(arg) = arg
""")
@test StaticLint.errorof(cst.args[1]) === nothing
# @test StaticLint.errorof(cst.args[2]) == StaticLint.IncorrectCallArgs
@test startswith(errorof(cst[2]), "Possible method call error.")
@test startswith(errorof(cst[2]), "Possible method call error")

end

Expand All @@ -542,7 +542,7 @@ f(arg) = arg
f(1, 2)
""")
# @test StaticLint.errorof(cst.args[2]) === StaticLint.IncorrectCallArgs
@test startswith(errorof(cst[2]), "Possible method call error.")
@test startswith(errorof(cst[2]), "Possible method call error")
end

let cst = parse_and_pass("""
Expand Down Expand Up @@ -1044,7 +1044,7 @@ 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 startswith(errorof(cst[2]), "Possible method call error.")
@test startswith(errorof(cst[2]), "Possible method call error")
end
let cst = parse_and_pass("""
for name in (:sdf, :asdf)
Expand All @@ -1055,7 +1055,7 @@ f(arg) = arg
@test StaticLint.scopehasbinding(scopeof(cst), "sdf")
@test !StaticLint.scopehasbinding(scopeof(cst.args[1]), "asdf")
# @test errorof(cst[2]) === StaticLint.IncorrectCallArgs
@test startswith(errorof(cst[2]), "Possible method call error.")
@test startswith(errorof(cst[2]), "Possible method call error")

end
end
Expand Down

0 comments on commit 548103b

Please sign in to comment.