Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
package org.checkerframework.framework.ajava;

import com.sun.source.tree.AnnotatedTypeTree;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ModifiersTree;
import com.sun.source.tree.ModuleTree;
import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.PackageTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.TypeParameterTree;
import java.util.Collections;
import java.util.List;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Given two javac ASTs representing the same Java file that may differ in annotations, tests if
* they have the same annotations.
*
* <p>To use this class, call {@link #scan} with the roots of the two ASTs. Then, check {@link
* #getAnnotationsMatch}.
*/
public class JavacAnnotationEqualityVisitor extends DoubleJavacVisitor {

/** True if no node with mismatched annotations has been seen. */
private boolean annotationsMatch = true;

/** If a node with mismatched annotations has been seen, stores the node from the first AST. */
private @MonotonicNonNull Tree mismatchedNode1 = null;

/** If a node with mismatched annotations has been seen, stores the node from the second AST. */
private @MonotonicNonNull Tree mismatchedNode2 = null;

/** Constructs a {@code JavacAnnotationEqualityVisitor}. */
public JavacAnnotationEqualityVisitor() {
annotationsMatch = true;
}
Comment on lines +25 to +37
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Fields and constructor are correct and consistent with the predecessor.

The @MonotonicNonNull semantics are correct: once a mismatch is recorded the fields never revert to null. This matches the design of AnnotationEqualityVisitor. Consider adding a note in the class-level Javadoc that the instance is single-use (a second scan call will not reset annotationsMatch), as the @MonotonicNonNull contract makes reset() impossible without creating a new instance.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@framework/src/main/java/org/checkerframework/framework/ajava/JavacAnnotationEqualityVisitor.java`
around lines 27 - 41, Add a short note to the class Javadoc of
JavacAnnotationEqualityVisitor stating that instances are single-use: once scan
is called and a mismatch is recorded (annotationsMatch toggled and
mismatchedNode1/mismatchedNode2 set), the visitor is not resettable and a new
instance must be created for another scan; reference the fields
annotationsMatch, mismatchedNode1 and mismatchedNode2 (and mention that reset()
is not provided/possible due to the `@MonotonicNonNull` contract) so readers know
the intended lifecycle.


/**
* Returns true if all visited pairs of nodes had matching annotations.
*
* @return true if all visited pairs of nodes had matching annotations
*/
public boolean getAnnotationsMatch() {
return annotationsMatch;
}

/**
* If a visited pair of nodes has had mismatched annotations, returns the node from the first AST
* where annotations differed, or null otherwise.
*
* @return the node from the first AST with differing annotations, or null
*/
public @Nullable Tree getMismatchedNode1() {
return mismatchedNode1;
}

/**
* If a visited pair of nodes has had mismatched annotations, returns the node from the second AST
* where annotations differed, or null otherwise.
*
* @return the node from the second AST with differing annotations, or null
*/
public @Nullable Tree getMismatchedNode2() {
return mismatchedNode2;
}

/**
* Returns the annotation trees on the given tree, or an empty list if the tree type does not
* carry annotations.
*
* <p>In javac's AST, annotations appear on {@link ModifiersTree} (declaration annotations),
* {@link AnnotatedTypeTree} (type-use annotations), {@link TypeParameterTree}, {@link
* PackageTree}, {@link ModuleTree}, and {@link NewArrayTree}.
*
* @param tree a tree
* @return the annotations on the tree
*/
public static List<? extends AnnotationTree> getAnnotations(Tree tree) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very useful method, thanks. Could you please define it in TreeUtils rather than here?

if (tree instanceof ModifiersTree t) {
return t.getAnnotations();
}
if (tree instanceof AnnotatedTypeTree t) {
return t.getAnnotations();
}
if (tree instanceof TypeParameterTree t) {
return t.getAnnotations();
}
if (tree instanceof PackageTree t) {
return t.getAnnotations();
}
if (tree instanceof ModuleTree t) {
return t.getAnnotations();
}
if (tree instanceof NewArrayTree t) {
return t.getAnnotations();
}
return Collections.emptyList();
}

/**
* Compares two lists of annotation trees by their string representations. Javac trees do not
* implement structural {@code equals}, so string comparison is used instead. Javac tree {@code
* toString} does not include comments, so no comment-stripping is needed.
*
* @param annos1 the first list of annotations
* @param annos2 the second list of annotations
* @return true if the two lists represent the same annotations
*/
private static boolean annotationsEqual(
List<? extends AnnotationTree> annos1, List<? extends AnnotationTree> annos2) {
if (annos1.size() != annos2.size()) {
return false;
}
for (int i = 0; i < annos1.size(); i++) {
if (!annos1.get(i).toString().equals(annos2.get(i).toString())) {
return false;
}
}
return true;
Comment thread
mernst marked this conversation as resolved.
}

@Override
protected Void defaultAction(Tree tree1, Tree tree2) {
if (!annotationsMatch) {
return null;
}

List<? extends AnnotationTree> annos1 = getAnnotations(tree1);
List<? extends AnnotationTree> annos2 = getAnnotations(tree2);

if (!annotationsEqual(annos1, annos2)) {
annotationsMatch = false;
mismatchedNode1 = tree1;
mismatchedNode2 = tree2;
}

return null;
}
Comment on lines +123 to +139
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

defaultAction logic is correct; one traversal note.

The early return when !annotationsMatch correctly prevents redundant annotation comparisons. Note, however, that because DoubleJavacVisitor.visitXyz methods call defaultAction and then unconditionally continue scanning children (see visitCompilationUnit and visitPackage in the base class), the full tree traversal continues after the first mismatch — only the per-node comparison work is skipped. For the typical file sizes in this use case the extra overhead is negligible (just a null check and return per remaining node), but if early exit becomes desirable, overriding scan(Tree, Tree) to guard on !annotationsMatch before delegating to super.scan would stop traversal immediately.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@framework/src/main/java/org/checkerframework/framework/ajava/JavacAnnotationEqualityVisitor.java`
around lines 127 - 143, The current defaultAction in
JavacAnnotationEqualityVisitor correctly skips per-node comparison when
annotationsMatch is false but traversal keeps running; to allow an immediate
stop on first mismatch, add an override of scan(Tree, Tree) in
JavacAnnotationEqualityVisitor that checks the annotationsMatch flag and returns
immediately if false, otherwise delegates to super.scan(tree1, tree2); keep
existing defaultAction, annotationsMatch, mismatchedNode1/mismatchedNode2,
getAnnotations and annotationsEqual logic unchanged so the early-exit only
prevents further recursion.

}
Comment on lines +23 to +140
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find test files covering AnnotationEqualityVisitor or the ajava visitor infrastructure
rg -rn --type=java -l 'AnnotationEqualityVisitor|JavacAnnotationEqualityVisitor' --glob '*Test*'

# Also look for tests in the framework test directory
fd -t f -e java 'Annotation' --search-path framework/src/test

Repository: typetools/checker-framework

Length of output: 164


🏁 Script executed:

#!/bin/bash

# Search for AnnotationEqualityVisitor (non-Javac version) definition
echo "=== Searching for AnnotationEqualityVisitor class definition ==="
rg -rn 'class AnnotationEqualityVisitor' --type=java

# Search for JavacAnnotationEqualityVisitor in test files
echo ""
echo "=== Searching for JavacAnnotationEqualityVisitor in all test files ==="
rg -rn 'JavacAnnotationEqualityVisitor' --type=java framework/src/test

# Look for test files in the ajava package
echo ""
echo "=== Looking for test files in ajava package ==="
fd -t f -e java . framework/src/test --path '*ajava*'

# List test directory structure near ajava
echo ""
echo "=== Test directory structure ==="
fd -t d . framework/src/test | grep -E '(ajava|test)' | head -20

Repository: typetools/checker-framework

Length of output: 2098


🏁 Script executed:

#!/bin/bash

# Search for usages of JavacAnnotationEqualityVisitor to understand how it's tested
echo "=== Searching for usages of JavacAnnotationEqualityVisitor ==="
rg -rn 'JavacAnnotationEqualityVisitor' --type=java

# Search for any test files that might reference ajava functionality
echo ""
echo "=== Searching for ajava references in test files ==="
rg -rn 'ajava' --type=java framework/src/test

# Check the main file structure in ajava
echo ""
echo "=== Files in ajava package ==="
fd -t f -e java . framework/src/main/java/org/checkerframework/framework/ajava

Repository: typetools/checker-framework

Length of output: 1882


🏁 Script executed:

#!/bin/bash

# Search for usage of AnnotationEqualityVisitor (the non-Javac version)
echo "=== Searching for usages of AnnotationEqualityVisitor ==="
rg -rn 'AnnotationEqualityVisitor' --type=java framework/src

# Look for test files that test ajava package functionality indirectly
echo ""
echo "=== Searching for InsertAjavaAnnotations usage (main entry point) ==="
rg -rn 'InsertAjavaAnnotations' --type=java framework/src/test

# Check if there are any ajava-related tests in integration or system test files
echo ""
echo "=== Test files mentioning annotation or visitor ==="
fd -t f -e java . framework/src/test/java/org/checkerframework/framework/test | xargs grep -l 'annotation\|Visitor' 2>/dev/null | head -10

Repository: typetools/checker-framework

Length of output: 1688


🏁 Script executed:

#!/bin/bash

# Check the TreeParserTest to see if it tests annotation equality
echo "=== TreeParserTest.java content (checking for annotation equality tests) ==="
head -100 framework/src/test/java/org/checkerframework/framework/test/junit/TreeParserTest.java

# Search for where annotation visitors are actually tested
echo ""
echo "=== Search for .ajava file processing in tests ==="
rg -rn '\.ajava' --type=java framework/src/test

# Check if there are any annotation file tests
echo ""
echo "=== Search for AnnotationFileStore usage (which would use the visitor) ==="
rg -rn 'AnnotationFileStore' --type=java framework/src

Repository: typetools/checker-framework

Length of output: 3939


🏁 Script executed:

#!/bin/bash

# Check BaseTypeVisitor to see how AnnotationEqualityVisitor is used
echo "=== Checking BaseTypeVisitor usage of AnnotationEqualityVisitor ==="
rg -A 5 -B 5 'new.*AnnotationEqualityVisitor' framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java

# Check if there are any test methods in the JavacAnnotationEqualityVisitor file itself
echo ""
echo "=== Full JavacAnnotationEqualityVisitor.java file ==="
cat framework/src/main/java/org/checkerframework/framework/ajava/JavacAnnotationEqualityVisitor.java

Repository: typetools/checker-framework

Length of output: 5455


Add unit tests for JavacAnnotationEqualityVisitor before client conversion.

Neither JavacAnnotationEqualityVisitor nor its non-Javac counterpart have unit test coverage. The class includes non-trivial logic—the getAnnotations dispatch across six tree types and toString()-based annotation comparison—that benefits from direct testing. At minimum, test cases covering matching annotations, mismatched annotations, and annotation-free nodes would provide confidence before the client conversion in the follow-up PR.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@framework/src/main/java/org/checkerframework/framework/ajava/JavacAnnotationEqualityVisitor.java`
around lines 25 - 144, Add unit tests for JavacAnnotationEqualityVisitor to
cover its non-trivial logic: write a test class that constructs minimal javac
Tree instances (or mocks) with annotated ModifiersTree, AnnotatedTypeTree,
TypeParameterTree, PackageTree, ModuleTree, and NewArrayTree and exercise
getAnnotations via the visitor's defaultAction to verify three
scenarios—matching annotations (assert getAnnotationsMatch() == true and
mismatched nodes == null), mismatched annotations (assert getAnnotationsMatch()
== false and getMismatchedNode1()/getMismatchedNode2() point to the differing
trees), and annotation-free nodes (assert empty annotation lists and no
mismatch). Use JavacAnnotationEqualityVisitor, getAnnotations, annotationsEqual,
defaultAction, getAnnotationsMatch, getMismatchedNode1, and getMismatchedNode2
as the referenced symbols when locating code to test.