Improve documentation by link getUpstreamCheckerNames and getSuppressWarningsPrefixes#1016
Improve documentation by link getUpstreamCheckerNames and getSuppressWarningsPrefixes#1016aosen-xiong wants to merge 5 commits into
getUpstreamCheckerNames and getSuppressWarningsPrefixes#1016Conversation
wmdietl
left a comment
wmdietl
left a comment
There was a problem hiding this comment.
I think the point of #1011 was to improve the documentation of these methods in BaseTypeChecker. That documentation should make clear that these two methods likely want similar behavior.
The improvement here might be good, but please also look at BaseTypeChecker (and also check whether these methods are overridden elsewhere).
|
Thanks! Since those two methods are in sourcechecker, I noticed there are five overrides for |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates Javadoc in InitializationChecker to better explain how upstream checkers and @SuppressWarnings prefixes are coordinated for AnnotatedFor support.
Changes:
- Expanded
getUpstreamCheckerNames()Javadoc to referencegetSuppressWarningsPrefixes()and add an explicit@returndescription. - Added new Javadoc for
getSuppressWarningsPrefixes()describing the"initialization"suppression prefix and noting its relationship to upstream checkers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Should there be further overrides then? |
| * Returns a list containing this checker name and all checkers it is a part of (that is, | ||
| * checkers that called it). | ||
| * | ||
| * <p>This list determines which {@code @AnnotatedFor} annotations are recognized as covering |
There was a problem hiding this comment.
Is that the only use case or is that just an example?
| * <p>Note: {@link #getSuppressWarningsPrefixes()} and {@link #getUpstreamCheckerNames()} are a | ||
| * related pair and should be kept in sync. If you override this method to add an additional | ||
| * prefix (for example, the prefix of an abstract parent class that this checker extends), you | ||
| * should also override {@link #getUpstreamCheckerNames()} to add the corresponding checker |
There was a problem hiding this comment.
Wouldn't it be nicer if we did that for the user? At the moment we only determine one standard prefix. Couldn't we just use all the upstream checker names directly?
Fixes #1011