SONARJAVA-7164 Implement rule S9437 - "getInstance()" methods should not return a new instance on every call

[rombirli]

Implements S9437, specified in https://github.com/SonarSource/rspec/pull/8339.

The rule raises when a static no-argument getInstance method has a body consisting only of return new <OwnType>(...);, so nothing is cached and every caller gets a separate object.

Scope decisions, matching the RSPEC:

• Only the no-argument form is reported. A parameterised getInstance(...) is a factory keyed on its arguments, following the JDK convention.
• Lazily initialised accessors are not reported, since their body reads or assigns a static field.
• Factory methods named create, newInstance, of or valueOf are not reported.
• Returning an anonymous subclass or an unrelated type is not reported.

Primary location is the instance creation, with a secondary location on the method name.

[hashicorp-vault-sonar-prod]

SONARJAVA-7164

[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

[gitar-bot]

Code Review ✅ Approved 3 closed / 3 findings

🟡 Medium risk · Adding S9437 changes analyzer findings for no-argument static getInstance methods; imperfect matching could flag intentional fresh-instance factories.

Implements rule S9437 to flag static no-argument getInstance methods whose body only returns new <OwnType>(...), so callers stop receiving a fresh object on every call. Resolved review feedback covers a static native test case for the null-body branch, erasure-based own-class checks for generic and raw instantiations, and corrected rule description wording. No issues remain open.

✅ 3 closed

✅ Quality: 'WithoutBody' sample has a body, so the null-block branch is untested

📄 java-checks-test-sources/default/src/main/java/checks/GetInstanceReturningNewInstanceCheckSample.java:155-165 📄 java-checks/src/main/java/org/sonar/java/checks/GetInstanceReturningNewInstanceCheck.java:75
The WithoutBody interface case is named after a bodiless method, but its getInstance() has a body (return null;). The only truly bodiless method, AbstractGetInstance.getInstance(), is not static, so it returns early at the isStatic() guard. This means the body == null branch in onlyStatementInstantiation is never run by the tests. Adding a static bodiless case such as static native Foo getInstance(); would cover that branch.

✅ Edge Case: Own-class check misses generic and raw instantiations

📄 java-checks/src/main/java/org/sonar/java/checks/GetInstanceReturningNewInstanceCheck.java:94-97
instantiatesOwnClass compares the created type's symbol to the method owner with JSymbol.equals. For type symbols that goes through JType.areEqual, which also compares type parameters, type arguments and raw-ness. In a generic class Box<T>, the owner is the generic declaration: it has type parameters [T] and no type arguments. Inside static <U> Box<U> getInstance() { return new Box<>(); }, new Box<>() gives a parameterized binding Box<U>. That binding has no type parameters and has [U] as type arguments, so the symbols are not equal. A raw new Box() also differs because isRawType is true for it. As a result, generic classes are never reported, and the sample file has no generic case. Comparing erasures avoids this, e.g. instantiation.symbolType().erasure().equals(method.symbol().owner().type().erasure()).

✅ Quality: Rule description says issues need no static field read

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9437.html:10-11 📄 java-checks/src/main/java/org/sonar/java/checks/GetInstanceReturningNewInstanceCheck.java:67-71
The rule HTML says an issue is raised only when the method returns a new instance "without reading or assigning any static field". The Javadoc says "Anything else includes reading or assigning a field". The check doesn't look at field accesses at all. It only requires the body to be a single return new Own(...), so return new Foo(DEFAULT_CONFIG); reads a static field and is still reported (correctly, since it is still a fresh instance). The user-facing description and the Javadoc state a condition the code doesn't implement. Describe the actual condition instead: an unconditional single return new <OwnType>(...) statement.

Verification Repos 5 inspected

Functional validation No results

Rules No rules evaluated

Cross-repo 5 repositories inspected 0 open impacts

SonarSource/sonarlint-language-server Inspected
SonarSource/sonarlint-vscode Inspected
SonarSource/sonar-lits Inspected
SonarSource/sonarqube Inspected
SonarSource/sonarqube-webapp Inspected

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

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}

[rombirli]

/pvf

[hashicorp-vault-sonar-prod]

📊 Performance Validation complete
🆕 3 new · 🗑️ 0 lost issues
View dashboard