Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving method call error #32

Merged
merged 2 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
17 changes: 16 additions & 1 deletion src/linting/checks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
41 changes: 24 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,9 +738,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

Expand All @@ -755,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 @@ -813,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 @@ -854,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 @@ -1027,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 @@ -1167,3 +1166,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
16 changes: 11 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 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
Expand Down Expand Up @@ -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("""
Expand All @@ -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("""
Expand Down Expand Up @@ -1040,7 +1043,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)
Expand All @@ -1050,7 +1054,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
Expand Down
Loading