semgrep.dev Java Cannot match annotation to partial class

[ben-elttam]

Now that Support for partial class header and method header for java #1830 has been merged I expected this to rule definition to work.
https://semgrep.dev/s/l01j?stepByStep=1

I expect:

@RequestMapping(...)
class $CLASS

To match:

@RestController
@RequestMapping("/api/login")
public class LoginController {

But it needs the ellipse on the class, which match/output the whole class, when I just want the class name.

Works but too verbose:

@RequestMapping(...)
class $CLASS {...}

Summarised as a table:

What	Annotation	Class (partial)	class (full)
Annotation	✔️ (contextless)	❌	✔️
Class (partial)	❌	✔️	N/A
Class (full)	✔️	N/A	✔️

Detailed Explanation:

I can match:

• Just the annotation (no context)
• Just the class (partial)
• The full class
• The annotation on the full class

But I cannot match the annotation to the (partial) class.

[underyx]

That table is so useful, thank you! @aryx since you made the two relevant fixes I assume you'll have a look at this issue as well?

[milo-minderbinder]

@aryx it might be a PEBKAC issue, but I think this fix only partially works. I can see the test case you added in #1883 works and can verify that in the live editor, but if I try to run @ben-elttam's example it fails. After some attempts at debugging where/why it fails, I've noticed that it seems to be due to the imports at the top of @ben-elttam 's example code -- specifically, it seems like there's some sort of greedy matching going on which hits the RequestMapping in the import statement and then fails (I'm guessing because the next token/character is a ; and not a parenthesis or class construct, maybe?). To illustrate, if I just change @ben-elttam 's example slightly by inserting an "s" at the end of RequestMapping in the import statement line, then the test case works as expected and matches the annotation on the class as expected: https://semgrep.dev/s/NxkN/?stepByStep=1

EDIT: actually, on further review, maybe the behavior above is expected and it was PEBKAC after all... I see if you edit the test case by specifying the fully qualified class name for the annotation, then it will match as expected and not fail due to the import statement: https://semgrep.dev/s/kWEA/?stepByStep=1

@aryx can you confirm which is the intended behavior? If it's the latter, I might submit a PR with a documentation update and clarifying example so others don't have the some confusion as I had

[ben-elttam]

What I’ve found is that if you import the full symbol, then the pattern
needs to be fully qualified to match the annotation.

I write these rules with pattern-either to handle it.
https://semgrep.dev/s/7gqQ/?stepByStep=1

[aryx]

We used to force to use the fully qualified name in the pattern, but I think it's confusing for people, so maybe we should
relax that constraint and let @RequestMapping in a pattern to match the code.

[aryx]

I've created another task to track the name resolution issue.

[aryx]

See #2029