feat: JdocSpecReader.streamFiles @MustBeClosed

[boolivar]

No description provided.

[qodo-code-review]

Review Summary by Qodo

Add file filtering and @MustBeClosed annotation to streamFiles

✨ Enhancement

Walkthroughs

Description

• 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

Diagram

flowchart 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"]

Loading

File Changes

1. jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java ✨ Enhancement +6/-3

Enhance file filtering and add resource management annotation

• Added @MustBeClosed annotation to streamFiles method to enforce resource management
• Enhanced file filtering in streamFiles to check for regular files using Files.isRegularFile()
• Changed .java extension matching to case-insensitive using StringUtils.endsWithIgnoreCase()
• Added imports for @MustBeClosed annotation and StringUtils from Apache Commons Lang3

jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (0)   📎 Requirement gaps (0)   🎨 UX Issues (0)

🐞\ ≡ Correctness (1) ☼ Reliability (1)

1. MustBeClosed stream not closed 🐞 ☼

Description

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.

Code

jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java[R41-45]

    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());
    }

Evidence

readSpecs() composes a stream pipeline from selectors.stream().flatMap(this::streamFiles) and
immediately collects, but does not manage closing of the streams returned by streamFiles().
streamFiles() is explicitly marked @MustBeClosed and, for DirectorySelector, returns
Files.walk(...), which holds OS resources until closed. The repository enables ErrorProne in the
build, so this pattern is likely to be reported as a MustBeClosed violation and/or create runtime
resource pressure.

jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java[41-58]
buildSrc/src/main/kotlin/jspecify-conventions.gradle.kts[4-29]
buildSrc/src/main/groovy/java-conventions.gradle[79-99]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### 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

---

2. FileSelector bypasses java filter 🐞 ≡

Description

streamFiles() filters DirectorySelector results to regular *.java files but returns FileSelector
paths unfiltered, so selecting a non-.java file can reach JavaFileParser.parse() and throw
JdocException. This makes behavior inconsistent and can break discovery unexpectedly.

Code

jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java[R50-55]

        if (selector instanceof DirectorySelector) {
            return Files.walk(((DirectorySelector) selector).getPath())
-                    .filter(path -> path.getFileName().toString().endsWith(".java"));
+                    .filter(path -> Files.isRegularFile(path) && StringUtils.endsWithIgnoreCase(path.getFileName().toString(), ".java"));
        }
        if (selector instanceof FileSelector) {
            return Stream.of(((FileSelector) selector).getPath());

Evidence

Only the DirectorySelector branch applies the new isRegularFile && endsWithIgnoreCase('.java')
filter; the FileSelector branch returns the selected path as-is. JavaFileParser.parse() throws when
parsing is unsuccessful, so a non-Java file (or unreadable path) selected via FileSelector will
propagate an exception out of readSpecs().

jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java[49-57]
jdoc-core/src/main/java/org/bool/jdoc/core/JavaFileParser.java[49-57]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`streamFiles()` applies a `.java` filter for directories but not for file selectors. This allows non-Java files to reach `JavaFileParser.parse()` and throw.

### Issue Context
The class-level intent is to read specs from selected Java files; directory traversal already enforces this after the PR change.

### Fix Focus Areas
- jdoc-junit-engine-commons/src/main/java/org/bool/jdoc/core/JdocSpecReader.java[49-57]

### Suggested fix
In the `FileSelector` branch, apply the same predicate used for directory results (regular file + `.java` case-insensitive). For example, return `Stream.of(path).filter(p -> Files.isRegularFile(p) && endsWithIgnoreCase(p.getFileName().toString(), ".java"))` (handling `getFileName()==null` if needed).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

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

[boolivar]

Stream.flatMap() closes mapped streams!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.26%. Comparing base (0b229b3) to head (1e44cd0).

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #135   +/-   ##
=========================================
  Coverage     90.26%   90.26%           
  Complexity      172      172           
=========================================
  Files            36       36           
  Lines           483      483           
  Branches         21       21           
=========================================
  Hits            436      436           
  Misses           40       40           
  Partials          7        7           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.