chore(quality): drive Sonar findings to zero and close deps backlog#98
Merged
Conversation
Document version.quarkus as a project-owned pin: the resolved parent chain (cui-java-parent 1.5.4 -> cui-java-bom 1.5.4) declares no Quarkus version, so the property overrides nothing and is the single source of truth feeding the quarkus-bom import. Value left unchanged at 3.37.3. Correct CLAUDE.md parent-POM line from cui-java-parent:1.5.1 to 1.5.4 to match pom.xml line 9.
Re-declare a <year>2026</year> override for com.mycila:license-maven-plugin in the local pre-commit profile. The parent (cui-java-parent 1.5.4) omits <year> on the stated basis that mycila defaults to the current year; that is false (mycila resolves the year from <inceptionYear>, which is absent throughout the chain), so headers pinned to a stale 2022. Sweep all 168 header-carrying Java sources (107 main, 61 test) from 2022 to 2026 via license:remove followed by license:format under -Ppre-commit. Owner and email remain inherited. Verified with license:check (exit 0) and a repeated quality gate that rewrites nothing.
Production (12 findings): unnamed catch patterns (S7467) in DirectoryAssetSource, PathConfinement, UpstreamAssetSource, ConfigValidator, ConfigLoader and GatewayEdgeRoute; remove the unused private OK constant (S1068); replace a forwarding lambda with a method reference (S1612). Tests (22 findings): hoist multi-throw lambdas so each assertThrows holds a single invocation (S5778), and collapse near-duplicate tests into parameterized ones (S5976, S1612). Test structure changes only; no assertion was weakened or removed. Two S6916 findings are suppressed in-code with rationale rather than fixed: the rule proposes a when-guard, but guards attach only to pattern labels, so a guard on an enum constant label is a compile error. ADR-9 and ADR-10 sequencing and error ownership are untouched.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cuioss-oliver, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
The deliverable-4 sweep renamed unused catch parameters to the unnamed pattern (catch (X e) -> catch (X _)). Each rename touches a line inside a pre-existing uncovered error path, which drags that already-uncovered line into SonarCloud's new-code window — taking PR new_coverage to 28.57% (5 of 7 new lines to cover uncovered) and failing the gate on that one condition. Cover all five rather than reverting the renames, which would reintroduce the findings the sweep fixed: - DirectoryAssetSource#realPathWithinRoot IOException path, triggered via an in-root symlink to a held descriptor of a deleted file (stats as a regular file, but toRealPath() fails) — the TOCTOU-removal case the method must fail closed on - ConfigLoader scalar coercion falling back to TextNode when an all-digit substitution overflows Long.parseLong - ConfigValidator CIDR with a non-numeric prefix length (10.0.0.0/abc) - ConfigValidator CIDR whose address part is not an IP literal (not-an-ip/24) - GatewayEdgeRoute#parseMethod rejecting TRACE, a valid HTTP verb absent from the gateway's HttpMethod enum Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpXbDJSEs9wAns2MftaBm7
…ests The coverage commit itself surfaced two new-code smells: - java:S7466 in DirectoryAssetSourceTest - use var for the try-with-resources unnamed variable instead of naming the type; the InputStream import goes with it - java:S5976 in ConfigValidatorTest - collapse the four malformed-CIDR tests into one @ParameterizedTest. Each entry still fails parsing at a different point (no slash, out-of-range prefix, non-numeric prefix, non-literal address), which the case comment records Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpXbDJSEs9wAns2MftaBm7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drives the SonarCloud residual backlog to zero for every finding fixable within this branch, closes out the snapshot-deploy publication root-cause investigation, records the Quarkus-version pin rationale (and fixes a stale parent-POM reference in
CLAUDE.md), and sweeps every Java license header's copyright year from 2022 to 2026.Changes
integration-testsandbenchmarksmodules both suppress publication viaskipPublishing— thecentral-publishing-maven-plugin's own user property — nevermaven.deploy.skip. The parent POM already setsmaven-deploy-pluginskip=truefor every module, somaven.deploy.skipwas never a candidate switch;skipPublishingis the only correct one.pom.xml,CLAUDE.md): added an in-repo comment next toversion.quarkusinpom.xmlexplaining why the property is a project-owned pin (the resolvedde.cuioss:cui-java-parent:1.5.4chain declares no Quarkus version at all), and correctedCLAUDE.md's stale "cui-java-parent:1.5.1" line to the actual resolved1.5.4.cuioss_API-Sheriffand partitioned the 64 open issues into 34 live (real, code-fixable) and 30 stale ghost findings — all 30 under the deleted pre-renamede.cuioss.sheriff.api.**package tree, each reporting line 0.api-sheriff/src/main/java/de/cuioss/sheriff/gateway/asset/DirectoryAssetSource.java,api-sheriff/src/main/java/de/cuioss/sheriff/gateway/asset/PathConfinement.java,api-sheriff/src/main/java/de/cuioss/sheriff/gateway/asset/UpstreamAssetSource.java,api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/ResolvedAsset.java,api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidator.java,api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/load/ConfigLoader.java,api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java, and the corresponding test files (e.g.ConfigValidatorTest.java,ConfigModelContractTest.java,TokenValidatorProducerTest.java,DispatchStageTest.java,ThoroughChecksStageTest.java).api-sheriff,integration-tests, andbenchmarks, plus apom.xmlcomment recording why the parent's pre-commit profile comment ("no<year>override needed — mycila defaults to the current year") is inaccurate for this project: no<inceptionYear>is declared anywhere in this project or its parent chain, so mycila's${year}resolution regresses to a stale value (2022) rather than tracking the current year.Suppressions
Two
@SuppressWarnings("java:S6916")(each paired with an in-code// NOSONARrationale) were added:api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/ResolvedAsset.java:72api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidator.java:560Both sites are canonical-constructor / compact bodies whose invariant check switches on an enum constant. S6916 suggests replacing the
ifinside eachcasewith awhenguard, but awhenguard can only attach to a pattern label, not an enum-constant label —case DIRECTORY when ...is a compile error (verified againstjavac --release 25). The suggested remedy is inapplicable at these sites, and restructuring to a type-pattern switch would rewrite the invariant check rather than address the finding.License-header file-count note
git diff --statreports 168 Java files changed for the header-year sweep, not the 167 a naivegrep -rlcount over*.javawould suggest. This is a false negative in the grep-based count, not a missed file: one of the 168 files is binary-classified by git's diff machinery (a CRLF/encoding artifact unrelated to header content), so a text-modegrep -lcount silently skips it whilegit diff --statstill reports the change. Verification for this deliverable usedlicense:check— the mycila plugin's own header-compliance verb — rather than a grep count;license:checkinspects every file the plugin is configured against regardless of git's binary classification, so it is the stronger, authoritative signal here.Deferred (blocked on post-merge events — NOT in this PR)
Three of the plan's eight deliverables are deliberately excluded from this PR because each depends on state this PR itself has not yet produced:
main.ConfigLoaderforjson-schema-validator3.0.6 on PR chore(deps): Bump com.networknt:json-schema-validator from 1.5.9 to 3.0.6 #70's own branch (dependabot/maven/com.networknt-json-schema-validator-3.0.6), on a separate checkout outside this plan's branch and worktree. Sequenced after this PR merges, per the plan's constraint: land this PR first and rebase chore(deps): Bump com.networknt:json-schema-validator from 1.5.9 to 3.0.6 #70 on top, rather than racing the two.Documentation
No doc-tree change was made, and none is warranted: nothing in this PR changes what an operator or contributor does differently. The
CLAUDE.mdedit is a one-line factual correction (the actual resolved parent-POM version), not a doc-layer description of new behavior; thepom.xmlcomments are in-code rationale for future maintainers touching that specific property/plugin configuration, not user- or developer-facing documentation. Per the project's three-layer documentation convention, this is stated explicitly rather than silently skipped.Test Plan
verify(full reactor) — passed, confirming the deliverable-1 close-out made no file changeverify -Ppre-commit -pl api-sheriff -am— passed with zero errors/warnings after the Sonar sweep (deliverable 4)verify -Ppre-commit— passed with zero errors/warnings after the license-header sweep (deliverable 6)Generated by plan-finalize skill