Extend RLC to collections of resources
Summary by CodeRabbit
-
New Features
- Introduces collection-ownership checking integrated with the Resource Leak Checker, with new annotations (e.g., owning/non-owning collections and field destructors).
- Defaults and ownership transfer for parameters, returns, fields, assignments, and new allocations.
- Loop-aware analysis (enhanced-for and index loops) to verify element must-call obligations.
- Improved diagnostics and dataflow visualization for iterated collection elements.
-
Bug Fixes
- Fewer false positives: iterator and map access now reflect non-owning returns, reducing spurious errors.
-
Documentation
- Manual updated with collection-ownership model, defaults, and usage guidance.
-
Tests
- Extensive new test suites for collections, fields, loops, and defaults.
@skehrli If you make CI pass, then CI will be more effective in warning us of regressions.
📝 Walkthrough
Walkthrough
Adds a Collection Ownership type system and integrates it into the Resource Leak Checker. New ownership qualifiers and annotations (OwningCollection, NotOwningCollection, PolyOwningCollection, OwningCollectionWithoutObligation, OwningCollectionBottom, CollectionFieldDestructor, CreatesCollectionObligation) are introduced. Implements checker components (Checker, AnnotatedTypeFactory, Analysis, Transfer, Store, Visitor), augments MustCall/RLCCalledMethods flows and dataflow to track iterated collection elements (IteratedCollectionElement, CFAbstractStore), and updates many APIs with ownership/nullness annotations, messages, docs, and tests.
Possibly related PRs
- typetools/checker-framework#7318: Edits ownership annotations and collection utilities (e.g., LinkedHashKeyedSet, ASTPath iterator signatures), overlapping code-level ownership-annotation changes in this PR.
Suggested reviewers
- smillst
Pre-merge checks and finishing touches
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 0faec762f4d0347697f9cbd477f1118e13e3f276 and 4c56f0cf91adc388c713356e52f00da0c46b14e5.
📒 Files selected for processing (1)
checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallInference.java(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java:767-798
Timestamp: 2025-10-22T20:43:32.957Z
Learning: In MustCallConsistencyAnalyzer.addObligationsForOwningCollectionReturn, treat a null result from CollectionOwnershipAnnotatedTypeFactory.getMustCallValuesOfResourceCollectionComponent(...) as “no obligations” and skip creating CollectionObligation(s); do not throw. This can occur for OwningCollection over non-resource element types.
Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java:767-798
Timestamp: 2025-10-22T20:43:32.957Z
Learning: When handling collection ownership in MustCallConsistencyAnalyzer, do not throw if CollectionOwnershipAnnotatedTypeFactory.getMustCallValuesOfResourceCollectionComponent(..) returns null; treat it as “no obligations” and skip creating CollectionObligation(s). Also, when constructing diagnostics that reference an element method name, fall back to "Unknown" if the list is null/empty.
Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/collectionownership/CollectionOwnershipAnnotatedTypeFactory.java:128-136
Timestamp: 2025-10-22T20:41:01.112Z
Learning: In checker/src/main/java/org/checkerframework/checker/collectionownership/CollectionOwnershipAnnotatedTypeFactory.java, prefer instance fields over static state for analysis caches (e.g., maps keyed by Tree/Block). There is at most one ATF per running checker, so static mutable fields are discouraged.
📚 Learning: 2025-10-22T20:43:32.957Z
Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java:767-798
Timestamp: 2025-10-22T20:43:32.957Z
Learning: When handling collection ownership in MustCallConsistencyAnalyzer, do not throw if CollectionOwnershipAnnotatedTypeFactory.getMustCallValuesOfResourceCollectionComponent(..) returns null; treat it as “no obligations” and skip creating CollectionObligation(s). Also, when constructing diagnostics that reference an element method name, fall back to "Unknown" if the list is null/empty.
Applied to files:
checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallInference.java
📚 Learning: 2025-10-22T20:41:01.112Z
Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/collectionownership/CollectionOwnershipAnnotatedTypeFactory.java:128-136
Timestamp: 2025-10-22T20:41:01.112Z
Learning: In checker/src/main/java/org/checkerframework/checker/collectionownership/CollectionOwnershipAnnotatedTypeFactory.java, prefer instance fields over static state for analysis caches (e.g., maps keyed by Tree/Block). There is at most one ATF per running checker, so static mutable fields are discouraged.
Applied to files:
checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallInference.java
📚 Learning: 2025-10-22T20:43:32.957Z
Learnt from: kelloggm
Repo: typetools/checker-framework PR: 7166
File: checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java:767-798
Timestamp: 2025-10-22T20:43:32.957Z
Learning: In MustCallConsistencyAnalyzer.addObligationsForOwningCollectionReturn, treat a null result from CollectionOwnershipAnnotatedTypeFactory.getMustCallValuesOfResourceCollectionComponent(...) as “no obligations” and skip creating CollectionObligation(s); do not throw. This can occur for OwningCollection over non-resource element types.
Applied to files:
checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallInference.java
🧬 Code graph analysis (1)
checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallInference.java (2)
checker/src/main/java/org/checkerframework/checker/collectionownership/CollectionOwnershipAnnotatedTypeFactory.java (1)
CollectionOwnershipAnnotatedTypeFactory(64-745)checker/src/main/java/org/checkerframework/checker/resourceleak/ResourceLeakUtils.java (1)
ResourceLeakUtils(40-468)
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 @coderabbitai help to get the list of available commands and usage tips.