Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 8 additions & 22 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,16 @@ Open work items for this repo. Cross-cutting tracking lives in
items here are streambuffer-specific or are this repo's slice of a
cross-cutting initiative.

## Open

- **jqwik pin policy** — see [`../workspace/policies/jqwik-prompt-injection.md`](../workspace/policies/jqwik-prompt-injection.md). `jqwik.version ≤ 1.9.3` is mandatory.

- **`@VisibleForTesting` audit.** No usages currently. Walk the production tree for package-private/protected methods or fields that exist purely so tests can reach them, and either annotate (`com.google.common.annotations.VisibleForTesting`) or move into the test source tree.
**Completed work is not recorded here.** It lives in git history and in
`crossrepostatus.md`; a finished item is deleted from this file rather than
annotated, so everything below is genuinely still open.

- **Null-safety refinement.** JSpecify + NullAway are enforced at compile time in **strict JSpecify mode** with the following extra options: `CheckOptionalEmptiness`, `AcknowledgeRestrictiveAnnotations`, `AcknowledgeAndroidRecent`, `AssertsEnabled` (see `pom.xml`). The package carries an explicit `@NullMarked` via `package-info.java`. The production code currently has no `@Nullable` markers because every value is non-null by construction (constructors reject `null`, no `return null` sites). Open follow-up: as new public API surfaces are added, evaluate whether `@Nullable` or `Optional<T>` would be more precise than the implicit non-null default.

- **SpotBugs `effort=Max` + `threshold=Low`** — ✅ **enforced at the gate** (`4374dea` + `e7e254a`). `pom.xml` `<effort>Max</effort>` + `<threshold>Low</threshold>`; `spotbugs:check` is part of `mvn verify` and fails on any unsuppressed finding. All findings were fixed at source (added `toString()`, contextful exception messages) — no project-wide suppressions. sb was the first sibling repo to reach the Max+Low gate.
## Open

- **No LogCaptor smoke test needed** — this module has no logging code (`org.slf4j.*` not used in `src/main/java/`). If logging is ever introduced, add a LogCaptor smoke test at the same time so the binding/configuration is exercised in tests.
- **jqwik pin policy** — see [`../workspace/policies/jqwik-prompt-injection.md`](../workspace/policies/jqwik-prompt-injection.md). `jqwik.version ≤ 1.9.3` is mandatory. A standing constraint, not a task: it has to be re-checked whenever the dependency is bumped.

- **Cross-repo code-quality TODOs** — see [`../workspace/policies/code-quality-todos.md`](../workspace/policies/code-quality-todos.md) for the canonical `@VisibleForTesting` design-fit review, package hierarchy review, and class/method naming review. This module is single-package and has no `@VisibleForTesting` usages; the package and naming reviews remain open.
- **`@VisibleForTesting` audit.** `StreamBuffer` has **15** package-private methods that exist so tests can reach them (`decideTrimExecution`, `shouldTrim`, `clampToMaxInt`, `decrementAvailableBytesBudget`, `calculateResultingChunks`, the five `shouldSkipTrim*`/`should*` predicates, `isAvailableBytesPositive`, `isMaxAllocSizeLessThanAvailable`, `shouldCheckEdgeCase`, `recordReadStatistics`, `shouldUpdateMaxObservedBytes`, `updateMaxObservedBytesIfNeeded`). None is annotated, and Guava is not a dependency here — so closing this means deciding between (a) adding a project-local marker annotation, which puts a new public type into the API surface of a deliberately one-class library, and (b) recording that the convention does not apply to this repo. Decide and act; do not leave it as a permanently open audit.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor wording issue: The text says "the five shouldSkipTrim*/should* predicates" but there are only four shouldSkipTrim* methods in the codebase:

  • shouldSkipTrimDueToEdgeCase
  • shouldSkipTrimDueToInvalidMaxBufferElements
  • shouldSkipTrimDueToSmallBuffer
  • shouldSkipTrimDueToSufficientBuffer

Suggest changing "five" to "four" for accuracy, or if the count was meant to include additional predicates, clarify which ones are grouped under "the five" vs. the separately-listed methods.


## Done (kept for history)
- **Null-safety refinement.** JSpecify + NullAway are enforced at compile time in **strict JSpecify mode** with the extra options `CheckOptionalEmptiness`, `AcknowledgeRestrictiveAnnotations`, `AcknowledgeAndroidRecent`, `AssertsEnabled` (see `pom.xml`); the package carries an explicit `@NullMarked` via `package-info.java`. The production code has no `@Nullable` markers because every value is non-null by construction (constructors reject `null`, no `return null` sites). Open follow-up: as new public API surfaces are added, evaluate whether `@Nullable` or `Optional<T>` would be more precise than the implicit non-null default.

- **Error Prone bug-pattern promotions to `ERROR`** — `ad95d66` (12 patterns promoted).
- **`javac -Werror` + `-Xlint:all,-serial,-options,-classfile,-processing`** — `7a4fbf0`. ElementType.MODULE blocker resolved by the module-level `@NullMarked` move.
- **`-parameters` javac arg** — `912f14b`.
- **`--release N`** instead of `-source N -target N` — `912f14b`.
- **Mutation-testing threshold enforcement (PIT)** — 100 % over the whole package.
- **Checker Framework as a second static-nullness pass** — `5a9be1b`.
- **JPMS `module-info.java`** — exports `net.ladenthin.streambuffer`; two-execution `maven-compiler-plugin` pattern (release 8 sources, release 9 module-info); the resulting jar carries `module-info.class` at its root and is backward-compatible with Java 8 classpath consumers. Module-level `@NullMarked` was intentionally NOT added — the per-package `package-info.java` annotation already covers the same nullness scope.
- **Banned-API enforcement** — Maven Enforcer (`c0148c8`); ArchUnit `Thread.sleep` / `System.exit` / `new Random` bans (`eaf4337`).
- **ArchUnit additions** — public-fields-final (`5dd816d`), internal-JDK banned-imports (`de29bd4`), `noTestFrameworksInProduction` + `noPackageCycles` (`bbdb505`). Full `layeredArchitecture` is N/A: this module is a single-package library.
- **Abstract the Java and test writing guidelines to a workspace-level shared layer.** Workspace version chain at [`../workspace/guides/src/CODE_WRITING_GUIDE-8.md`](../workspace/guides/src/CODE_WRITING_GUIDE-8.md) and [`../workspace/guides/test/TEST_WRITING_GUIDE-8.md`](../workspace/guides/test/TEST_WRITING_GUIDE-8.md). Canonical TDD skill at [`../workspace/.claude/skills/java-tdd-guide/SKILL.md`](../workspace/.claude/skills/java-tdd-guide/SKILL.md). This repo has no project-specific writing-guide supplements (production code is a single class).
- **Standardised CLAUDE.md template** — [`../workspace/templates/CLAUDE.md.template`](../workspace/templates/CLAUDE.md.template).
- **Cross-repo code-quality TODOs** — see [`../workspace/policies/code-quality-todos.md`](../workspace/policies/code-quality-todos.md) for the canonical `@VisibleForTesting` design-fit review, package hierarchy review, and class/method naming review. This module is single-package, so the package review is trivially satisfied; the naming review is still open.
Loading