SONARJAVA-6268 Fix issue in primitveType() and primitiveWrapperType() when the sema is incomplete

[GabrielFleischer]

No description provided.

[hashicorp-vault-sonar-prod]

SONARJAVA-6268

[sonar-review-alpha]

Summary

This PR fixes a null pointer risk in JType.primitiveWrapperType() and JType.primitiveType() when the semantic analyzer cannot resolve a type. Previously, both methods would pass a potentially null result from sema.resolveType() directly to sema.type() without checking. This can happen in real-world scenarios where the classpath is incomplete or semantic information is unavailable.

The fix is straightforward: extract the resolved type into a variable, check for null, and return null early if resolution fails. This mirrors the existing null-check pattern already present for the name variable in both methods.

The PR also adds five new test cases covering the happy path and the failure scenarios when semantic resolution fails.

What reviewers should know

Code changes: All logic changes are in JType.java in two methods (~137-163):

• Both primitiveWrapperType() and primitiveType() follow the same fix pattern
• The fix is defensive, not changing the happy path behavior

Testing: New tests in JTypeTest.java worth reviewing:

• primitiveWrapperType_returns_null_when_semantic_cannot_resolve_wrapper_type() and primitiveType_returns_null_when_semantic_cannot_resolve_primitive_type() are the critical ones that exercise the bug fix with mock semantics
• The other new tests validate basic behavior (primitive/non-primitive types)
• One test method was renamed (names → fullyQualifiedNamesToNames) for clarity; check that the rename is consistent

What to watch for: The tests use spy() on the semantic analyzer to force null returns. Verify that the mock setup correctly simulates the incomplete classpath scenario described in the issue.

---

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[sonar-review-alpha]

LGTM! ✅

The fix is correct and minimal. Both primitiveWrapperType() and primitiveType() now guard against a null return from sema.resolveType() before passing the result to sema.type(), preventing a NPE when the classpath is incomplete.

One behavioral nuance worth noting: when resolveType() returns null, the method exits early without populating the cache field, so subsequent calls re-enter the if-block and call resolveType() again. This is consistent with the pre-existing name == null early-return above it, and is harmless in the degraded-classpath scenario where this code path is reached.

🗣️ Give feedback

[alban-auzeill]

Good fix. The null check before passing the result of sema.resolveType(name) to sema.type(...) is the right defensive move. Even though in practice ECJ almost always returns UnknownType rather than null, the incomplete-classpath edge case is real enough to warrant this guard — a NPE in primitiveType() or primitiveWrapperType() would be a confusing failure with no obvious cause.

The test approach is pragmatic: since reproducing a half-initialized ECJ semantic model in an integration test is essentially infeasible, using a spy to simulate the resolveType returning null is the cleanest way to verify the contract. The test comments explaining why the scenario is contrived are a nice touch.

The rename of names() / @MethodSource("names") to fullyQualifiedNamesToNames() is also a welcome cleanup — the previous name was a collision waiting to confuse.

[GabrielFleischer]

Good fix. The null check before passing the result of sema.resolveType(name) to sema.type(...) is the right defensive move. Even though in practice ECJ almost always returns UnknownType rather than null, the incomplete-classpath edge case is real enough to warrant this guard — a NPE in primitiveType() or primitiveWrapperType() would be a confusing failure with no obvious cause.

The test approach is pragmatic: since reproducing a half-initialized ECJ semantic model in an integration test is essentially infeasible, using a spy to simulate the resolveType returning null is the cleanest way to verify the contract. The test comments explaining why the scenario is contrived are a nice touch.

The rename of names() / @MethodSource("names") to fullyQualifiedNamesToNames() is also a welcome cleanup — the previous name was a collision waiting to confuse.

Thanks ClaudeAlban