jamesfredley commented on PR #15686: URL: https://github.com/apache/grails-core/pull/15686#issuecomment-4576811515
## Code-analysis status: configuration now surfaces the violations (converted to draft) I pushed a config change and converted this PR to **draft**, because once the analysis actually runs it reveals a far larger problem than expected. ### Root cause of the previous "red with no report" `pmdVersion` was `6.55.0`. PMD 6.x **cannot parse Java 21+ source**, so `pmdMain` threw `PMDException: Error while processing` on essentially every `.java` file and never produced findings. Because the PMD/SpotBugs tasks failed, `aggregateAnalysisViolations` (which only generates the Markdown report, it does not gate) never ran, so the job was red with `No files were found ... build/reports/violations/`. Two changes in the latest commit: 1. **`pmdVersion` 6.55.0 → 7.25.0** - supports Java 21 through 26 (the Micronaut/Forge modules compile on Java 25). 2. **Code-analysis CI runs report-only** (`-Pgrails.codeanalysis.ignoreFailures=true`) so PMD + SpotBugs complete, the aggregate report is generated, and the full violation list is published to the job summary instead of aborting. ### The scope problem With PMD parsing correctly, the **Core build alone** reports: - **5,016 PMD violations** - **187 SpotBugs violations** The Forge and Gradle-plugin builds are not measured yet - realistic total is **~8,000-12,000 violations**. Top PMD rules: | Count | Rule | Nature | | ---: | :--- | :--- | | 1,758 | MissingOverride | trivial/safe (`@Override`) | | 282 | LooseCoupling | mechanical | | 264 | AvoidCatchingGenericException | behavior-risky | | 247 | AvoidLiteralsInIfCondition | stylistic | | 239 | AvoidReassigningParameters | behavior-risky | | 203 | LiteralsFirstInComparisons | mechanical | | 171 | ConstructorCallsOverridableMethod | behavior-risky | | 150 | CloseResource | behavior-risky | | 147 | AssignmentInOperand | mixed | | 140 | UseVarargs | mixed | Top SpotBugs bug patterns: | Count | Pattern | | ---: | :--- | | 46 | EC_UNRELATED_TYPES_USING_POINTER_EQUALITY | | 21 | DM_DEFAULT_ENCODING | | 20 | RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE | | 16 | MS_SHOULD_BE_FINAL | | 12 | RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT | The ruleset currently enables the **entire** `bestpractices` + `errorprone` + `security` categories, which is why the count is so high - a large share are pure style preferences, and many of the rest require behavior-changing edits to framework internals. ### Recommendation Fixing 8,000-12,000 locations across the framework in one PR is neither reviewable nor safely mergeable. Suggested path before this can gate: 1. Curate the PMD ruleset down to a high-signal subset (and review the SpotBugs `reportLevel`/`effort`). 2. Bulk-fix the safe mechanical rules (e.g. the 1,758 `MissingOverride`). 3. Baseline or defer the behavior-risky rules to follow-up PRs. > Note: PR #15687 (grails-jacoco) is stacked on top of this branch and inherits these jobs, so it is affected by the same decision. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
