feat: JdocSpecReader.streamFiles file filter, @MustBeClosed annotation#135
feat: JdocSpecReader.streamFiles file filter, @MustBeClosed annotation#135boolivar wants to merge 2 commits into
Conversation
Review Summary by QodoAdd file filtering and @MustBeClosed annotation to streamFiles
WalkthroughsDescription• Add @MustBeClosed annotation to streamFiles method • Improve file filtering with regular file check • Use case-insensitive .java extension matching • Add Apache Commons Lang3 dependency import Diagramflowchart LR
A["streamFiles method"] --> B["Add @MustBeClosed annotation"]
A --> C["Enhance file filter logic"]
C --> D["Check Files.isRegularFile"]
C --> E["Case-insensitive .java check"]
File Changes1. jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java
|
Code Review by Qodo
|
| public List<SpecSource> readSpecs(List<DiscoverySelector> selectors) { | ||
| return selectors.stream() | ||
| .flatMap(this::streamFiles).distinct() | ||
| .map(parser::parse).collect(Collectors.toList()); | ||
| .flatMap(this::streamFiles).distinct().map(parser::parse) | ||
| .collect(Collectors.toList()); | ||
| } |
There was a problem hiding this comment.
1. Mustbeclosed stream not closed 🐞 Bug ☼ Reliability
JdocSpecReader.streamFiles() is annotated @MustBeClosed but readSpecs() flatMaps it without explicitly closing anything, which can violate ErrorProne’s MustBeClosed contract and may leak Files.walk resources on some execution paths (e.g., exceptions). This can fail the build (ErrorProne enabled) or exhaust file handles during discovery.
Agent Prompt
### Issue description
`streamFiles()` returns a `Files.walk()` stream and is annotated `@MustBeClosed`, but the current `readSpecs()` pipeline does not explicitly manage closing. With ErrorProne enabled, this can be flagged as a `@MustBeClosed` contract violation, and it also risks leaking file descriptors if streams aren’t closed.
### Issue Context
- `Files.walk()` returns a resource-backed `Stream<Path>`.
- The project enables ErrorProne via build conventions.
- `@MustBeClosed` communicates (and can enforce) that the returned value must be closed by the caller.
### Fix Focus Areas
- jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java[41-58]
### Suggested direction
Pick one of these approaches:
1) **Refactor to avoid returning a close-requiring stream**: for `DirectorySelector`, use `try (Stream<Path> walk = Files.walk(...)) { return walk.filter(...).toList().stream(); }` so the resource is closed before returning.
2) **Remove `@MustBeClosed`** if relying on downstream closure semantics is intentional and verified.
3) **Inline directory walking into `readSpecs()`** with explicit `try-with-resources` around `Files.walk()` per directory selector, and merge results into a single `Stream`/`List`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Stream.flatMap() closes mapped streams!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #135 +/- ##
============================================
- Coverage 90.22% 90.02% -0.21%
- Complexity 172 173 +1
============================================
Files 36 36
Lines 481 481
Branches 21 21
============================================
- Hits 434 433 -1
Misses 40 40
- Partials 7 8 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.