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

Unused function parameter detection fails in cases it shouldn't #10

Open
kshyatt opened this issue Apr 26, 2024 · 3 comments
Open

Unused function parameter detection fails in cases it shouldn't #10

kshyatt opened this issue Apr 26, 2024 · 3 comments

Comments

@kshyatt
Copy link

kshyatt commented Apr 26, 2024

function a(b::Vector{Int})
    return [c^2 for c in b]
end

will cause semgrep to error with the current rules saying b is unused.

Similarly, Val types used to dispatch (or any ::Type{SomeType}) arguments cause unused parameter errors:

function a(b::Vector{Int}, ::Val{true})
    return b
end

this will generate an error saying ::Val{true} is unused.

@kshyatt
Copy link
Author

kshyatt commented Jun 28, 2024

Wrapping an argument in @nospecialize also causes semgrep to whine

@iuliadmtru
Copy link
Collaborator

In the first example, the problem is caused by how Semgrep parses the list comprehension. If you write only the comprehension in a file ([c^2 for c in b]), say test.jl, and run semgrep scan --dump-ast -l julia test.jl you'll see this AST:

Pr(
  [ExprStmt(
     Comprehension(Array,
       (Call(IdSpecial((Op(Pow), ())),
          [Arg(
             N(
               Id(("c", ()),
                 {id_info_id=1; id_flags=Ref(0); id_resolved=Ref(None);
                  id_type=Ref(None); id_svalue=Ref(None); })));
           Arg(L(Int((Some(2), ()))))]),
        [])), ())])

There is no information about the iteration variable or the iterator. So there's no way to know that b, the iterator in this case, is unused. It might not be the only example of this. Maybe you found more in the meantime? I could file an issue on Semgrep with more general examples.

The second example (with ::Val{true}) now works for me with Semgrep 1.81.0 (the latest is 1.84).

And the last one is again a parsing problem. Semgrep doesn't think the syntax is correct. I'm not sure if the problem comes from the Julia tree-sitter or from the generic AST generated by Semgrep.

@iuliadmtru
Copy link
Collaborator

Updates:

  • The anonymous arguments case does not work, actually. I can think of a fix for this (something like this), but this issue needs to be solved for that.
  • I filed an issue for the @nospecialize case.
  • List comprehensions are now parsed correctly! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants