Skip to content

Document why removeAnnosFromScene skips declaration annotations - #8127

Merged
mernst merged 5 commits into
typetools:masterfrom
mernst:wpi-review-fix-50
Sep 11, 2026
Merged

mernst merged 5 commits into
typetools:masterfrom
mernst:wpi-review-fix-50

Conversation

@mernst

@mernst mernst commented Sep 9, 2026

Copy link
Copy Markdown
Member

ASceneWrapper.removeAnnosFromScene filters the field, return, receiver,
and parameter type elements, but not aclass.tlAnnotationsHere. That is
correct: annosToIgnore is populated only by
WholeProgramInferenceScenesStorage.addAnnotationsToATypeElement, so it
never holds a declaration annotation. Say so.

Also record, as a TODO, that precondition and postcondition type elements
are not visited even though they can hold ignorable annotations.

mernst and others added 2 commits September 4, 2026 05:47
`ASceneWrapper.removeAnnosFromScene` filters the field, return, receiver,
and parameter type elements, but not `aclass.tlAnnotationsHere`.  That is
correct: `annosToIgnore` is populated only by
`WholeProgramInferenceScenesStorage.addAnnotationsToATypeElement`, so it
never holds a declaration annotation.  Say so.

Also record, as a TODO, that precondition and postcondition type elements
are not visited even though they can hold ignorable annotations.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 46 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a8f612b4-7d7c-4dcb-a59e-b269ea658102

📥 Commits

Reviewing files that changed from the base of the PR and between 5e5d094 and f1e8924.

📒 Files selected for processing (1)
  • framework/src/main/java/org/checkerframework/common/wholeprograminference/scenelib/ASceneWrapper.java

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8e910f51-197f-43fe-9b3b-cfc7c614fc86

📥 Commits

Reviewing files that changed from the base of the PR and between 94d5c6e and 5e5d094.

📒 Files selected for processing (1)
  • framework/src/main/java/org/checkerframework/common/wholeprograminference/scenelib/ASceneWrapper.java

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The Javadoc for removeAnnosFromScene now documents the annotation kinds that the method visits. It explains why formal-parameter declaration annotations are visited and why their removal is harmless. It also records a TODO for type annotations on inferred preconditions and postconditions.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 5e5d0

This change clarifies annotation-removal behavior and known limitations without changing runtime behavior. No current merge-readiness risk remains.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

mernst and others added 3 commits September 9, 2026 00:06
The claim that this method visits only type annotations is false for
formal parameters: addDeclarationAnnotationToFormalParameter stores the
declaration annotation in param.type.tlAnnotationsHere, which is the set
that this method removes annotations from.  The removal is still safe,
but because annosToIgnore records only supported type qualifiers, not
because declaration annotations are unvisited.

Also, the TODO said that inferred pre- and postconditions are recorded
under TypeUseLocation.FIELD.  That holds only for contracts about field
expressions; contracts about a formal parameter or the receiver are
recorded under TypeUseLocation.PARAMETER.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mernst
mernst merged commit 0d7f81f into typetools:master Sep 11, 2026
27 checks passed
@mernst
mernst deleted the wpi-review-fix-50 branch September 11, 2026 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant