SONARJAVA-6947 Share an interface between checks that require caching

[aurelien-coet-sonarsource]

---

Summary by Gitar

• New caching infrastructure:
  • Added FileCachingCheck mixin interface for JSON-based per-file data caching
• Refactored checks for caching:
  • Updated ComponentScanPackageGatherer, and BeanDefinitionGatherer to implement FileCachingCheck

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

SONARJAVA-6947

[gitar-bot]

Code Review ✅ Approved 11 resolved / 11 findings

Refactoring to share a caching interface between checks that require per-file data caching. Introduces FileCachingCheck mixin and JsonCacheFormat for standardized JSON-based caching, updating SpringBeansShouldBeAccessibleCheck, ComponentScanPackageGatherer, and BeanDefinitionGatherer to use the new infrastructure. Multiple documentation and implementation issues were addressed: javadoc corrections for exception types, logging behavior, and API descriptions; fixes for cache read I/O error handling, exception labeling in writeToCache, double-counting in restoreFromCache, and stray characters. No issues remain.

✅ 11 resolved

✅ Quality: JsonCacheFormat javadoc promises IAE but throws other types

📄 java-frontend/src/main/java/org/sonar/java/caching/JsonCacheFormat.java:41-44 📄 java-frontend/src/main/java/org/sonar/java/caching/JsonCacheFormat.java:72-74 📄 java-frontend/src/main/java/org/sonar/java/caching/JsonCacheFormat.java:76 📄 java-frontend/src/main/java/org/sonar/java/caching/JsonCacheFormat.java:188-196 📄 java-frontend/src/test/java/org/sonar/java/caching/JsonCacheFormatTest.java:160-169 📄 java-frontend/src/test/java/org/sonar/java/caching/JsonCacheFormatTest.java:241-247
The class javadoc states "Every accessor below is strict: anything unexpected — a missing property, a wrong JSON type, a fractional number where an integer is required — throws IllegalArgumentException", and requiredInt/parseDocument repeat that with explicit @throws IllegalArgumentException tags. Neither holds: requiredInt ends in getAsBigDecimal().intValueExact(), which throws ArithmeticException for 1.5 or 99999999999 (asserted in JsonCacheFormatTest:167-168 and :245-246), and parseDocument lets Gson's JsonSyntaxException escape for non-JSON input (JsonCacheFormatTest:82-86 only asserts RuntimeException). Today FileCachingCheck.readFromCache catches RuntimeException so nothing leaks, but this is a new public utility documented for rule authors, and the code it replaced (ExcessiveContentRequestCheck.loadFromPreviousAnalysis) caught exactly IllegalArgumentException — a caller following the javadoc would abort the analysis on a corrupt entry. Either normalize the thrown type or correct the docs.

✅ Quality: "Cache problems are never fatal" is false for read I/O errors

📄 java-frontend/src/main/java/org/sonar/java/caching/FileCachingCheck.java:41-42 📄 java-frontend/src/main/java/org/sonar/java/caching/FileCachingCheck.java:104-109 📄 java-checks/src/test/java/org/sonar/java/checks/UselessPackageInfoCheckTest.java:168-170 📄 java-checks/src/test/java/org/sonar/java/checks/spring/SpringBeansShouldBeAccessibleCheckTest.java:297-299
The interface javadoc asserts "Cache problems are never fatal", but readFromCache calls cacheContext.getReadCache().readBytes(cacheKey) outside any try block, and JavaReadCacheImpl.readBytes throws CacheReadException when the underlying stream fails (JavaReadCacheImpl.java:57). The tests retained by this PR confirm the fatal path: cache_deserialization_throws_IOException in both UselessPackageInfoCheckTest and SpringBeansShouldBeAccessibleCheckTest still assert AnalysisException with an IOException root cause. Either bring the read call inside the failure handling so an I/O error degrades to a cache miss like the other cases, or narrow the javadoc claim to the three cases it actually covers.

✅ Quality: writeToCache's catch covers serialize(), mislabelling its failures

📄 java-frontend/src/main/java/org/sonar/java/caching/FileCachingCheck.java:85-90
serialize(data) is evaluated inside the try whose catch (IllegalArgumentException e) logs "Tried to write multiple times to cache key '{}'". IllegalArgumentException is the signal the write cache uses for a duplicate key, but an implementor's serialize can also raise it — and then the entry is silently never written while the log blames a colliding write, so the file is re-parsed on every subsequent analysis with no usable diagnostic. Compute the bytes before entering the try so the handler only covers the cache call it is written for.

✅ Quality: Stray '≥' character in projectToNames javadoc @return

📄 java-frontend/src/main/java/org/sonar/java/model/springcontext/BeanDefinitionGatherer.java:258
The reflowed javadoc on projectToNames ends with @return The name of each dependency, mapped by type.≥ — a stray ≥ introduced while reformatting the comment in this PR. It renders into the generated documentation as garbage.

✅ Quality: Fallback javadoc says "same class" but suppression is file-wide

📄 java-frontend/src/main/java/org/sonar/java/utils/SpringUtils.java:351-357 📄 java-frontend/src/main/java/org/sonar/java/model/springcontext/ComponentScanPackageGatherer.java:128-141
springBootApplicationScanPackages's javadoc states the useOwnPackageAsFallback flag "lets a caller suppress it when scanning is already configured by another annotation on the same class", but the only caller that passes false derives it from packagesCollectedAtFileLevel.isEmpty() — file-level state, not class-level. Trigger: one file declaring class A with @ComponentScan("com.foo") and a second top-level class B with a bare @SpringBootApplication; A fills packagesCollectedAtFileLevel, so B's own package is never registered for scanning even though nothing on B configures scanning. The gatherer's own javadoc describes the real, file-scoped rule ("once something already contributed a package for this file"), so the two docs added in this diff contradict each other; align the SpringUtils wording with the file-scoped behaviour (or scope the flag per class at the call site).

...and 6 more resolved from earlier reviews

Review coverage

Functional validation No results

Rules No rules evaluated

Auto-approval Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
2 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube