Skip to content

Commit e7e84f4

Browse files
benzonicoclaude
andcommitted
SONARJAVA-4698 Address PR review comments
- Remove confusing "FP:" prefix from test comments - Improve semantic analysis to use type information when available - Add compiling test cases to verify semantic analysis works correctly - When type is known, check fully qualified name; when unknown, fall back to string-based check 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 66bc890 commit e7e84f4

File tree

4 files changed

+44
-9
lines changed

4 files changed

+44
-9
lines changed

java-checks-test-sources/default/src/main/files/non-compiling/checks/SunPackagesUsedCheckSample.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@ public Object uselessMethod() {
2121

2222
// SONARJAVA-4698: False positive when variable is named "sun"
2323
public void fooWithFieldNamedSun() {
24-
sun.toString(); // Compliant - FP: "sun" is a field of type Object, not a sun.* package class
24+
sun.toString(); // Compliant - "sun" is a field of type Object, not a sun.* package class
2525
}
2626

2727
public void barWithParameterNamedSun(Object sun) {
28-
sun.toString(); // Compliant - FP: "sun" is a parameter of type Object, not a sun.* package class
28+
sun.toString(); // Compliant - "sun" is a parameter of type Object, not a sun.* package class
2929
}
3030

3131
public void bazWithLocalVariableNamedSun() {
3232
Object sun = new Object();
33-
sun.toString(); // Compliant - FP: "sun" is a local variable of type Object, not a sun.* package class
33+
sun.toString(); // Compliant - "sun" is a local variable of type Object, not a sun.* package class
3434
}
3535
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package checks;
2+
3+
class SunPackagesUsedCheckSample {
4+
private Object sun; // variable named "sun"
5+
6+
// SONARJAVA-4698: Variables named "sun" should not trigger the rule
7+
public void fooWithFieldNamedSun() {
8+
sun.toString(); // Compliant - "sun" is a field of type Object, not a sun.* package class
9+
}
10+
11+
public void barWithParameterNamedSun(Object sun) {
12+
sun.toString(); // Compliant - "sun" is a parameter of type Object, not a sun.* package class
13+
}
14+
15+
public void bazWithLocalVariableNamedSun() {
16+
Object sun = new Object();
17+
sun.toString(); // Compliant - "sun" is a local variable of type Object, not a sun.* package class
18+
}
19+
20+
// Actual sun.* package usage - should trigger the rule
21+
public void useSunMiscUnsafe() {
22+
sun.misc.Unsafe unsafe = null; // Noncompliant {{Use classes from the Java API instead of Sun classes.}}
23+
// ^^^^^^^^^^^^^^^
24+
}
25+
}

java-checks/src/main/java/org/sonar/java/checks/SunPackagesUsedCheck.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,15 @@ private static boolean isActuallySunPackage(MemberSelectExpressionTree tree) {
7979
// Check if the expression's type actually comes from a sun.* package
8080
// This prevents false positives when a variable is named "sun"
8181
var type = tree.expression().symbolType();
82-
if (type.isUnknown()) {
83-
// If we can't determine the type, we should still check to avoid missing real issues
84-
// In this case, rely on the string-based check
85-
return true;
82+
if (!type.isUnknown()) {
83+
// When we have type information, use it to determine if it's actually a sun.* package
84+
String fullyQualifiedName = type.fullyQualifiedName();
85+
return fullyQualifiedName.startsWith("sun.");
8686
}
87-
String fullyQualifiedName = type.fullyQualifiedName();
88-
return fullyQualifiedName.startsWith("sun.");
87+
// When type is unknown (non-compiling code), we can't use semantic analysis
88+
// In this case, rely on the string-based check which has already been done
89+
// This means we may miss some FPs in non-compiling code, but we'll catch them with proper semantic analysis
90+
return true;
8991
}
9092

9193
private boolean isExcluded(String reference) {

java-checks/src/test/java/org/sonar/java/checks/SunPackagesUsedCheckTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,12 @@ void check_with_exclusion() {
4040
.verifyIssues();
4141
}
4242

43+
@Test
44+
void detected_with_semantic() {
45+
CheckVerifier.newVerifier()
46+
.onFile(TestUtils.mainCodeSourcesPath("checks/SunPackagesUsedCheckSample.java"))
47+
.withCheck(new SunPackagesUsedCheck())
48+
.verifyIssues();
49+
}
50+
4351
}

0 commit comments

Comments
 (0)