SONARJAVA-6525 Decrease FPs for the rule S117

[asya-vorobeva]

Local variables inside methods may now use leading or trailing underscores (e.g. _good, good_, __myVar__), as a signal that the variable is temporary, unimportant, or intentionally unnamed.
The core name between underscores is still validated against the configured format. The leniency does not apply to method parameters or fields.

[hashicorp-vault-sonar-prod]

SONARJAVA-6525

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Updates S117 rule to allow leading and trailing underscores in local variable names, but fails to exclude constructor parameters from this leniency, causing incorrect validation logic.

✅ 1 resolved

✅ Bug: Constructor parameters wrongly get underscore leniency

📄 java-checks/src/main/java/org/sonar/java/checks/naming/BadLocalVariableNameCheck.java:95-97
The new logic decides whether to strip leading/trailing underscores using boolean isMethodParameter = parent != null && parent.is(Tree.Kind.METHOD);. In the sonar-java AST a constructor is a MethodTree whose Tree.Kind is CONSTRUCTOR, not METHOD (Tree.java:108 vs 113). As a result, a constructor parameter's parent reports kind CONSTRUCTOR, so isMethodParameter is false and stripLeadingTrailingUnderscores is applied to it.

This contradicts the stated intent ("The leniency does not apply to method parameters or fields"): a constructor parameter such as MyClass(int _myParam) or MyClass(int _BAD_) would have its underscores stripped before the format check, so names that should be reported are silently accepted — a false negative. Method parameters and constructor parameters should be treated identically.

Fix: also treat Tree.Kind.CONSTRUCTOR parents as parameters (and consider adding a non-compiling test for a constructor parameter with a leading underscore).

Options

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

Comment with these commands to change:

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 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[francois-mora-sonarsource]

There is a gap between the documented exception and the implementation. The RSPEC PR says the underscore leniency applies only to local variables declared inside a method body, and that method parameters are not affected. The current implementation instead strips leading/trailing underscores from any visited variable that is not a method or constructor parameter, which includes named lambda parameters such as _name -> _name. This makes lambda parameters compliant even though they are not covered by the documented exception. I do not think we need to assume here whether lambda parameters should or should not be exempt; either direction can be valid, but the implementation and documentation should make the same choice explicitly.