konveyor / analyzer-lsp

Add-on that is focused on providing analysis based on the Language Server Protocol.
Apache License 2.0
12 stars 45 forks source link

[BUG] Regression with location: ANNOTATION in java provider, not seeing rule match #620

Open jwmatthews opened 5 months ago

jwmatthews commented 5 months ago

Is there an existing issue for this?

Konveyor version

Latest as of June 13 2024

Priority

Undefined (Default)

Current Behavior

We noticed a regression where a custom rule that used to match no longer does. Below is the information

Source file, using an '@Remote' src/main/java/org/jboss/as/quickstarts/ejb/remote/stateful/CounterBean.java

package org.jboss.as.quickstarts.ejb.remote.stateful;

import javax.ejb.Remote;
import javax.ejb.Stateful;

/**
 * @author Jaikiran Pai
 */
@Stateful
@Remote(RemoteCounter.class)
public class CounterBean implements RemoteCounter {

    private int count = 0;

    @Override
    public void increment() {
        this.count++;
    }

    @Override
    public void decrement() {
        this.count--;
    }

    @Override
    public int getCount() {
        return this.count;
    }
}

Link to custom rule

  ruleID: remote-ejb-to-quarkus-00000
  when:
    or:
    - java.referenced:
        location: ANNOTATION
        pattern: javax.ejb.Remote
    - java.referenced:
        location: ANNOTATION
        pattern: jakarta.ejb.Remote

Link to full output.yaml

- name: kai/quarkus
  description: Quarkus focused rules to help migrate from Java EE
  unmatched:
  - remote-ejb-to-quarkus-00000

We originally suspected this may be related to the sample app having 2 different modules in it for client and server-side. I ran Kantra against those individual modules, i.e. ran against the 'server-side' module, and I still did not see the custom rule for the Remote annotation match.

At this point, I think the issue may be related to the ANNOTATION location.

Expected Behavior

Assume we'd have a match like this:

[](- name: kai/quarkus description: Quarkus focused rules to help migrate from Java EE violations: remote-ejb-to-quarkus-00000: description: Remote EJBs are not supported in Quarkus category: mandatory labels:

How Reproducible

Always (Default)

Steps To Reproduce

I have a short reproducer here: https://github.com/jwmatthews/issue_reproducers/tree/main/kantra/ejb_remote

  1. git clone https://github.com/konveyor-ecosystem/ejb-remote
  2. Run: ./analyze.sh You can find the referenced custom rule at: https://github.com/jwmatthews/issue_reproducers/blob/main/kantra/ejb_remote/custom_rules/03-remote-ejb-to-quarkus.windup.yaml

Environment

Running on MacOS arm64 

./kantra version
version: latest
SHA: a89ffa3bce466bb46ecc658a2b07e7d2bec69af1
image: quay.io/konveyor/kantra

Anything else?

This appears to be a regression as we saw matches for this rule + sample application about 3 months ago, here is an example we checked in from an older notebook example.

https://github.com/konveyor-ecosystem/kai/blob/main/notebooks/ejb_remote/analysis_report/ejb-remote/output.yaml#L1278

This is related to an issue we are tracking in kai: https://github.com/konveyor-ecosystem/kai/issues/204

There is another open issue on java.referenced location ANNOTATION, don't think it's related, but will link for awareness: https://github.com/konveyor/analyzer-lsp/issues/508

konveyor-ci-bot[bot] commented 5 months ago

This issue is currently awaiting triage. If contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance. The triage/accepted label can be added by org members.

shawn-hurley commented 3 months ago

When running this locally, I notice that the search scope for the project is empty, and that is why we are getting zero results.

At this time, I can only think of two reasons:

  1. I typo'ed the test environment
  2. That the unresolvable POM is causing jdt.LS to not be able to create any search scopes.

I will continue to work on this, as source only mode should be able to work even with this broken pom file.

shawn-hurley commented 3 months ago

I am postive that this is currently the more correct behavior, and the reasoning is this.

Currently this application, does not compile, and more importantly does not have any references to either Jakarta or Javax.

I think what is happening, is becuas the language server does not have these types, when we are asking for them, it is unable to find any reference for them. I tried to get this POM in a place that was usable, but was unable to.

My primary concern is two fold:

  1. If we are able to find these, it means that we are finding things that the language server is just guessing at, we did a lot of work to make this more stable in the 0.5 timeframe and I worry that going back will cause similar problems
  2. That folks have built up a system of rules based on the old behavior that now doesn't work, and we are breaking them.

I don't have a good way forward, as both have huge pitfalls in the overall health of the results and to users.

@dymurray @jwmatthews @fabianvf @pranavgaikwad @jmle Looking for help and guidance on what our next steps should be.