Refactor cache to determine if an element is annotatedfor

[aosen-xiong]

In this PR, I refactored the isElementAnnotatedForThisChecker and its cache from QualifierDefaults to AnnotatedTypeFactory to foster code reuse in both determining the default and whether error should be suppressed. I am making the method SourceChecker#isElementAnnotatedForThisChecker abstract to reduce code duplication.

[wmdietl]

Thanks!

[Copilot]

Pull request overview

This PR refactors the cache and method for determining if an element is annotated for a checker, moving it from QualifierDefaults to AnnotatedTypeFactory to enable code reuse. The method isElementAnnotatedForThisChecker is renamed to isElementAnnotatedForThisCheckerOrUpstreamChecker and made abstract in SourceChecker.

• Moved the isElementAnnotatedForThisChecker method and its cache from QualifierDefaults to AnnotatedTypeFactory
• Made SourceChecker#isElementAnnotatedForThisCheckerOrUpstreamChecker an abstract method
• Implemented the abstract method in all subclasses (BaseTypeChecker, AggregateChecker, and utility checkers)

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
QualifierDefaults.java	Removed private isElementAnnotatedForThisChecker method and elementAnnotatedFors cache; updated calls to use the factory method
AnnotatedTypeFactory.java	Added public isElementAnnotatedForThisCheckerOrUpstreamChecker method with the cache implementation
SourceChecker.java	Changed method from private to protected abstract; removed implementation and imports; updated method calls
BaseTypeChecker.java	Implemented abstract method by delegating to the type factory
AggregateChecker.java	Implemented abstract method to return false (delegates to subcheckers)
SignaturePrinter.java	Implemented abstract method to throw BugInCF (not expected to be called)
JavaCodeStatistics.java	Implemented abstract method to throw BugInCF (not expected to be called)
AnnotationStatistics.java	Implemented abstract method to throw BugInCF (not expected to be called)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wmdietl]

@aosen-xiong Please go through the copilot suggestions and see what should be addressed.

[aosen-xiong]

Weird, CI failed again.

[aosen-xiong]

@wmdietl ping for review.

[aosen-xiong]

@wmdietl Hi Werner, can you please review this PR? Thanks. After this one, we can move to #1304.

[aosen-xiong]

@thisisalexandercook Hi Alex, can you also review this PR? Thanks!

[thisisalexandercook]

EDIT

This refactor makes sense in light of the postInit(); chain discussed here.

I've PRed a potential tightening here: aosen-xiong#23

Alternatively we could also consider publishing the type factory for use in its postInit() phase, i.e. set something like this.checker.setPostInitTypeFactory(this); in the body of GenericAnnotatedTypeFactory.postInit();. The downside is that it exposes a partially initialized type factory through the checker, which sounds like something we would want to avoid. But we are technically querying the factory during postInit(); already anyway, this would just give the checker a route to it, so maybe worth considering?

[aosen-xiong]

@wmdietl Alex refined the improvements in aosen-xiong#24 (comment). Can you take a look of this PR?

[wmdietl]

Copilot refuses to review cross-repo PRs, but in a chat it had these comments:

The 10 suggestions identified for PR #1331 are:

• PR title – Capitalize @AnnotatedFor in the title
• SourceChecker abstract method Javadoc – @param elt … or null is wrong; the method now throws on null
• BaseTypeChecker override – @Nullable on elt is inconsistent with the implementation that throws BugInCF for null
• shouldSuppressWarnings(Element,…) – Removing the @AnnotatedFor check silently changes API behavior; needs documentation/justification
• shouldSuppressWarnings(TreePath,…) – "Return false immediately" comments removed without replacement; new caching logic needs explanation
• shouldSuppressWarnings(TreePath,…) – The isElementAnnotatedForThisCheckerOrUpstreamChecker(packageElement) call is now redundant since the method walks enclosing elements itself
• QualifierDefaults.applyConservativeDefaults – The isParsingAnnotationFiles() guard needs a comment explaining the initialization-cycle issue it prevents
• AnnotationStatistics / JavaCodeStatistics – throw new BugInCF("Unexpected call…") may be too aggressive; return false with a comment would be safer
• SignaturePrinter – Same concern as above
• AggregateChecker Javadoc – Missing @param elt and @return documentation tags

@aosen-xiong Can you see which of these make sense to address?