Remove an unprinted annotation from the AST, not from the pretty-printer - #8183
Conversation
To avoid cluttering an ajava file, whole-program inference does not print the invisible qualifiers. It suppressed them by overriding the pretty-printer's three `visit(...AnnotationExpr)` methods to return without printing. By the time the pretty-printer visits an annotation, it has already printed the whitespace that separates the annotation from what follows it, so each suppressed annotation leaves a stray space or blank line: `java.util. Date`, `static double`, `String [] []`. That partly defeats the purpose of not printing the annotation, which is to reduce clutter. Instead, remove the annotations that should not be printed from a clone of the compilation unit, and print that. The pretty-printer then outputs no separator for them. The clone is necessary because the removal is a side effect, and the AST is printed once per checker that was run. No test output changes: the test checkers declare no invisible qualifier, so nothing is removed in the test suite. This refactoring is worthwhile on its own for the stray whitespace it fixes for a checker that does declare one, and it is a prerequisite for omitting irrelevant annotations, which would otherwise leave the same stray whitespace on every annotation it omits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthrough
Priority: ⬇️ Low Change: Bug fix Merge Risk: ⚪ Minimal · up to The output change removes invisible annotations from the cloned AST without leaving a verified annotation path unfiltered, so no merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
1 participant
Part 2 of 6, splitting #8177 into reviewable pieces. This one is independent of #8182 and can merge in parallel with it; parts 3-6 build on it.
To avoid cluttering an ajava file, whole-program inference does not print the invisible qualifiers. It suppressed them by overriding the pretty-printer's three
visit(...AnnotationExpr)methods to return without printing. By the time the pretty-printer visits an annotation it has already printed the whitespace that separates the annotation from what follows it, so each suppressed annotation leaves a stray space or blank line:java.util. Date,static double,String [] [].Instead, remove the annotations that should not be printed from a clone of the compilation unit, and print that. The pretty-printer then outputs no separator for them. The clone is needed because the removal is a side effect and the AST is printed once per checker that was run.
No test output changes. The test checkers declare no invisible qualifier, so nothing is removed in the test suite. The refactoring is worthwhile on its own for the stray whitespace it fixes for a checker that does declare one, and it is a prerequisite for #8184: omitting irrelevant annotations in the pretty-printer would leave the same stray whitespace on every annotation omitted.
🤖 Generated with Claude Code