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

[asya-vorobeva]

When I've created this ticket I've had in mind that this unification should be applied to all the rules that use caching. Currently we already have 5 existing rules which use caching in our main branch. Correct approach would be:

• implement PR based on our main branch where we introduce this mechanism and apply to existing rules
• rebase epic-SNARJAVA-6237 to master after we merge the first PR and create PR to implement this mechanism for these Gatherer checks.

If we would plan to merge epic branch to master soon then we could implement this unification for all rules here. But I'm not sure about the timeline of this merge, as we have several follow-ups to be fixed. Also the approach with 2 different PRs looks more accurate.

The PR implementation itself looks good, only a little comment below.

[asya-vorobeva]

We use this method only with parseDocument(toContent(data), CACHE_FORMAT_VERSION) expression. Let's get rid of it and extract parseDocument(...) to the private method instead.

[gitar-bot]

Code Review ✅ Approved 11 closed / 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 closed

✅ 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 closed 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

[asya-vorobeva]

💯