Repository navigation
Collapse the purity.not.* message-key variants into purity.* - #8242
Conversation
Rename the message keys purity.not.deterministic.X, purity.not.sideeffectfree.X, and purity.not.deterministic.not.sideeffectfree.X to purity.X. The required purity, and for purity.call the adjective describing the callee, become message arguments, so every rendered message is unchanged. Build the message key from a switch over the reasons, so that each key is a literal that the CompilerMessages checker verifies; this removes a @SuppressWarnings("compilermessages"). This breaks existing warning suppressions that use the old keys, so bump the version to 4.3.0.
Adapt reportPurityError() to the new ImpurityReason record, and update the purity message keys in the tests that upstream added.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: typetools/checker-framework/.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. 📝 WalkthroughWalkthroughThe Purity Checker now reports violations with generalized Merge Risk: ⚪ Minimal · up to Purity diagnostics now use the unified keys, with updated tests and documentation. No material merge risk remains. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 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 |
Use the message key "purity.<reason>" for a purity violation reason that PurityChecker does not produce, rather than throwing BugInCF. A checker that records its own reason via PurityResult.addNotDetReason, addNotSEFreeReason, or addNotBothReason defines the key in its own messages.properties file. In Issue951.java, put each of two purity.call errors on its own line. The two errors previously had distinct message keys; now that both keys are purity.call, two expected diagnostics on one line collapse into one, because TestDiagnostic equality ignores the message text. In the changelog, state that a suppression that uses an old key silently stops working, and that the new key cannot distinguish the purity property that the method failed to have.
Track the enclosing type to compare in a single nullable variable, so that the nullness checker can relate the null check to the use.
mernst
changed the title
purity.not.* message-key variants into purity.*
2 participants
Merge together:
#8242
typetools/jdk#298
plume-lib/bcel-util#584
plume-lib/javac-parse#312
plume-lib/merging#397
plume-lib/plume-util#786
plume-lib/reflection-util#545
Rename the message keys
purity.not.deterministic.X,purity.not.sideeffectfree.X, andpurity.not.deterministic.not.sideeffectfree.Xtopurity.X. The required purity, and forpurity.callthe adjective describing the callee, become message arguments, so every rendered message is unchanged.Build the message key from a switch over the reasons, so that each key is a literal that the CompilerMessages checker verifies; this removes a
@SuppressWarnings("compilermessages").This breaks existing warning suppressions that use the old keys.