DoubleJavacVisitor walks two javac ASTs in lockstep

[avenger2597]

This PR adds the initial DoubleJavacVisitor that walks two javac ASTs in lockstep. It includes the basic traversal logic and helpers needed to keep the two trees aligned, similar to what the existing JavaParser-based visitor does.

This is infrastructure-only and sets up later work to move ajava processing from JavaParser to javac.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new public abstract class DoubleJavacVisitor that traverses two Javac ASTs in lockstep. Provides a protected constructor, a protected abstract defaultAction(Tree, Tree) hook, a central scan entry point with structural and null checks, and helper methods scanExpr and scanList. Supplies paired visitXyz overrides that call defaultAction and recursively traverse children, throwing BugInCF on desynchronization. Type-use-level annotations are ignored when pairing. Also adds "Ankit Yadav" to docs/manual/contributors.tex.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a80dcf and c9487d2.

📒 Files selected for processing (1)

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

📚 Learning: 2025-10-22T20:40:48.819Z

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

Applied to files:

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🔇 Additional comments (4)

framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java (4)

1-83: Well-structured infrastructure for synchronized AST traversal.

The class design is clean with clear separation of concerns:

• Abstract defaultPairAction provides a focused extension point
• Comprehensive documentation explains usage patterns and javac-specific considerations (wrapper nodes)
• The visitor pattern delegation through defaultAction is appropriate

---

103-235: Solid traversal and normalization implementation.

The core scanning methods are well-implemented with:

• Explicit null handling and alignment validation
• Clear error messages including class name and context
• Proper normalization to handle javac wrapper nodes

The normalization loop correctly handles nested wrappers by iterating until stable.

Consider whether additional wrapper node types should be normalized. The current implementation handles ParenthesizedTree and ExpressionStatementTree. Are there other javac wrapper constructs that could cause alignment issues between the two ASTs?

---

244-288: Clarify the intent of final modifiers on visitor methods.

The methods visitCompilationUnit, visitPackage, and visitImport are marked final, while other visitor methods (e.g., visitClass, visitMethod) are not.

If this pattern is intentional—locking down top-level structural traversal while allowing flexibility in content nodes—consider documenting this design decision in the class javadoc to guide subclass implementers.

---

244-611: The implemented visitor method pattern is intentional and well-designed. The class is documented to work by having subclasses override defaultPairAction() to handle all tree types uniformly, then override specific visit* methods only for nodes requiring recursive traversal. Missing visitor methods like visitIf, visitWhile, visitLambda, etc., intentionally fall through to defaultAction(), which calls defaultPairAction() without automatic recursion. This allows subclasses to centralize comparison logic in defaultPairAction() and selectively add recursion where needed.

The final modifier on visitCompilationUnit, visitPackage, and visitImport is intentional—these top-level structural nodes lock down the traversal backbone, while non-final content nodes (Class, Method, Variable) remain open for override flexibility.

[msridhar]

This looks pretty good to me as a start! I had a few (mostly minor) comments.

As to whether to land this code separately without any usages or tests, I'm not sure about that. Perhaps @mernst could comment. But I like being able to look over the code in a PR that is not huge.

[mernst]

Let's land it separately, but not until I have had time to do a code review.

[mernst]

Let's land it separately, but not until I have had time to do a code review.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9487d2 and 91baf9b.

📒 Files selected for processing (1)

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-22T20:40:48.819Z

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

Applied to files:

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🔇 Additional comments (11)

framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java (11)

1-51: LGTM!

The imports are appropriate for the tree types being visited, and the class documentation clearly explains the dual-tree traversal pattern and usage instructions.

---

53-80: LGTM!

The protected visibility for defaultPairAction is appropriate for a subclass hook method. The defaultAction override correctly delegates to defaultPairAction, ensuring all tree pairs (including those without explicit visitXxx overrides) go through the hook.

Note: The AI-generated summary incorrectly states this is a "public abstract method" when it's actually protected.

---

110-112: AI summary mentions normalization methods that don't exist.

The AI-generated summary lists normalize(Tree), unwrapParentheses(Tree), and unwrapExpressionStatement(Tree) as public methods, but these are not present in the code. The comment here indicates normalization may be re-introduced later. This is just a summary discrepancy, not a code issue.

---

133-144: The null-check duplication is acceptable for better error messages.

While scanOpt duplicates the null-checking logic from scan, having a distinct error message mentioning "scanOpt" aids debugging. If you prefer DRY, you could extract a helper that takes the method name, but the current approach is clear and maintainable.

---

155-157: LGTM!

The lack of @Nullable on scanExpr parameters is appropriate—this method is for call sites where expressions are known to be non-null. The Javadoc clearly documents its purpose as an intent-documenting wrapper.

---

168-178: LGTM!

The size check before iteration prevents subtle misalignment bugs. The error message includes both lists and their sizes, which aids debugging.

---

180-254: LGTM!

The compilation unit, package, import, and class visitors correctly traverse all structural children. The final modifier on the first three prevents subclass interference with foundational traversal, while visitClass remains overridable.

---

256-301: LGTM!

visitMethod comprehensively scans all relevant children including the less obvious ones (receiver parameter, default value for annotation methods). visitVariable correctly handles the common subset; the Javadoc appropriately notes that record component details are handled elsewhere.

---

303-402: LGTM!

Type-related visitors (visitModifiers through visitPrimitiveType) correctly traverse their child structures. Leaf nodes like visitPrimitiveType appropriately have no recursive scan calls.

---

404-465: LGTM!

visitTypeParameter, visitWildcard, visitIdentifier, and visitMemberSelect correctly handle their respective structures. Using scanOpt for the optional wildcard bound is appropriate.

---

467-552: LGTM!

Statement visitors (visitBlock through visitTry) correctly traverse their children. visitTry comprehensively handles resources, body block, catch clauses, and optional finally block.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java (1)

98-121: Consider using IllegalStateException instead of Error.

Error is conventionally reserved for serious JVM-level problems. For application-level invariant violations like mismatched tree structures, IllegalStateException is more appropriate. This applies to lines 104, 114, 138, and 170.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91baf9b and 1cdd455.

📒 Files selected for processing (1)

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-22T20:40:48.819Z

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

Applied to files:

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: typetools.checker-framework (misc_jdk25)
• GitHub Check: typetools.checker-framework (typecheck_part1_jdk25)
• GitHub Check: typetools.checker-framework (nonjunit_jdk25)
• GitHub Check: typetools.checker-framework (junit_jdk25)
• GitHub Check: typetools.checker-framework (inference_part1_jdk25)
• GitHub Check: typetools.checker-framework (inference_part2_jdk25)
• GitHub Check: typetools.checker-framework (typecheck_part2_jdk25)

🔇 Additional comments (12)

framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java (12)

1-28: LGTM!

Package declaration and imports are appropriate for the visitor implementation.

---

48-51: LGTM!

Class declaration and constructor are well-structured for an abstract visitor base.

---

53-80: LGTM!

The delegation pattern from defaultAction to defaultPairAction cleanly bridges the SimpleTreeVisitor API to the paired-traversal abstraction.

---

146-157: LGTM!

scanExpr provides clear intent at call sites while delegating to scan.

---

159-178: LGTM!

The size validation and lockstep iteration correctly enforce list alignment between the two ASTs.

---

187-198: LGTM!

Comprehensive traversal of compilation unit children with appropriate use of scanOpt for optional module/package and scanList for imports and type declarations.

---

207-231: LGTM!

Package and import visitors correctly traverse their child nodes.

---

264-317: LGTM!

Method, variable, and modifiers visitors comprehensively traverse their child nodes, with appropriate use of scanOpt for optional elements like receiver parameters and initializers.

---

326-435: LGTM!

Type-related visitors (annotation, annotated type, parameterized type, array type, primitive type, type parameter, wildcard) correctly traverse their respective child nodes with consistent patterns.

---

444-553: LGTM!

Expression and statement visitors correctly traverse their child nodes. The visitTry implementation comprehensively handles resources, block, catches, and finally block.

Note: Many common AST node types (if, for, while, method invocation, lambda, etc.) don't have explicit visit methods yet. They'll fall back to defaultAction, which calls defaultPairAction but doesn't recurse into children. This is acceptable for an initial infrastructure implementation—subclasses can add visitor overrides as needed.

---

133-144: Redundant null-checking: scan already handles these cases.

The scan method already handles both-null (returns) and one-null (throws). scanOpt duplicates this logic before delegating to scan. Consider simplifying:

🔎 Proposed refactor

   public final void scanOpt(@Nullable Tree tree1, @Nullable Tree tree2) {
-    if (tree1 == null && tree2 == null) {
-      return;
-    }
-    if (tree1 == null || tree2 == null) {
-      throw new Error(
-          String.format(
-              "%s.scanOpt: one tree is null: tree1=%s tree2=%s",
-              this.getClass().getCanonicalName(), tree1, tree2));
-    }
     scan(tree1, tree2);
   }

Alternatively, if distinct error messages from scanOpt are valuable for debugging, keep the current implementation.

Likely an incorrect or invalid review comment.

---

241-254: The review comment is incorrect. ClassTree in the Tree API does not have a getRecordComponents() method. Record components in the javac Tree API are represented as VariableTree nodes within ClassTree.getMembers(), which this visitor already scans. The current implementation correctly handles record components through the scanList(tree1.getMembers(), tree2cls.getMembers()) call. No changes are needed.

Likely an incorrect or invalid review comment.

[msridhar]

I think @mernst should look this over next so I've assigned him.

[coderabbitai]

Actionable comments posted: 1

Fix all issues with AI Agents 🤖

In
@framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java:
- Around line 475-493: The Javadoc for visitExpressionStatement incorrectly
references a removed normalization step; update the comment to remove or
rephrase the "may be redundant when normalize unwraps expression statements"
part and instead state that the method scans the expression and supports direct
invocation on ExpressionStatementTree nodes (mention visitExpressionStatement,
ExpressionStatementTree, scanExpr) so the comment accurately reflects current
behavior.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f2cf81 and 5f0beae.

📒 Files selected for processing (1)

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-22T20:40:48.819Z

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

Applied to files:

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🔇 Additional comments (1)

framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java (1)

30-39: Clarify the class javadoc per maintainer feedback.

The phrase "except for differences in annotations between the Java file and its corresponding .ajava file" (lines 34-35) is ambiguous. Do you mean the trees are structurally identical except that annotation nodes may differ (be present, absent, or have different values)?

Based on past review comments, the maintainer requested clarification of this phrasing.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI Agents

In
@framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java:
- Around line 148-167: The method DoubleJavacVisitor.scanList throws
java.lang.Error on a list-size mismatch; change the thrown type to
java.lang.IllegalStateException to reflect an application-level invariant
violation: replace the throw new Error(...) with throw new
IllegalStateException(...) while preserving the existing formatted message
(String.format(...)) so callers get the same diagnostic text.
- Around line 90-110: Replace the two throws of java.lang.Error in
DoubleJavacVisitor.scan with java.lang.IllegalStateException to reflect
application-level invariant violations; in the scan(@Nullable Tree tree1,
@Nullable Tree tree2) method change both "throw new Error(String.format(...))"
instances (the null-tree check and the mismatched-kind check) to "throw new
IllegalStateException(String.format(...))" while preserving the existing
formatted messages and the use of this.getClass().getCanonicalName(), tree1,
tree2, and tree1.getKind()/tree2.getKind().
- Around line 122-133: Replace the thrown Error in DoubleJavacVisitor.scanOpt
with an IllegalStateException: locate the scanOpt method and change the throw
new Error(...) call to throw new IllegalStateException(...) so it matches the
exception type used in scan and follows proper exception hierarchy and
consistency.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f0beae and a03e0a4.

📒 Files selected for processing (1)

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-22T20:40:48.819Z

Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/mustcall/MustCallVisitor.java:470-515
Timestamp: 2025-10-22T20:40:48.819Z
Learning: In MustCallVisitor.java (Checker Framework), prefer using Name.equals(...) or Objects.equals(...) for com.sun.source.util.Name comparisons (instead of ==/!=). This should be the default unless the Interning Checker is explicitly used to guarantee reference equality.

Applied to files:

• framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java

🔇 Additional comments (1)

framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java (1)

177-220: Clarify or fix inconsistent final modifiers on visit methods.

visitCompilationUnit (line 177), visitPackage (line 197), and visitImport (line 214) are declared final, while all other visitXxx methods are not. The class Javadoc states that "subclasses are expected to override visitXxx methods for the tree kinds they care about" (lines 61-62), which suggests visit methods should be overridable.

Is there a specific reason these three methods must not be overridden, or is this an inconsistency that should be resolved?

If these methods should be overridable like the others, remove the final modifier. If they must remain final, consider documenting why in the Javadoc to clarify the design intent.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java`:
- Around line 213-231: The scanAnnotations method in DoubleJavacVisitor uses
positional comparison (Math.min and index-based scanning) which assumes
corresponding annotations share the same indices and any extras appear at the
end; update the Javadoc for DoubleJavacVisitor.scanAnnotations to explicitly
state this positional correspondence assumption and that the method will only
compare annotations up to the shorter list length, so annotations must not be
interspersed and extra annotations are expected to be appended at the end of the
lists.

[avenger2597]

@mernst I’ve updated the class-level and method Javadocs to more clearly explain the structure, traversal model, and current limitations of this visitor. In particular, the documentation now calls out where traversal is explicit, why this subclasses SimpleTreeVisitorrather than TreeScanner, and which kinds of mismatches (e.g., wildcard direction, annotation handling, record components on older JDKs) are intentionally out of scope for this initial infrastructure. Please take a look and let me know if anything is still unclear or if you think additional cases should be documented.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java`:
- Around line 41-43: In the Javadoc for DoubleJavacVisitor (the comment
referencing the main entry point {`@link` `#scan`}), fix the grammatical mistake by
inserting the missing conjunction so the phrase reads "scan checks and invokes"
instead of "scan checks invokes"; update the sentence that mentions {`@link`
Tree#accept} and {`@code` visitXyz} accordingly to maintain clarity.

♻️ Duplicate comments (1)

framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java (1)

31-35: Explicit receiver-parameter differences are documented as allowed, but enforced as strict.

The class Javadoc states receiver-parameter differences between a .java and .ajava tree are acceptable, yet visitMethod calls scan on receiver parameters (line 324), which will throw when one side is null. Either align the behavior to tolerate receiver-parameter mismatches or tighten the Javadoc to require receiver parameters be pre-aligned before traversal.

[mernst]

Thanks for the explanations and improvements; I appreciate it. I have a few more questions and suggestions.

[mernst]

What is "traversed explicitly"? What would be an example of traversal that is non-explicit?

[avenger2597]

I realized I was mistaken about the javac AST structure here. I initially thought there was a separate “record components” list exposed on ClassTree, but that’s not the case: the public javac tree API does not provide any getRecordComponents() or equivalent accessor (please correct me if I’m mistaken).

Record declarations themselves are traversed normally, via visitClass(ClassTree, …), which matches how DoubleJavaParserVisitor traverses RecordDeclaration. There is no additional record-specific list being skipped; the traversal here is already consistent with what JavaParser does at the declaration level.

I’ve updated the Javadoc to remove the misleading wording that suggested record components were “not traversed explicitly”.

[avenger2597]

This answers everything related to records, so closing those comments. Please let me know if you have any other questions related to that.

[mernst]

Can you please expand on why this design is necessary and why the Tree.accept method cannot be used?

[avenger2597]

In JavaParser, recursion is typically expressed as child1.accept(this, child2), but that call is almost always guarded by Optional (e.g., getScope().ifPresent(...), getInitializer().ifPresent(...), etc.). The JavaParser API enforces at the type level that callers handle missing children before recursing.

The javac tree API does not provide this structure: many child getters return nullable values, and calling accept directly would either require repetitive null checks at every call site or risk accidental NullPointerExceptions.

The scan(Tree, Tree) helper exists mainly to provide the same safety that JavaParser’s Optional.ifPresent(...) calls provide, since the javac API represents missing children with null instead of Optional. That is why I didn't use Tree.accept directly.

I have also updated the Javadoc to make it more clear.

[avenger2597]

I have updated the wording of the Javadoc, and above comment answers other questions related to scan method.

[mernst]

Which of the children are the "main children"? Which ones are skipped?

[avenger2597]

Nothing is skipped. I updated the javadoc to make it clear what exactly is being scanned.

[mernst]

Please be specific about which JDK versions.

[mernst]

What is "record-specific structure"?

[mernst]

If I only care about methods, don't I also need to overide visitCompilationUnit and visitClass, even though they are not tree kinds I care about?

[avenger2597]

No, you only need to override the visitXyz methods for the kinds you care about.

The base class already provides visitCompilationUnit, visitClass, etc., and those methods drive traversal into children (e.g., visitCompilationUnit scans type decls; visitClass scans members; visitMethod scans body). So a subclass that “only cares about methods” can override visitMethod and rely on the base implementation of visitCompilationUnit/visitClass to reach every MethodTree.

You’d only override visitCompilationUnit/visitClass if you want to change how traversal proceeds at those nodes (e.g., skip some subtree, or add extra checks).

[mernst]

This statement makes it sound like overriding defaultAction() is essential. Is that true?

[avenger2597]

Yes, in this class it’s required in the sense that it’s abstract, so a subclass has to provide some implementation to compile. But it doesn’t have to “do” anything: callers can implement it as a no-op if they only care about overriding specific visitXyz methods.

Conceptually, it matches DoubleJavaParserVisitor: you always implement defaultAction as the action that runs on every matched pair, and then optionally override specific visitXyz methods.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In
`@framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java`:
- Around line 188-200: The Javadoc for visitPackage is incorrect about scanning
annotations; update the comment for the visitPackage(PackageTree ptree1, Tree
tree2) method to state that package annotation lists are not traversed
(consistent with the class-level contract) and that only the package name is
scanned via scan(ptree1.getPackageName(), ptree2.getPackageName()); keep the
description concise and reflect that annotations on the package are
intentionally skipped.
- Around line 35-43: The class Javadoc for DoubleJavacVisitor contains a
duplicated paragraph describing the entry point; remove the redundant copy so
only a single paragraph describes {`@link` `#scan`(Tree, Tree)}. Edit the Javadoc in
DoubleJavacVisitor to keep one clear entry-point paragraph (mentioning {`@link`
`#scan`(Tree, Tree)} and its behavior) and delete the repeated block.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java`:
- Around line 239-262: The visitMethod currently calls
scan(mtree1.getReceiverParameter(), mtree2.getReceiverParameter()) which throws
when only one receiver exists; change visitMethod to tolerate asymmetric
receiver parameters by checking the two receiver values first: retrieve both via
getReceiverParameter(), and if either is null (but not both) skip the scan
(treat as allowed difference), otherwise call scan(...) when both non-null;
update the visitMethod implementation (reference: visitMethod,
getReceiverParameter, scan) so the code path accepts one-side-only receiver
parameters instead of invoking scan on a null pair.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@framework/src/main/java/org/checkerframework/framework/ajava/DoubleJavacVisitor.java`:
- Around line 312-319: The local variable name `ttree2` in the method
visitAnnotatedType should follow the existing naming convention and match the
parameter prefix (change to `atree2`); update the declaration AnnotatedTypeTree
ttree2 to AnnotatedTypeTree atree2 and replace all uses (the
defaultAction(atree1, ttree2) and scan(...) calls) to use atree2 so the
parameter/variable naming is consistent with other visit methods.

[avenger2597]

@mernst I’ve updated the PR to
(1) remove annotation traversal from the base visitor, since annotations may legitimately differ between .java and .ajava,
(2) simplify and clarify the Javadoc around traversal and defaultAction() to avoid implying extra guarantees, and
(3) relax receiver-parameter handling to tolerate .java vs .ajava differences while keeping the rest of the structure strict.

I believe this addresses all the open review questions, but please let me know if I missed anything or if there are other concerns to look at.