🍒 🐛 Try to fit annotations into on-demand imports for member che…

[jmle]

…cking (#208)

• Try to fit annotations into on-demand imports for member checking
• Remove unneeded document
• Address automatic PR review comments

⚠️ DNM until release-0.9 is cut

Summary by CodeRabbit

• Bug Fixes
  • Enhanced annotation resolution to evaluate multiple fully-qualified name candidates for more accurate matching.
  • Added support for nested annotations within member-value pairs so inner annotations are properly recognized.
  • Improved import handling to better resolve on-demand and single-type annotation names when determining candidates.

[coderabbitai]

📝 Walkthrough

Walkthrough

The refactoring replaces single fully-qualified name resolution in annotation queries with a candidate-based approach. The implementation now iterates over multiple FQN candidates per annotation, adds support for nested annotations and varied member-value scenarios, and expands import-collection logic.

Changes

Cohort / File(s)	Summary
Annotation Query Refactoring java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java	Replaced single getFQN() with getFQNCandidates() returning multiple FQN candidates. Updated matching to iterate candidates and handle nested annotations and memberValuePairs (including arrays of annotations). tryToGetImports() now returns a List of imports with safer error handling. Added ArrayList import.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibbled names until I found a few,
Candidates sprouted where one seed once grew.
Nested petals unfurl in import light,
Now annotations hop and take their flight! 🥕✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title uses emoji decoration and is truncated with ellipsis, making it vague and difficult to understand the actual change being made.	Remove emoji decorations and complete the title to clearly describe the main change, such as 'Support on-demand imports for annotation resolution in member checking'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[shawn-hurley]

If this is a cherry-pick, should we be release-0.9?

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java`:
- Around line 190-199: The getFQNCandidates(IAnnotation) method currently
returns no candidates when an annotation name is unqualified and not imported;
update getFQNCandidates to append implicit-scope fallback candidates by adding
the annotation name qualified with the annotation's enclosing package and with
"java.lang" (i.e., current package + "." + name and "java.lang." + name) before
returning; use tryToGetImports(annotation) and the annotation's owning
ICompilationUnit or type to determine the current package, and ensure these
fallbacks are only added when imports did not yield matches and the name
contains no dot (unqualified).
- Around line 51-55: In WithAnnotationQuery, the loop that iterates
getFQNCandidates(annotation) should not return doElementsMatch(...) directly
because a false result may hide later valid candidates; change the logic in the
block that calls getAnnotationQuery().matchesAnnotation(fqn) so that you call
doElementsMatch((Annotation) annotation) and if it returns true return true,
otherwise continue checking remaining fqn candidates and remaining annotations,
and only return false after all candidates/annotations are exhausted; apply the
same fix pattern to the other similar blocks (the ones around the occurrences
currently at lines 64-66 and 72-74) so each match only short-circuits on a true
result.
- Around line 203-206: The current filter uses endsWith(name) and can match
suffixes incorrectly; change the import matching in WithAnnotationQuery so it
only considers single-type imports (skip on-demand imports like "pkg.*") and
compares the simple name exactly: for each IImportDeclaration use its
getElementName(), ignore names that end with ".*", compute the simple name as
the substring after the last '.' (or the whole string if no dot) and require
simpleName.equals(name) before mapping to getElementName() and adding to
candidates.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1fa7e7 and 2d4ee54.

📒 Files selected for processing (1)

• java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java

[shawn-hurley]

super silly question but could I import xxx.yy.zzz.* that as ccc.AAA annotation and just reference it as (@ccc.AAA)?

If that makes no sense I get it, just wondering if that is a usecase that the java language supports that we have to handle.

[jmle]

Sorry mate I don't understand, do you mean using something like an as keyword? That doesn't exist in Java, but you can import a.b.c.D; and then also use @a.b.c.D in the code (although it wouldn't make much sense)

[shawn-hurley]

Just wondering about an edge case, generally I think this is better behavior.

Is there any way to add a test for this in the integration tests?

[jmle]

If this is a cherry-pick, should we be release-0.9?

Yep totally

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java (1)

190-220: ⚠️ Potential issue | 🟠 Major

Add implicit-scope FQN fallbacks for unqualified annotation names.

When an annotation is unqualified and imports don’t resolve it, this returns no candidates; valid annotations from the current package or java.lang (e.g., @Deprecated) are missed.

Suggested fix

 import org.eclipse.jdt.core.IAnnotation;
+import org.eclipse.jdt.core.IJavaElement;
 import org.eclipse.jdt.core.IImportDeclaration;

         // On-demand: one candidate per pkg.*
         for (IImportDeclaration imp : imports) {
             String elementName = imp.getElementName();
             if (elementName != null && elementName.endsWith(".*")) {
                 String prefix = elementName.substring(0, elementName.length() - 2);
                 if (!prefix.isEmpty()) {
                     candidates.add(prefix + "." + name);
                 }
             }
         }
+        // Implicit scope fallback for unqualified names
+        if (candidates.isEmpty()) {
+            IJavaElement pkg = annotation.getAncestor(IJavaElement.PACKAGE_FRAGMENT);
+            if (pkg != null && pkg.getElementName() != null && !pkg.getElementName().isEmpty()) {
+                candidates.add(pkg.getElementName() + "." + name);
+            }
+            candidates.add("java.lang." + name);
+        }
         return candidates;
     }

#!/bin/bash
set -euo pipefail
FILE="java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java"

echo "Inspect getFQNCandidates for implicit fallback handling:"
rg -n -C4 'getFQNCandidates|java\.lang|PACKAGE_FRAGMENT|getAncestor\(' "$FILE"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java`
around lines 190 - 220, getFQNCandidates currently only considers explicit
imports and on-demand imports, so unqualified annotation names from the current
package or java.lang are missed; update getFQNCandidates (which receives
IAnnotation and calls tryToGetImports) to, after processing imports and
on-demand imports, add fallback candidates for the enclosing package and for
java.lang by resolving the annotation's compilation unit or parent type to
obtain its package (use the same ancestor lookup you use elsewhere) and then add
package + "." + name and "java.lang." + name when not already present in
candidates.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java`:
- Around line 52-53: The code is performing unsafe casts to the internal
org.eclipse.jdt.internal.core.Annotation; change the doElementsMatch(Annotation)
signature in WithAnnotationQuery to doElementsMatch(IAnnotation) (use
org.eclipse.jdt.core.IAnnotation), update all callers (where
getAnnotationQuery().matchesAnnotation(fqn) && doElementsMatch((Annotation)
annotation) and the other two similar sites) to pass the annotation variable as
an IAnnotation (remove casts) and replace any instanceof Annotation checks with
instanceof IAnnotation; update imports to use IAnnotation and adjust any
internal-Annotation-specific method calls to the public IAnnotation API so no
internal classes are referenced or cast.

---

Duplicate comments:
In
`@java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java`:
- Around line 190-220: getFQNCandidates currently only considers explicit
imports and on-demand imports, so unqualified annotation names from the current
package or java.lang are missed; update getFQNCandidates (which receives
IAnnotation and calls tryToGetImports) to, after processing imports and
on-demand imports, add fallback candidates for the enclosing package and for
java.lang by resolving the annotation's compilation unit or parent type to
obtain its package (use the same ancestor lookup you use elsewhere) and then add
package + "." + name and "java.lang." + name when not already present in
candidates.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fde627aa-6117-4891-b48a-27246232325e

📥 Commits

Reviewing files that changed from the base of the PR and between 2d4ee54 and 44f9e5e.

📒 Files selected for processing (1)

• java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

FILE="java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java"

echo "=== File existence and size ==="
if [ -f "$FILE" ]; then
  wc -l "$FILE"
  echo "File exists"
else
  echo "File not found"
  exit 1
fi

echo
echo "=== Lines 50-75 (context around mentioned lines) ==="
sed -n '50,75p' "$FILE"

echo
echo "=== All imports ==="
head -50 "$FILE" | grep -E "^import"

echo
echo "=== Method signatures for doElementsMatch ==="
rg -n 'doElementsMatch' "$FILE" -B2 -A5

Repository: konveyor/java-analyzer-bundle

Length of output: 4926

---

Remove unsafe casts to internal Annotation class; use IAnnotation interface instead.

Direct casts to the internal org.eclipse.jdt.internal.core.Annotation can throw ClassCastException when JDT returns an IAnnotation implementation that is not from the internal package. Lines 53, 62, and 70 all perform such casts; line 70 is particularly problematic as it checks instanceof IAnnotation but still unsafely casts to the internal Annotation class.

Update the doElementsMatch() method signature to accept IAnnotation and remove all unsafe casts:

Proposed changes

-    private boolean doElementsMatch(Annotation annotation) throws JavaModelException {
+    private boolean doElementsMatch(IAnnotation annotation) throws JavaModelException {

-                            return doElementsMatch((Annotation) annotation);
+                            return doElementsMatch(annotation);

-                                    Annotation innerAnnotation = (Annotation) objs[i];
+                                    if (!(objs[i] instanceof IAnnotation)) {
+                                        continue;
+                                    }
+                                    IAnnotation innerAnnotation = (IAnnotation) objs[i];

-                                Annotation innerAnnotation = (Annotation) member.getValue();
+                                IAnnotation innerAnnotation = (IAnnotation) member.getValue();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@java-analyzer-bundle.core/src/main/java/io/konveyor/tackle/core/internal/symbol/WithAnnotationQuery.java`
around lines 52 - 53, The code is performing unsafe casts to the internal
org.eclipse.jdt.internal.core.Annotation; change the doElementsMatch(Annotation)
signature in WithAnnotationQuery to doElementsMatch(IAnnotation) (use
org.eclipse.jdt.core.IAnnotation), update all callers (where
getAnnotationQuery().matchesAnnotation(fqn) && doElementsMatch((Annotation)
annotation) and the other two similar sites) to pass the annotation variable as
an IAnnotation (remove casts) and replace any instanceof Annotation checks with
instanceof IAnnotation; update imports to use IAnnotation and adjust any
internal-Annotation-specific method calls to the public IAnnotation API so no
internal classes are referenced or cast.