From e9633975d6a44415a0ae6af24b940aa78a416208 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 13:04:38 +0200 Subject: [PATCH 01/10] Replace "if" statement with pattern match guard Reported by Sonar S6916 --- .../java/checks/CompilationOrPreparationInLoopCheck.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java index 8dfa122b5fd..db19387cbcf 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java @@ -200,10 +200,9 @@ public void visitAssignmentExpression(AssignmentExpressionTree tree) { public void visitUnaryExpression(UnaryExpressionTree tree) { super.visitUnaryExpression(tree); switch (tree.kind()) { - case POSTFIX_INCREMENT, POSTFIX_DECREMENT, PREFIX_INCREMENT, PREFIX_DECREMENT -> { - if (tree.expression().is(Tree.Kind.IDENTIFIER)) { - names.add(((IdentifierTree) tree.expression()).name()); - } + case POSTFIX_INCREMENT, POSTFIX_DECREMENT, PREFIX_INCREMENT, PREFIX_DECREMENT + when tree.expression().is(Tree.Kind.IDENTIFIER) -> { + names.add(((IdentifierTree) tree.expression()).name()); } default -> { // not a mutation From 11bd08eb0cf323b311836a927b6131da089e5507 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 13:56:35 +0200 Subject: [PATCH 02/10] Remove "continue" statements and revert unsupported "when" guard Replace continue statements with inverted conditions in HashCodeMismatchedFieldsCheck and LocalVariablesShouldNotSpanSwitchCaseGroupsCheck. Revert the pattern match guard in CompilationOrPreparationInLoopCheck which used an unsupported "when" syntax, restoring the original "if" statement. Co-Authored-By: Claude Opus 4.6 --- .../CompilationOrPreparationInLoopCheck.java | 7 +++-- .../checks/HashCodeMismatchedFieldsCheck.java | 28 +++++++++---------- ...lesShouldNotSpanSwitchCaseGroupsCheck.java | 11 ++++---- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java index db19387cbcf..8dfa122b5fd 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java @@ -200,9 +200,10 @@ public void visitAssignmentExpression(AssignmentExpressionTree tree) { public void visitUnaryExpression(UnaryExpressionTree tree) { super.visitUnaryExpression(tree); switch (tree.kind()) { - case POSTFIX_INCREMENT, POSTFIX_DECREMENT, PREFIX_INCREMENT, PREFIX_DECREMENT - when tree.expression().is(Tree.Kind.IDENTIFIER) -> { - names.add(((IdentifierTree) tree.expression()).name()); + case POSTFIX_INCREMENT, POSTFIX_DECREMENT, PREFIX_INCREMENT, PREFIX_DECREMENT -> { + if (tree.expression().is(Tree.Kind.IDENTIFIER)) { + names.add(((IdentifierTree) tree.expression()).name()); + } } default -> { // not a mutation diff --git a/java-checks/src/main/java/org/sonar/java/checks/HashCodeMismatchedFieldsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/HashCodeMismatchedFieldsCheck.java index e2ca285604e..11726053b3d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/HashCodeMismatchedFieldsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/HashCodeMismatchedFieldsCheck.java @@ -118,15 +118,14 @@ private static EqualsAndHashCode find(ClassTree classTree) { MethodTree hashCodeMethod = null; List otherMethods = new ArrayList<>(); for (Tree member : classTree.members()) { - if (!(member instanceof MethodTree methodTree) || methodTree.block() == null) { - continue; - } - if (MethodTreeUtils.isEqualsMethod(methodTree)) { - equalsMethod = methodTree; - } else if (MethodTreeUtils.isHashCodeMethod(methodTree)) { - hashCodeMethod = methodTree; - } else { - otherMethods.add(methodTree); + if (member instanceof MethodTree methodTree && methodTree.block() != null) { + if (MethodTreeUtils.isEqualsMethod(methodTree)) { + equalsMethod = methodTree; + } else if (MethodTreeUtils.isHashCodeMethod(methodTree)) { + hashCodeMethod = methodTree; + } else { + otherMethods.add(methodTree); + } } } if (equalsMethod == null || hashCodeMethod == null) { @@ -140,12 +139,11 @@ private static Map> collectHelperFields(S Map> fieldsByHelper = new HashMap<>(); for (MethodTree helper : otherMethods) { Symbol.MethodSymbol helperSymbol = helper.symbol(); - if (helperSymbol.isUnknown() || !helper.parameters().isEmpty()) { - continue; - } - ReadAndAssignedFields helperFields = collectReadFields(helper, owner, Map.of(), Role.HELPER); - if (!helperFields.failed()) { - fieldsByHelper.put(helperSymbol, helperFields.readFields()); + if (!helperSymbol.isUnknown() && helper.parameters().isEmpty()) { + ReadAndAssignedFields helperFields = collectReadFields(helper, owner, Map.of(), Role.HELPER); + if (!helperFields.failed()) { + fieldsByHelper.put(helperSymbol, helperFields.readFields()); + } } } return fieldsByHelper; diff --git a/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java index 7896e744823..46517468300 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java @@ -51,12 +51,11 @@ public void visitNode(Tree tree) { List caseGroups = ((SwitchTree) tree).cases(); for (int index = 0; index < caseGroups.size(); index++) { CaseGroupTree caseGroup = caseGroups.get(index); - if (!caseGroup.labels().get(0).isFallThrough()) { - continue; - } - for (StatementTree statement : caseGroup.body()) { - if (statement instanceof VariableTree variable) { - reportIfAccessedFromLaterGroup(variable, caseGroups, index + 1); + if (caseGroup.labels().get(0).isFallThrough()) { + for (StatementTree statement : caseGroup.body()) { + if (statement instanceof VariableTree variable) { + reportIfAccessedFromLaterGroup(variable, caseGroups, index + 1); + } } } } From 0d0992cf9be5266d9a372f19040f4ce0d48b6be7 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 15:28:43 +0200 Subject: [PATCH 03/10] S9345: Make classes final Add final keyword to classes that have no subclasses: - InternalSyntaxTrivia - HardCodedSecretCheck - SmapFile - Jasper.ServletContext Co-Authored-By: Claude Opus 4.6 --- .../main/java/org/sonar/java/checks/HardCodedSecretCheck.java | 2 +- .../main/java/org/sonar/java/model/InternalSyntaxTrivia.java | 2 +- java-frontend/src/main/java/org/sonar/java/model/SmapFile.java | 2 +- java-jsp/src/main/java/org/sonar/java/jsp/Jasper.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/HardCodedSecretCheck.java b/java-checks/src/main/java/org/sonar/java/checks/HardCodedSecretCheck.java index 19cef5bfb1e..eada3b0ec50 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/HardCodedSecretCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/HardCodedSecretCheck.java @@ -35,7 +35,7 @@ import static org.sonar.java.checks.HardcodedIpCheck.IP_V6_ALONE; @Rule(key = "S6418") -public class HardCodedSecretCheck extends AbstractHardCodedCredentialChecker { +public final class HardCodedSecretCheck extends AbstractHardCodedCredentialChecker { private static final String DEFAULT_SECRET_WORDS = "api[_.-]?key,auth,credential,secret,token"; private static final String DEFAULT_RANDOMNESS_SENSIBILITY= "5.0"; diff --git a/java-frontend/src/main/java/org/sonar/java/model/InternalSyntaxTrivia.java b/java-frontend/src/main/java/org/sonar/java/model/InternalSyntaxTrivia.java index e34d9442ad3..d7f341bc29c 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/InternalSyntaxTrivia.java +++ b/java-frontend/src/main/java/org/sonar/java/model/InternalSyntaxTrivia.java @@ -24,7 +24,7 @@ import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.TreeVisitor; -public class InternalSyntaxTrivia extends JavaTree implements SyntaxTrivia { +public final class InternalSyntaxTrivia extends JavaTree implements SyntaxTrivia { private final CommentKind commentKind; diff --git a/java-frontend/src/main/java/org/sonar/java/model/SmapFile.java b/java-frontend/src/main/java/org/sonar/java/model/SmapFile.java index 877fc90e4bf..d19e7b18335 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/SmapFile.java +++ b/java-frontend/src/main/java/org/sonar/java/model/SmapFile.java @@ -43,7 +43,7 @@ *

* We expect only single JSP stratum, with single FileSection and LineSection. Moreover only single file is expected in FileSection */ -public class SmapFile { +public final class SmapFile { private static final Pattern LINE_INFO = Pattern.compile("(?\\d+)" + "(?:#(?\\d+))?" + diff --git a/java-jsp/src/main/java/org/sonar/java/jsp/Jasper.java b/java-jsp/src/main/java/org/sonar/java/jsp/Jasper.java index 42cf2bc1b24..54f3bfab15a 100644 --- a/java-jsp/src/main/java/org/sonar/java/jsp/Jasper.java +++ b/java-jsp/src/main/java/org/sonar/java/jsp/Jasper.java @@ -236,7 +236,7 @@ static Path outputDir(SensorContext sensorContext) { /** * Overloading log methods so messages are redirected to scanner log */ - static class ServletContext extends JspCServletContext { + static final class ServletContext extends JspCServletContext { public ServletContext(URL aResourceBaseURL, ClassLoader classLoader) throws JasperException { super(/* not used */ null, aResourceBaseURL, classLoader, false, true); From e50c188cef4c459964af6afdb915460b091d8bd7 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 15:29:23 +0200 Subject: [PATCH 04/10] S6880: Replace if/else chains with switch expressions Convert instanceof if/else chains to Java 21+ pattern-matching switch expressions in 6 locations for improved readability. Co-Authored-By: Claude Opus 4.6 --- .../java/checks/DateTimeConversionsCheck.java | 12 ++++----- .../java/checks/PatternMatchUsingIfCheck.java | 24 +++++++++-------- .../java/checks/TryWithResourcesCheck.java | 12 ++++----- ...tilityClassWithPublicConstructorCheck.java | 15 +++++------ .../java/checks/helpers/StringUtils.java | 13 ++++----- .../org/sonar/java/model/JSymbolMetadata.java | 27 +++++++++---------- 6 files changed, 47 insertions(+), 56 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java index f7fee6daeee..afe6f41ca06 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java @@ -99,12 +99,12 @@ private static boolean isLocalDateOrTime(Type type) { private static ExpressionTree skipParenthesesAndCasts(ExpressionTree expression) { ExpressionTree result = expression; while (true) { - if (result instanceof ParenthesizedTree parenthesizedTree) { - result = parenthesizedTree.expression(); - } else if (result instanceof TypeCastTree typeCastTree) { - result = typeCastTree.expression(); - } else { - return result; + switch (result) { + case ParenthesizedTree parenthesizedTree -> result = parenthesizedTree.expression(); + case TypeCastTree typeCastTree -> result = typeCastTree.expression(); + default -> { + return result; + } } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/PatternMatchUsingIfCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PatternMatchUsingIfCheck.java index ce1d538a9cb..18942fb45df 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PatternMatchUsingIfCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PatternMatchUsingIfCheck.java @@ -217,18 +217,20 @@ private JavaQuickFix computeQuickFix(List cases, IfStatementTree topLevelI } private void writeCase(Case caze, StringBuilder sb, int baseIndent, boolean canLiftReturn) { - if (caze instanceof PatternMatchCase patternMatchCase) { - sb.append("case ").append(QuickFixHelper.contentForTree(patternMatchCase.pattern, context)); - if (!patternMatchCase.guards().isEmpty()) { - List guards = patternMatchCase.guards(); - sb.append(" when "); - join(guards, " && ", sb); + switch (caze) { + case PatternMatchCase patternMatchCase -> { + sb.append("case ").append(QuickFixHelper.contentForTree(patternMatchCase.pattern, context)); + if (!patternMatchCase.guards().isEmpty()) { + List guards = patternMatchCase.guards(); + sb.append(" when "); + join(guards, " && ", sb); + } } - } else if (caze instanceof EqualityCase equalityCase) { - sb.append("case "); - join(equalityCase.constants, ", ", sb); - } else { - sb.append("default"); + case EqualityCase equalityCase -> { + sb.append("case "); + join(equalityCase.constants, ", ", sb); + } + default -> sb.append("default"); } sb.append(" -> "); if (canLiftReturn) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java index 5696f4be99f..d85cf3cd341 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java @@ -97,15 +97,13 @@ public void visitNode(Tree tree) { } private static boolean isNewAutocloseableOrBuilder(Tree tree, JavaFileScannerContext context) { - if (tree instanceof NewClassTree newClass) { - return newClass.symbolType().isSubtypeOf("java.lang.AutoCloseable"); - } else if (tree instanceof MethodInvocationTree mit) { - return AUTOCLOSEABLE_FACTORY_MATCHER.matches(mit) || + return switch (tree) { + case NewClassTree newClass -> newClass.symbolType().isSubtypeOf("java.lang.AutoCloseable"); + case MethodInvocationTree mit -> AUTOCLOSEABLE_FACTORY_MATCHER.matches(mit) || (context.getJavaVersion().isJava21Compatible() && AUTOCLOSEABLE_JAVA21_MATCHER.matches(mit)) || (context.getJavaVersion().isJava26Compatible() && AUTOCLOSEABLE_JAVA26_MATCHER.matches(mit)); - } else { - return false; - } + default -> false; + }; } private static boolean isFollowedByTryWithFinally(Tree tree) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java index 22fa4f5384a..9f80f174d32 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java @@ -128,15 +128,12 @@ private static boolean hasPublicAccess(AnnotationTree annotation) { } private static boolean isAccessLevelNotPublic(ExpressionTree tree) { - String valueName; - if (tree instanceof MemberSelectExpressionTree mset) { - valueName = mset.identifier().name(); - } else if (tree instanceof IdentifierTree identifier) { - valueName = identifier.name(); - } else { - return false; - } - return !"PUBLIC".equals(valueName); + String valueName = switch (tree) { + case MemberSelectExpressionTree mset -> mset.identifier().name(); + case IdentifierTree identifier -> identifier.name(); + default -> null; + }; + return valueName != null && !"PUBLIC".equals(valueName); } private static List computeQuickFixes(ClassTree classTree) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java index 2c2e755afcb..9def709ec63 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java +++ b/java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java @@ -60,14 +60,11 @@ public static int countMatches(@Nullable String string, @Nullable String pattern public static String[] flatten(Object ... args) { List result = new ArrayList<>(); for (Object arg : args) { - if (arg instanceof String s) { - result.add(s); - } else if (arg instanceof String[] arr) { - Collections.addAll(result, arr); - } else if (arg instanceof Collection col) { - result.addAll((Collection) col); - } else { - throw new IllegalArgumentException("Unsupported argument type: " + arg.getClass()); + switch (arg) { + case String s -> result.add(s); + case String[] arr -> Collections.addAll(result, arr); + case Collection col -> result.addAll((Collection) col); + default -> throw new IllegalArgumentException("Unsupported argument type: " + arg.getClass()); } } return result.toArray(new String[0]); diff --git a/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java b/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java index 7c1bb897713..488771bde49 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java @@ -408,22 +408,19 @@ public List values() { } private Object convertAnnotationValue(Object value) { - if (value instanceof IVariableBinding iVariableBinding) { - return sema.variableSymbol(iVariableBinding); - } else if (value instanceof ITypeBinding iTypeBinding) { - return sema.typeSymbol(iTypeBinding); - } else if (value instanceof IAnnotationBinding iAnnotationBinding) { - return sema.annotation(iAnnotationBinding); - } else if (value instanceof Object[] a) { - // Godin: probably better to not modify original array - Object[] result = new Object[a.length]; - for (int i = 0; i < a.length; i++) { - result[i] = convertAnnotationValue(a[i]); + return switch (value) { + case IVariableBinding iVariableBinding -> sema.variableSymbol(iVariableBinding); + case ITypeBinding iTypeBinding -> sema.typeSymbol(iTypeBinding); + case IAnnotationBinding iAnnotationBinding -> sema.annotation(iAnnotationBinding); + case Object[] a -> { + Object[] result = new Object[a.length]; + for (int i = 0; i < a.length; i++) { + result[i] = convertAnnotationValue(a[i]); + } + yield result; } - return result; - } else { - return value; - } + default -> value; + }; } } From a2bf73e533da8b06ad009ff96b8c469bfdc1a16c Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 15:30:14 +0200 Subject: [PATCH 05/10] S9357: Convert anonymous classes to lambdas Replace anonymous implementations of functional interfaces with lambda expressions in 8 locations across test files. Co-Authored-By: Claude Opus 4.6 --- .../com/sonar/it/java/suite/TestUtils.java | 10 +++------- .../internal/JavaCheckVerifierTest.java | 20 +++++++------------ .../java/DefaultJavaResourceLocatorTest.java | 6 +----- .../sonar/java/ast/JavaAstScannerTest.java | 18 +++++------------ .../org/sonar/java/model/JParserTest.java | 13 ++---------- 5 files changed, 18 insertions(+), 49 deletions(-) diff --git a/its/plugin/tests/src/test/java/com/sonar/it/java/suite/TestUtils.java b/its/plugin/tests/src/test/java/com/sonar/it/java/suite/TestUtils.java index 45255fbfdbc..a781cdb0026 100644 --- a/its/plugin/tests/src/test/java/com/sonar/it/java/suite/TestUtils.java +++ b/its/plugin/tests/src/test/java/com/sonar/it/java/suite/TestUtils.java @@ -23,7 +23,6 @@ import com.sonar.orchestrator.container.Server; import com.sonar.orchestrator.junit4.OrchestratorRule; import java.io.File; -import java.io.FilenameFilter; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -57,12 +56,9 @@ public static File homeDir() { } public static File pluginJar(String artifactId) { - return Iterables.getOnlyElement(Arrays.asList(new File(homeDir(), "plugins/" + artifactId + "/target/").listFiles(new FilenameFilter() { - @Override - public boolean accept(File dir, String name) { - return name.endsWith(".jar") && !name.endsWith("-sources.jar"); - } - }))); + return Iterables.getOnlyElement(Arrays.asList(new File(homeDir(), "plugins/" + artifactId + "/target/").listFiles( + (dir, name) -> name.endsWith(".jar") && !name.endsWith("-sources.jar") + ))); } public static File projectDir(String projectName) { diff --git a/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java b/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java index 6b9ff77a6cc..5baac685309 100644 --- a/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java +++ b/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java @@ -245,12 +245,9 @@ void context_return_good_root_working_directory() { assertThatCode(() -> { JavaCheckVerifier.newInstance() .onFile(TEST_FILE) - .withCheck(new JavaFileScanner() { - @Override - public void scanFile(JavaFileScannerContext context) { - assertThat(context.getRootProjectWorkingDirectory().getPath()).isEqualTo(rootWorkDir); - } - }) + .withCheck((JavaFileScanner) context -> + assertThat(context.getRootProjectWorkingDirectory().getPath()).isEqualTo(rootWorkDir) + ) .withProjectLevelWorkDir(rootWorkDir) .verifyNoIssues(); }).doesNotThrowAnyException(); @@ -432,13 +429,10 @@ void compilationUnitModifier_modify_tree() { classTree.complete((ModifiersTreeImpl) classTree.modifiers(), classTree.declarationKeyword(), ident); }; - var check = new JavaFileScanner() { - @Override - public void scanFile(JavaFileScannerContext context) { - CompilationUnitTree tree = context.getTree(); - ClassTreeImpl classTree = (ClassTreeImpl) tree.types().get(0); - assertThat(classTree.simpleName().name()).isEqualTo("Modified"); - } + var check = (JavaFileScanner) context -> { + CompilationUnitTree tree = context.getTree(); + ClassTreeImpl classTree = (ClassTreeImpl) tree.types().get(0); + assertThat(classTree.simpleName().name()).isEqualTo("Modified"); }; JavaCheckVerifier.newInstance() diff --git a/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java b/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java index d01c4f3c101..304e792cece 100644 --- a/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java +++ b/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java @@ -127,11 +127,7 @@ interface I { private void method() { class B { - Object obj = new I() { - @Override - public void foo() { - // empty implementation - } + Object obj = (I) () -> { }; } } diff --git a/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java b/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java index 00ff3723a2c..6102da4c3c5 100644 --- a/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java +++ b/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java @@ -164,12 +164,7 @@ void should_not_fail_whole_analysis_upon_parse_error_and_notify_audit_listeners( @Test void should_handle_analysis_cancellation() { - JavaFileScanner visitor = spy(new JavaFileScanner() { - @Override - public void scanFile(JavaFileScannerContext context) { - JavaAstScannerTest.this.context.setCancelled(true); - } - }); + JavaFileScanner visitor = spy((JavaFileScanner) context -> JavaAstScannerTest.this.context.setCancelled(true)); scanTwoFilesWithVisitor(visitor, false, false); @@ -451,13 +446,10 @@ void scanWithoutParsing_filters_out_the_files_that_could_be_successfully_scanned @Test void test_modifyCompilationUnit_modify_ast() { - var check = new JavaFileScanner() { - @Override - public void scanFile(JavaFileScannerContext context) { - CompilationUnitTree tree = context.getTree(); - ClassTreeImpl classTree = (ClassTreeImpl) tree.types().get(0); - assertThat(classTree.simpleName().symbol().isUnknown()).isTrue(); - } + var check = (JavaFileScanner) context -> { + CompilationUnitTree tree = context.getTree(); + ClassTreeImpl classTree = (ClassTreeImpl) tree.types().get(0); + assertThat(classTree.simpleName().symbol().isUnknown()).isTrue(); }; VisitorsBridge visitorsBridge = new VisitorsBridge( diff --git a/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java b/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java index 57d8dfeb19b..8c6cf1cbad7 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java @@ -856,18 +856,9 @@ private void assertResultsOfParsing(List results, List inputFiles = Arrays.asList(TestUtils.inputFile("src/test/files/metrics/Classes.java"), TestUtils.inputFile("src/test/files/metrics/Methods.java")); - BiConsumer action = spy(new BiConsumer() { - @Override - public void accept(InputFile inputFile, JParserConfig.Result result) { - // Do nothing - } - }); - BooleanSupplier isCanceled = spy(new BooleanSupplier() { - @Override - public boolean getAsBoolean() { - return false; - } + BiConsumer action = spy((BiConsumer) (inputFile, result) -> { }); + BooleanSupplier isCanceled = spy((BooleanSupplier) () -> false); FILE_BY_FILE .create(MAXIMUM_SUPPORTED_JAVA_VERSION, DEFAULT_CLASSPATH) From 093b4f6bb54661285e5c15389e55ff44844d5623 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 15:30:37 +0200 Subject: [PATCH 06/10] S6916, S6485, S6878: Miscellaneous code improvements - S6916: Use pattern matching instanceof in CompilationOrPreparationInLoopCheck - S6485: Use HashMap.newHashMap() in AnnotationFieldReferenceFinder - S6878: Use record pattern in SpelExpressionCheck Co-Authored-By: Claude Opus 4.6 --- .../java/checks/CompilationOrPreparationInLoopCheck.java | 4 ++-- .../org/sonar/java/checks/spring/SpelExpressionCheck.java | 4 ++-- .../checks/unused/utils/AnnotationFieldReferenceFinder.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java index 8dfa122b5fd..1e293d0cddc 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java @@ -201,8 +201,8 @@ public void visitUnaryExpression(UnaryExpressionTree tree) { super.visitUnaryExpression(tree); switch (tree.kind()) { case POSTFIX_INCREMENT, POSTFIX_DECREMENT, PREFIX_INCREMENT, PREFIX_DECREMENT -> { - if (tree.expression().is(Tree.Kind.IDENTIFIER)) { - names.add(((IdentifierTree) tree.expression()).name()); + if (tree.expression() instanceof IdentifierTree identifier) { + names.add(identifier.name()); } } default -> { diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java index 3a677ba3f22..89e78aad884 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java @@ -246,8 +246,8 @@ static Placeholder parse(ParseCtx ctx, int startIdx) { } else { state = new DefaultValue(0, expr, idx + 1); } - } else if (state instanceof DefaultValue d && current == '}' && d.nestingLevel == 0) { - return new Placeholder(ctx.offset(), new Range(startIdx, idx + 1), d.expr, expressionSource.substring(d.startDefault, idx).trim()); + } else if (state instanceof DefaultValue(int nestingLevel, String expr, int startDefault) && current == '}' && nestingLevel == 0) { + return new Placeholder(ctx.offset(), new Range(startIdx, idx + 1), expr, expressionSource.substring(startDefault, idx).trim()); } else if (state instanceof DefaultValue d) { if (SpEL.matchPrefix(expressionSource, idx)) { SpEL.parse(ctx, idx); diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/utils/AnnotationFieldReferenceFinder.java b/java-checks/src/main/java/org/sonar/java/checks/unused/utils/AnnotationFieldReferenceFinder.java index f7c12a44233..ab706332f1f 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/utils/AnnotationFieldReferenceFinder.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/utils/AnnotationFieldReferenceFinder.java @@ -72,7 +72,7 @@ private AnnotationFieldReferenceFinder(HashMap fieldNameTo * Constructs an instance of this visitor that looks for references of the given fields inside annotations. */ public static AnnotationFieldReferenceFinder findReferencesTo(Collection fields) { - var fieldNameToVariableTree = new HashMap(fields.size()); + var fieldNameToVariableTree = HashMap.newHashMap(fields.size()); for (var variable : fields) { var fieldName = variable.simpleName().name(); From 6302572f92b8b3c7017a694ae4ea5ce0868396ea Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 15:31:08 +0200 Subject: [PATCH 07/10] S9358: Move conditional expressions inside operations Extract ternary expressions into intermediate variables to improve readability in 5 locations. Co-Authored-By: Claude Opus 4.6 --- ...ssertThrowsInsteadOfTryCatchFailCheck.java | 34 +++++++------------ ...roidMobileDatabaseEncryptionKeysCheck.java | 4 +-- .../java/org/sonar/java/model/JParser.java | 13 ++++--- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java b/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java index f90048c68f9..df56716e9fe 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java @@ -169,15 +169,11 @@ private Replacements junitReplacement( ", %s".formatted(contentFor(argument)) ).orElse(""); - return isTryBlock ? - new Replacements( - "assertThrows(%s, () -> ".formatted(typeClass(firstCaughtTypeInTry(tryStatement))), - "%s);".formatted(argumentsSuffix) - ) : - new Replacements( - "assertDoesNotThrow(() -> ", - "%s);".formatted(argumentsSuffix) - ); + String prefix = isTryBlock + ? "assertThrows(%s, () -> ".formatted(typeClass(firstCaughtTypeInTry(tryStatement))) + : "assertDoesNotThrow(() -> "; + String suffix = "%s);".formatted(argumentsSuffix); + return new Replacements(prefix, suffix); } private Replacements assertJReplacement( @@ -185,18 +181,14 @@ private Replacements assertJReplacement( TryStatementTree tryStatement, boolean isTryBlock ) { - var failureMessagePart = failArguments.isEmpty() ? - "" : - ".withFailMessage(%s)".formatted(contentFor(failArguments.get(0))); - return isTryBlock ? - new Replacements( - "assertThatCode(() -> ", - ")%s.isInstanceOf(%s);".formatted(failureMessagePart, typeClass(firstCaughtTypeInTry(tryStatement))) - ) : - new Replacements( - "assertThatCode(() -> ", - ")%s.doesNotThrowAnyException();".formatted(failureMessagePart) - ); + var failureMessagePart = failArguments.isEmpty() + ? "" + : ".withFailMessage(%s)".formatted(contentFor(failArguments.get(0))); + String prefix = "assertThatCode(() -> "; + String suffix = isTryBlock + ? ")%s.isInstanceOf(%s);".formatted(failureMessagePart, typeClass(firstCaughtTypeInTry(tryStatement))) + : ")%s.doesNotThrowAnyException();".formatted(failureMessagePart); + return new Replacements(prefix, suffix); } private String contentFor(Tree tree) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/AndroidMobileDatabaseEncryptionKeysCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/AndroidMobileDatabaseEncryptionKeysCheck.java index d26a20ee86b..a9cd3d379bc 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/AndroidMobileDatabaseEncryptionKeysCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/AndroidMobileDatabaseEncryptionKeysCheck.java @@ -92,8 +92,8 @@ public void visitNode(Tree tree) { private void reportIssueIfHardCoded(MethodInvocationTree mit, String argName) { Arguments arguments = mit.arguments(); - ExpressionTree passwordArg = arguments.size() == 1 ? arguments.get(0) : arguments.get(1); - reportIssueIfHardCoded(passwordArg, argName); + int argIndex = arguments.size() == 1 ? 0 : 1; + reportIssueIfHardCoded(arguments.get(argIndex), argName); } private void reportIssueIfHardCoded(ExpressionTree expressionTree, String messageArg) { diff --git a/java-frontend/src/main/java/org/sonar/java/model/JParser.java b/java-frontend/src/main/java/org/sonar/java/model/JParser.java index fbaf8af9307..7e80b322777 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JParser.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JParser.java @@ -976,9 +976,10 @@ private EnumConstantTreeImpl processEnumConstantDeclaration(EnumConstantDeclarat final InternalSyntaxToken closeParToken; if (tokenManager.get(openParTokenIndex).tokenType == TerminalToken.TokenNameLPAREN) { openParToken = createSyntaxToken(openParTokenIndex); - closeParToken = e.arguments().isEmpty() - ? firstTokenAfter(e.getName(), TerminalToken.TokenNameRPAREN) - : firstTokenAfter((ASTNode) e.arguments().get(e.arguments().size() - 1), TerminalToken.TokenNameRPAREN); + ASTNode closeParAnchor = e.arguments().isEmpty() + ? e.getName() + : (ASTNode) e.arguments().get(e.arguments().size() - 1); + closeParToken = firstTokenAfter(closeParAnchor, TerminalToken.TokenNameRPAREN); } else { openParToken = null; closeParToken = null; @@ -2737,9 +2738,11 @@ private JavaTree.WildcardTreeImpl convertWildcardType(WildcardType e) { if (bound == null) { t = new JavaTree.WildcardTreeImpl(questionToken); } else { + Tree.Kind wildcardKind = e.isUpperBound() ? Tree.Kind.EXTENDS_WILDCARD : Tree.Kind.SUPER_WILDCARD; + TerminalToken boundTokenType = e.isUpperBound() ? TerminalToken.TokenNameextends : TerminalToken.TokenNamesuper; t = new JavaTree.WildcardTreeImpl( - e.isUpperBound() ? Tree.Kind.EXTENDS_WILDCARD : Tree.Kind.SUPER_WILDCARD, - e.isUpperBound() ? firstTokenBefore(bound, TerminalToken.TokenNameextends) : firstTokenBefore(bound, TerminalToken.TokenNamesuper), + wildcardKind, + firstTokenBefore(bound, boundTokenType), convertType(bound) ).complete(questionToken); } From 22910aba332c3d72fcfdcbf2b983eec8911efbfd Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 15:35:26 +0200 Subject: [PATCH 08/10] S909: Remove continue statements Replace continue statements with inverted conditions across 28 files. For each occurrence, the if-continue pattern is replaced by inverting the condition and wrapping the remaining loop body inside the if block. Co-Authored-By: Claude Opus 4.6 --- ...dCodedCredentialsShouldNotBeUsedCheck.java | 5 +- .../verifier/internal/Expectations.java | 36 +++++----- .../internal/InternalCheckVerifier.java | 19 +++--- .../java/checks/AbstractPrintfChecker.java | 20 +++--- .../checks/ConditionalRuleCacheUtils.java | 39 ++++++----- .../checks/EqualsMismatchedMembersCheck.java | 18 +++-- ...tatelessGatherersOmitInitializerCheck.java | 28 ++++---- .../sonar/java/checks/HardcodedURICheck.java | 13 +--- .../checks/IdenticalCasesInSwitchCheck.java | 32 +++++---- .../checks/IncorrectOrderOfMembersCheck.java | 16 ++--- .../InterfaceAsConstantContainerCheck.java | 5 +- .../checks/LoopExecutingAtMostOnceCheck.java | 20 +++--- ...rridesInRecordWithArrayComponentCheck.java | 19 +++--- .../java/checks/ModifiersOrderCheck.java | 15 ++--- .../java/checks/OptionalAsParameterCheck.java | 12 ++-- .../sonar/java/checks/PseudoRandomCheck.java | 17 +++-- .../RedundantThrowsDeclarationCheck.java | 33 +++++---- ...witchCasesShouldBeCommaSeparatedCheck.java | 28 ++++---- ...fSequentialForSequentialGathererCheck.java | 27 ++++---- .../SerialVersionUidInRecordCheck.java | 13 ++-- .../MissingPathVariableAnnotationCheck.java | 67 +++++++------------ .../RedundantSpringAnnotationCheck.java | 19 +++--- ...ConfigurationWithAutowiredFieldsCheck.java | 8 +-- .../checks/tests/ParameterizedTestCheck.java | 38 +++++------ .../checks/unused/UnusedTestRuleCheck.java | 6 +- .../org/sonar/java/cfg/LiveVariables.java | 7 +- .../org/sonar/java/classpath/JavaSdkUtil.java | 22 +++--- .../org/sonar/java/model/JMethodSymbol.java | 18 ++--- 28 files changed, 265 insertions(+), 335 deletions(-) diff --git a/java-checks-aws/src/main/java/org/sonar/java/checks/security/HardCodedCredentialsShouldNotBeUsedCheck.java b/java-checks-aws/src/main/java/org/sonar/java/checks/security/HardCodedCredentialsShouldNotBeUsedCheck.java index 3a463607b95..8a13c5e2146 100644 --- a/java-checks-aws/src/main/java/org/sonar/java/checks/security/HardCodedCredentialsShouldNotBeUsedCheck.java +++ b/java-checks-aws/src/main/java/org/sonar/java/checks/security/HardCodedCredentialsShouldNotBeUsedCheck.java @@ -111,10 +111,9 @@ private void checkArguments(Arguments arguments, CredentialMethod method) { var secondaryLocations = new ArrayList(); if (isExpressionDerivedFromPlainText(argument, secondaryLocations, new HashSet<>())) { String value = ExpressionsHelper.getConstantValueAsString(argument).value(); - if (value != null && SecretClassifier.isKnownNonSecret(value)) { - continue; + if (value == null || !SecretClassifier.isKnownNonSecret(value)) { + reportIssue(argument, ISSUE_MESSAGE, secondaryLocations, null); } - reportIssue(argument, ISSUE_MESSAGE, secondaryLocations, null); } } } diff --git a/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/Expectations.java b/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/Expectations.java index 17fb7a0bd41..70e4170a634 100644 --- a/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/Expectations.java +++ b/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/Expectations.java @@ -448,26 +448,24 @@ void consolidateQuickFixes() { List quickFixesForIssue = new ArrayList<>(); for (String quickFixId : entry.getValue()) { - if (NO_QUICK_FIX_ID.equals(quickFixId)) { - // When the id corresponds to the "no quick fix id", it means that we expect no quick fix for this issue. - continue; + if (!NO_QUICK_FIX_ID.equals(quickFixId)) { + allQuickFixIds.add(quickFixId); + String message = quickfixesMessages.get(quickFixId); + if (message == null) { + throw new AssertionError("Missing message for quick fix: " + quickFixId); + } + List edits = quickfixesEdits.get(quickFixId); + if (edits == null) { + throw new AssertionError("Missing edits for quick fix: " + quickFixId); + } + + JavaQuickFix javaQuickFix = JavaQuickFix.newQuickFix(message).addTextEdits( + edits.stream() + .map(edit -> getEdit(edit, issueTextSpan, quickFixId)) + .toList() + ).build(); + quickFixesForIssue.add(javaQuickFix); } - allQuickFixIds.add(quickFixId); - String message = quickfixesMessages.get(quickFixId); - if (message == null) { - throw new AssertionError("Missing message for quick fix: " + quickFixId); - } - List edits = quickfixesEdits.get(quickFixId); - if (edits == null) { - throw new AssertionError("Missing edits for quick fix: " + quickFixId); - } - - JavaQuickFix javaQuickFix = JavaQuickFix.newQuickFix(message).addTextEdits( - edits.stream() - .map(edit -> getEdit(edit, issueTextSpan, quickFixId)) - .toList() - ).build(); - quickFixesForIssue.add(javaQuickFix); } quickFixes.put(issueTextSpan, quickFixesForIssue); } diff --git a/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifier.java b/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifier.java index c069b6c5631..ac3f8474c7d 100644 --- a/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifier.java +++ b/java-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/InternalCheckVerifier.java @@ -659,18 +659,15 @@ public void accept(Set issues) { for (AnalyzerMessage issue : issues) { AnalyzerMessage.TextSpan primaryLocation = issue.primaryLocation(); List expected = expectedQuickFixes.get(primaryLocation); - if (expected == null) { - // We don't have to always test quick fixes, we do nothing if there is no expected quick fix. - continue; - } - List actual = actualQuickFixes.get(primaryLocation); - if (expected.isEmpty()) { - if (actual != null && !actual.isEmpty()) { - throw new AssertionError(String.format("[Quick Fix] Issue on line %d contains quick fixes while none where expected", primaryLocation.startLine)); + if (expected != null) { + List actual = actualQuickFixes.get(primaryLocation); + if (expected.isEmpty()) { + if (actual != null && !actual.isEmpty()) { + throw new AssertionError(String.format("[Quick Fix] Issue on line %d contains quick fixes while none where expected", primaryLocation.startLine)); + } + } else { + validateIfSameSize(expected, actual, issue); } - // Else: no issue in both expected and actual, nothing to do - } else { - validateIfSameSize(expected, actual, issue); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/AbstractPrintfChecker.java b/java-checks/src/main/java/org/sonar/java/checks/AbstractPrintfChecker.java index cd783f0c69c..975d8d5bde0 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/AbstractPrintfChecker.java +++ b/java-checks/src/main/java/org/sonar/java/checks/AbstractPrintfChecker.java @@ -201,17 +201,17 @@ protected List getParameters(String formatString, MethodInvocationTree m while (matcher.find()) { if (firstArgumentIsLT(params, matcher.group(2))) { reportMissingPrevious(mit); - continue; - } - StringBuilder param = new StringBuilder(); - for (int groupIndex : new int[] {1, 2, 5, 6}) { - if (matcher.group(groupIndex) != null) { - param.append(matcher.group(groupIndex)); + } else { + StringBuilder param = new StringBuilder(); + for (int groupIndex : new int[] {1, 2, 5, 6}) { + if (matcher.group(groupIndex) != null) { + param.append(matcher.group(groupIndex)); + } + } + String specifier = param.toString(); + if(!"%".equals(specifier)) { + params.add(specifier); } - } - String specifier = param.toString(); - if(!"%".equals(specifier)) { - params.add(specifier); } } return params; diff --git a/java-checks/src/main/java/org/sonar/java/checks/ConditionalRuleCacheUtils.java b/java-checks/src/main/java/org/sonar/java/checks/ConditionalRuleCacheUtils.java index 7cc7b26cefe..41a56c6d0cb 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/ConditionalRuleCacheUtils.java +++ b/java-checks/src/main/java/org/sonar/java/checks/ConditionalRuleCacheUtils.java @@ -112,27 +112,26 @@ static CachedFileData deserialize(byte[] data) { int issueCount = Integer.parseInt(header[1]); List issues = new ArrayList<>(); for (int i = 1; i < lines.length; i++) { - if (lines[i].isEmpty()) { - continue; + if (!lines[i].isEmpty()) { + String[] parts = lines[i].split("\\|", -1); + int sl = Integer.parseInt(parts[0]); + int sc = Integer.parseInt(parts[1]); + int el = Integer.parseInt(parts[2]); + int ec = Integer.parseInt(parts[3]); + String message = new String(dec.decode(parts[4]), StandardCharsets.UTF_8); + String replacement = new String(dec.decode(parts[5]), StandardCharsets.UTF_8); + boolean hasImport = "1".equals(parts[6]); + ImportEditData importEdit = null; + if (hasImport) { + int isl = Integer.parseInt(parts[7]); + int isc = Integer.parseInt(parts[8]); + int iel = Integer.parseInt(parts[9]); + int iec = Integer.parseInt(parts[10]); + String importRepl = new String(dec.decode(parts[11]), StandardCharsets.UTF_8); + importEdit = new ImportEditData(isl, isc, iel, iec, importRepl); + } + issues.add(new CachedIssue(sl, sc, el, ec, message, replacement, importEdit)); } - String[] parts = lines[i].split("\\|", -1); - int sl = Integer.parseInt(parts[0]); - int sc = Integer.parseInt(parts[1]); - int el = Integer.parseInt(parts[2]); - int ec = Integer.parseInt(parts[3]); - String message = new String(dec.decode(parts[4]), StandardCharsets.UTF_8); - String replacement = new String(dec.decode(parts[5]), StandardCharsets.UTF_8); - boolean hasImport = "1".equals(parts[6]); - ImportEditData importEdit = null; - if (hasImport) { - int isl = Integer.parseInt(parts[7]); - int isc = Integer.parseInt(parts[8]); - int iel = Integer.parseInt(parts[9]); - int iec = Integer.parseInt(parts[10]); - String importRepl = new String(dec.decode(parts[11]), StandardCharsets.UTF_8); - importEdit = new ImportEditData(isl, isc, iel, iec, importRepl); - } - issues.add(new CachedIssue(sl, sc, el, ec, message, replacement, importEdit)); } return new CachedFileData(totalCount, issueCount, issues); } diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java index 47657662d3a..8bebc47b819 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java @@ -98,17 +98,15 @@ public void visitNode(Tree tree) { ComparisonCollector collector = new ComparisonCollector(owner); methodTree.block().accept(collector); for (ComparisonSite comparison : collector.comparisons) { - // Order-independent equality: (a, b) || (b, a) in the same statement is not a mismatch. - if (collector.pairsByStatement.get(comparison.statement).contains(comparison.pair().reversed())) { - continue; + if (!collector.pairsByStatement.get(comparison.statement).contains(comparison.pair().reversed())) { + reportIssue( + comparison.tree, + String.format(ISSUE_MESSAGE, comparison.thisMember.displayName, comparison.otherMember.displayName), + List.of( + new JavaFileScannerContext.Location(SECONDARY_THIS, comparison.thisMember.tree), + new JavaFileScannerContext.Location(SECONDARY_OTHER, comparison.otherMember.tree)), + null); } - reportIssue( - comparison.tree, - String.format(ISSUE_MESSAGE, comparison.thisMember.displayName, comparison.otherMember.displayName), - List.of( - new JavaFileScannerContext.Location(SECONDARY_THIS, comparison.thisMember.tree), - new JavaFileScannerContext.Location(SECONDARY_OTHER, comparison.otherMember.tree)), - null); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java index aef0bfb474e..0544777335d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java @@ -94,21 +94,19 @@ public void visitNode(Tree tree) { MethodInvocationTree mit = (MethodInvocationTree) tree; for (Case c : CASES) { - if (!c.matcher.matches(mit)) { - continue; - } - - var argPredicate = c.pred; - var issues = argPredicate.predicate.apply(mit.arguments().get(argPredicate.argIdx)); - if (!issues.isEmpty()) { - - var secondaries = issues.subList(1, issues.size()) - .stream() - .map(element -> new JavaFileScannerContext.Location("", element)) - .toList(); - - context.reportIssue(this, issues.get(0), c.msg, secondaries, null); - return; + if (c.matcher.matches(mit)) { + var argPredicate = c.pred; + var issues = argPredicate.predicate.apply(mit.arguments().get(argPredicate.argIdx)); + if (!issues.isEmpty()) { + + var secondaries = issues.subList(1, issues.size()) + .stream() + .map(element -> new JavaFileScannerContext.Location("", element)) + .toList(); + + context.reportIssue(this, issues.get(0), c.msg, secondaries, null); + return; + } } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/HardcodedURICheck.java b/java-checks/src/main/java/org/sonar/java/checks/HardcodedURICheck.java index d78de95b441..48af76f2686 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/HardcodedURICheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/HardcodedURICheck.java @@ -118,17 +118,10 @@ public void leaveFile(JavaFileScannerContext context) { } for(VariableData v : hardCodedUri) { - // equals to an identifier with unknown semantic, we cannot compare their symbols - if (idNamesWithoutSemantic.contains(v.identifier())) { - continue; + if (!idNamesWithoutSemantic.contains(v.identifier()) + && !(idNamesWithSemantic.contains(v.identifier()) && idSymbols.contains(v.symbol()))) { + reportHardcodedURI(v.initializer()); } - - // idNamesWithSemantic is used to only compare the symbols when their string identifier are the same - // as comparing symbols is costly - if (idNamesWithSemantic.contains(v.identifier()) && idSymbols.contains(v.symbol())) { - continue; - } - reportHardcodedURI(v.initializer()); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/IdenticalCasesInSwitchCheck.java b/java-checks/src/main/java/org/sonar/java/checks/IdenticalCasesInSwitchCheck.java index 695f60a3a14..2f219f746db 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/IdenticalCasesInSwitchCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/IdenticalCasesInSwitchCheck.java @@ -85,14 +85,13 @@ protected Map> checkSwitchStatement(SwitchTree Set duplicates = new HashSet<>(); for (CaseGroupTree caseGroupTree : cases) { index++; - if (duplicates.contains(caseGroupTree)) { - continue; - } - for (int i = index; i < cases.size(); i++) { - CaseGroupTree caseI = cases.get(i); - if (SyntacticEquivalence.areEquivalent(caseGroupTree.body(), caseI.body())) { - duplicates.add(caseI); - identicalBranches.computeIfAbsent(caseGroupTree, k -> new HashSet<>()).add(caseI); + if (!duplicates.contains(caseGroupTree)) { + for (int i = index; i < cases.size(); i++) { + CaseGroupTree caseI = cases.get(i); + if (SyntacticEquivalence.areEquivalent(caseGroupTree.body(), caseI.body())) { + duplicates.add(caseI); + identicalBranches.computeIfAbsent(caseGroupTree, k -> new HashSet<>()).add(caseI); + } } } } @@ -125,15 +124,14 @@ private static IfElseChain collectIdenticalBranches(List allBranc IfElseChain ifElseChain = new IfElseChain(); Set duplicates = new HashSet<>(); for (int i = 0; i < allBranches.size(); i++) { - if (duplicates.contains(allBranches.get(i))) { - continue; - } - for (int j = i + 1; j < allBranches.size(); j++) { - StatementTree statement1 = allBranches.get(i); - StatementTree statement2 = allBranches.get(j); - if (SyntacticEquivalence.areEquivalentIncludingSameVariables(statement1, statement2)) { - duplicates.add(statement2); - ifElseChain.branches.computeIfAbsent(statement1, k -> new HashSet<>()).add(statement2); + if (!duplicates.contains(allBranches.get(i))) { + for (int j = i + 1; j < allBranches.size(); j++) { + StatementTree statement1 = allBranches.get(i); + StatementTree statement2 = allBranches.get(j); + if (SyntacticEquivalence.areEquivalentIncludingSameVariables(statement1, statement2)) { + duplicates.add(statement2); + ifElseChain.branches.computeIfAbsent(statement1, k -> new HashSet<>()).add(statement2); + } } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/IncorrectOrderOfMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/IncorrectOrderOfMembersCheck.java index 4ef43722d7a..db97a503ccd 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/IncorrectOrderOfMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/IncorrectOrderOfMembersCheck.java @@ -44,8 +44,8 @@ public void visitClass(ClassTree tree) { int prev = 0; for (int i = 0; i < tree.members().size(); i++) { final Tree member = tree.members().get(i); - final int priority; - IdentifierTree identifier; + int priority = -1; + IdentifierTree identifier = null; if (member.is(Tree.Kind.VARIABLE)) { VariableTree variable = ((VariableTree) member); if (variable.symbol().isStatic()) { @@ -60,13 +60,13 @@ public void visitClass(ClassTree tree) { } else if (member.is(Tree.Kind.METHOD)) { priority = 3; identifier = ((MethodTree) member).simpleName(); - } else { - continue; } - if (priority < prev) { - context.reportIssue(this, identifier, "Move this " + NAMES[priority] + " to comply with Java Code Conventions."); - } else { - prev = priority; + if (identifier != null) { + if (priority < prev) { + context.reportIssue(this, identifier, "Move this " + NAMES[priority] + " to comply with Java Code Conventions."); + } else { + prev = priority; + } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/InterfaceAsConstantContainerCheck.java b/java-checks/src/main/java/org/sonar/java/checks/InterfaceAsConstantContainerCheck.java index 7be9d479bf6..e7b8871e863 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/InterfaceAsConstantContainerCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/InterfaceAsConstantContainerCheck.java @@ -53,10 +53,9 @@ private static List collectConstantsLocation(Cl // the interface doesn't hold only constants return Collections.emptyList(); } - if (member.is(Tree.Kind.EMPTY_STATEMENT)) { - continue; + if (!member.is(Tree.Kind.EMPTY_STATEMENT)) { + constantLocations.add(new JavaFileScannerContext.Location("", ((VariableTree) member).simpleName())); } - constantLocations.add(new JavaFileScannerContext.Location("", ((VariableTree) member).simpleName())); } return constantLocations; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/LoopExecutingAtMostOnceCheck.java b/java-checks/src/main/java/org/sonar/java/checks/LoopExecutingAtMostOnceCheck.java index 8ce88221c58..98052349dc6 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/LoopExecutingAtMostOnceCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/LoopExecutingAtMostOnceCheck.java @@ -160,18 +160,14 @@ private static boolean hasPredecessorInBlock(CFG.Block block, Tree loop) { } else { Tree predecessorFirstElement = predecessorElements.get(0); - if (isForStatementInitializer(predecessorFirstElement, loop)) { - // skip 'for' loops initializers - continue; - } - - if (isForStatementUpdate(predecessorFirstElement, loop)) { - // there is no way to reach the 'for' loop update - return !predecessor.predecessors().isEmpty(); - } - - if (isDescendant(predecessorFirstElement, loop)) { - return true; + if (!isForStatementInitializer(predecessorFirstElement, loop)) { + if (isForStatementUpdate(predecessorFirstElement, loop)) { + return !predecessor.predecessors().isEmpty(); + } + + if (isDescendant(predecessorFirstElement, loop)) { + return true; + } } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/MissingOverridesInRecordWithArrayComponentCheck.java b/java-checks/src/main/java/org/sonar/java/checks/MissingOverridesInRecordWithArrayComponentCheck.java index 13e2e2e44c5..e6143589e5f 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/MissingOverridesInRecordWithArrayComponentCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/MissingOverridesInRecordWithArrayComponentCheck.java @@ -83,16 +83,15 @@ public static Optional inspectRecord(ClassTree tree) { boolean hashCodeIsOverridden = false; boolean toStringIsOverridden = false; for (Tree member : tree.members()) { - if (!member.is(Tree.Kind.METHOD)) { - continue; - } - MethodTree method = (MethodTree) member; - if (EQUALS_MATCHER.matches(method)) { - equalsIsOverridden = true; - } else if (HASH_CODE_MATCHER.matches(method)) { - hashCodeIsOverridden = true; - } else if (TO_STRING_MATCHER.matches(method)) { - toStringIsOverridden = true; + if (member.is(Tree.Kind.METHOD)) { + MethodTree method = (MethodTree) member; + if (EQUALS_MATCHER.matches(method)) { + equalsIsOverridden = true; + } else if (HASH_CODE_MATCHER.matches(method)) { + hashCodeIsOverridden = true; + } else if (TO_STRING_MATCHER.matches(method)) { + toStringIsOverridden = true; + } } } return computeMessage(equalsIsOverridden, hashCodeIsOverridden, toStringIsOverridden); diff --git a/java-checks/src/main/java/org/sonar/java/checks/ModifiersOrderCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ModifiersOrderCheck.java index f588de06576..1575d68cd2d 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/ModifiersOrderCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/ModifiersOrderCheck.java @@ -146,15 +146,12 @@ private static List removalOfAllModifiers(ModifiersTre int numberModifiers = modifiersTree.size(); for (int i = 0; i < numberModifiers; i++) { ModifierTree current = modifiersTree.get(i); - if (current.is(Tree.Kind.ANNOTATION)) { - continue; - } - if (i == (numberModifiers - 1)) { - // Last: remove last token and potential space - removals.add(AnalyzerMessage.textSpanBetween(current, true, QuickFixHelper.nextToken(modifiersTree), false)); - } else { - // Take into account neighboring modifiers (can be on different lines) - removals.add(AnalyzerMessage.textSpanBetween(current, true, modifiersTree.get(i + 1), false)); + if (!current.is(Tree.Kind.ANNOTATION)) { + if (i == (numberModifiers - 1)) { + removals.add(AnalyzerMessage.textSpanBetween(current, true, QuickFixHelper.nextToken(modifiersTree), false)); + } else { + removals.add(AnalyzerMessage.textSpanBetween(current, true, modifiersTree.get(i + 1), false)); + } } } return removals; diff --git a/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java b/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java index 89e4390b720..f2f90eb7f11 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java @@ -52,14 +52,12 @@ public void visitNode(Tree tree) { for (VariableTree parameter : methodTree.parameters()) { SymbolMetadata parameterMetadata = parameter.symbol().metadata(); - if (parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.RequestParam") - || parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.PathVariable")) { - continue; + if (!parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.RequestParam") + && !parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.PathVariable")) { + TypeTree typeTree = parameter.type(); + Optional msg = expectedTypeInsteadOfOptional(typeTree.symbolType()); + msg.ifPresent(s -> reportIssue(typeTree, s)); } - - TypeTree typeTree = parameter.type(); - Optional msg = expectedTypeInsteadOfOptional(typeTree.symbolType()); - msg.ifPresent(s -> reportIssue(typeTree, s)); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java index 58e55a231c9..fb07d4b38de 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java @@ -194,15 +194,14 @@ static List tokenizeIdentifier(String identifier) { List words = new ArrayList<>(); Pattern splitPattern = Pattern.compile("(?=[A-Z])"); for (String part : identifier.split("_")) { - if (part.isEmpty()) { - continue; - } - if (isAllUppercaseWithLetter(part)) { - words.add(part.toLowerCase(Locale.ROOT)); - } else { - for (String sub : splitPattern.split(part)) { - if (!sub.isEmpty()) { - words.add(sub.toLowerCase(Locale.ROOT)); + if (!part.isEmpty()) { + if (isAllUppercaseWithLetter(part)) { + words.add(part.toLowerCase(Locale.ROOT)); + } else { + for (String sub : splitPattern.split(part)) { + if (!sub.isEmpty()) { + words.add(sub.toLowerCase(Locale.ROOT)); + } } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/RedundantThrowsDeclarationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/RedundantThrowsDeclarationCheck.java index e8ca9170644..169fdb7747a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/RedundantThrowsDeclarationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/RedundantThrowsDeclarationCheck.java @@ -83,24 +83,23 @@ private void checkMethodThrownList(MethodTree methodTree, ListTree thr for (TypeTree typeTree : thrownList) { Type exceptionType = typeTree.symbolType(); - if (exceptionType.isUnknown()) { - continue; - } - String fullyQualifiedName = exceptionType.fullyQualifiedName(); - if (!reported.contains(fullyQualifiedName)) { - String superTypeName = isSubclassOfAny(exceptionType, thrownList); - if (superTypeName != null && !exceptionType.isSubtypeOf("java.lang.RuntimeException")) { - reportIssueWithQuickfix(methodTree, typeTree, String.format( - "Remove the declaration of thrown exception '%s' which is a subclass of '%s'.", fullyQualifiedName, superTypeName)); - } else if (declaredMoreThanOnce(fullyQualifiedName, thrownList)) { - reportIssueWithQuickfix(methodTree, typeTree, String.format( - "Remove the redundant '%s' thrown exception declaration(s).", fullyQualifiedName)); - } else if (canNotBeThrown(methodTree, exceptionType, thrownExceptions) && (!isOverridableMethod || undocumentedExceptionNames.contains(exceptionType.name()))) { - reportIssueWithQuickfix(methodTree, typeTree, String.format( - "Remove the declaration of thrown exception '%s', as it cannot be thrown from %s's body.", fullyQualifiedName, - methodTreeType(methodTree))); + if (!exceptionType.isUnknown()) { + String fullyQualifiedName = exceptionType.fullyQualifiedName(); + if (!reported.contains(fullyQualifiedName)) { + String superTypeName = isSubclassOfAny(exceptionType, thrownList); + if (superTypeName != null && !exceptionType.isSubtypeOf("java.lang.RuntimeException")) { + reportIssueWithQuickfix(methodTree, typeTree, String.format( + "Remove the declaration of thrown exception '%s' which is a subclass of '%s'.", fullyQualifiedName, superTypeName)); + } else if (declaredMoreThanOnce(fullyQualifiedName, thrownList)) { + reportIssueWithQuickfix(methodTree, typeTree, String.format( + "Remove the redundant '%s' thrown exception declaration(s).", fullyQualifiedName)); + } else if (canNotBeThrown(methodTree, exceptionType, thrownExceptions) && (!isOverridableMethod || undocumentedExceptionNames.contains(exceptionType.name()))) { + reportIssueWithQuickfix(methodTree, typeTree, String.format( + "Remove the declaration of thrown exception '%s', as it cannot be thrown from %s's body.", fullyQualifiedName, + methodTreeType(methodTree))); + } + reported.add(fullyQualifiedName); } - reported.add(fullyQualifiedName); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/SwitchCasesShouldBeCommaSeparatedCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SwitchCasesShouldBeCommaSeparatedCheck.java index afcdbd158e7..d971968bbca 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/SwitchCasesShouldBeCommaSeparatedCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/SwitchCasesShouldBeCommaSeparatedCheck.java @@ -53,22 +53,20 @@ public void visitNode(Tree tree) { for (CaseGroupTree aCase : switchExpression.cases()) { List labels = aCase.labels(); int size = labels.size(); - if (size == 1) { - continue; - } - - Deque caseLabels = labels.stream() - .filter(label -> "case".equals(label.caseOrDefaultKeyword().text())) - .collect(Collectors.toCollection(ArrayDeque::new)); + if (size > 1) { + Deque caseLabels = labels.stream() + .filter(label -> "case".equals(label.caseOrDefaultKeyword().text())) + .collect(Collectors.toCollection(ArrayDeque::new)); - if (caseLabels.size() > 1) { - CaseLabelTree lastLabel = caseLabels.removeLast(); - ((DefaultJavaFileScannerContext) context).newIssue() - .forRule(this) - .onTree(lastLabel) - .withMessage(MESSAGE) - .withSecondaries(caseLabels.stream().map(label -> new JavaFileScannerContext.Location("", label)).toList()) - .report(); + if (caseLabels.size() > 1) { + CaseLabelTree lastLabel = caseLabels.removeLast(); + ((DefaultJavaFileScannerContext) context).newIssue() + .forRule(this) + .onTree(lastLabel) + .withMessage(MESSAGE) + .withSecondaries(caseLabels.stream().map(label -> new JavaFileScannerContext.Location("", label)).toList()) + .report(); + } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/UseOfSequentialForSequentialGathererCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UseOfSequentialForSequentialGathererCheck.java index 245724e40d7..4ee14530c71 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UseOfSequentialForSequentialGathererCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UseOfSequentialForSequentialGathererCheck.java @@ -88,20 +88,19 @@ public void visitNode(Tree tree) { MethodInvocationTree mit = (MethodInvocationTree) tree; for (Case caze : CASES) { - if (!caze.matcher.matches(mit)) { - continue; - } - var argumentPredicate = caze.pred; - var issues = argumentPredicate.predicate.apply(mit.arguments().get(argumentPredicate.argIdx)); - if (!issues.isEmpty()) { - - var secondaries = issues.subList(1, issues.size()) - .stream() - .map(element -> new JavaFileScannerContext.Location("", element)) - .toList(); - - context.reportIssue(this, issues.get(0), caze.msg, secondaries, null); - return; + if (caze.matcher.matches(mit)) { + var argumentPredicate = caze.pred; + var issues = argumentPredicate.predicate.apply(mit.arguments().get(argumentPredicate.argIdx)); + if (!issues.isEmpty()) { + + var secondaries = issues.subList(1, issues.size()) + .stream() + .map(element -> new JavaFileScannerContext.Location("", element)) + .toList(); + + context.reportIssue(this, issues.get(0), caze.msg, secondaries, null); + return; + } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/serialization/SerialVersionUidInRecordCheck.java b/java-checks/src/main/java/org/sonar/java/checks/serialization/SerialVersionUidInRecordCheck.java index c8fb397629f..5883deabcd5 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/serialization/SerialVersionUidInRecordCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/serialization/SerialVersionUidInRecordCheck.java @@ -41,13 +41,12 @@ public void visitNode(Tree tree) { return; } for (Tree member : targetRecord.members()) { - if (!member.is(Tree.Kind.VARIABLE)) { - continue; - } - VariableTree variable = (VariableTree) member; - if (isSerialVersionUIDField(variable) && setsTheValueToZero(variable)) { - reportIssue(variable, "Remove this redundant \"serialVersionUID\" field"); - return; + if (member.is(Tree.Kind.VARIABLE)) { + VariableTree variable = (VariableTree) member; + if (isSerialVersionUIDField(variable) && setsTheValueToZero(variable)) { + reportIssue(variable, "Remove this redundant \"serialVersionUID\" field"); + return; + } } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java index 31f90cfd176..e708bfad1fa 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java @@ -102,14 +102,13 @@ public void visitNode(Tree tree) { private static Set extractModelAttributeMethodParameter(List methods){ Set modelAttributeMethodParameter = new HashSet<>(); for (var method : methods) { - if (!method.symbol().metadata().isAnnotatedWith(MODEL_ATTRIBUTE_ANNOTATION)) { - continue; - } - for (var parameter : method.parameters()) { - SymbolMetadata metadata = parameter.symbol().metadata(); - var arguments = metadata.valuesForAnnotation(PATH_VARIABLE_ANNOTATION); - if (arguments != null) { - modelAttributeMethodParameter.add(extractPathMethodParameters(parameter, arguments).value); + if (method.symbol().metadata().isAnnotatedWith(MODEL_ATTRIBUTE_ANNOTATION)) { + for (var parameter : method.parameters()) { + SymbolMetadata metadata = parameter.symbol().metadata(); + var arguments = metadata.valuesForAnnotation(PATH_VARIABLE_ANNOTATION); + if (arguments != null) { + modelAttributeMethodParameter.add(extractPathMethodParameters(parameter, arguments).value); + } } } } @@ -173,11 +172,9 @@ private void checkParametersAndPathTemplate(MethodTree method, Set model String fullyQualifiedName = ann.annotationType().symbolType().fullyQualifiedName(); var values = method.symbol().metadata().valuesForAnnotation(fullyQualifiedName); - if (values == null || !MAPPING_ANNOTATIONS.contains(fullyQualifiedName)) { - continue; + if (values != null && MAPPING_ANNOTATIONS.contains(fullyQualifiedName)) { + templateVariables.add(new UriInfo<>(ann, templateVariablesFromMapping(values))); } - - templateVariables.add(new UriInfo<>(ann, templateVariablesFromMapping(values))); } // we handle the case where a path variable doesn't match to uri parameter (/{aParam}/) @@ -296,18 +293,14 @@ private Set extractClassAndRecordProperties(MethodTree method) { for (var parameter : method.parameters()) { Type parameterType = parameter.type().symbolType(); - if (parameterType.isUnknown() - || isStandardDataType(parameterType) || parameterType.isSubtypeOf(MAP) - || requiresModelAttributeAnnotation(parameter.symbol().metadata())) { - continue; - } - - if (parameterType.isSubtypeOf("java.lang.Record") && springWebVersion != SpringWebVersion.LESS_THAN_5_3) { - // Extract record's components - properties.addAll(extractRecordProperties(parameterType)); - } else if (parameterType.isClass()) { - // Extract setter properties from the class - properties.addAll(extractSetterProperties(parameterType)); + if (!parameterType.isUnknown() + && !isStandardDataType(parameterType) && !parameterType.isSubtypeOf(MAP) + && !requiresModelAttributeAnnotation(parameter.symbol().metadata())) { + if (parameterType.isSubtypeOf("java.lang.Record") && springWebVersion != SpringWebVersion.LESS_THAN_5_3) { + properties.addAll(extractRecordProperties(parameterType)); + } else if (parameterType.isClass()) { + properties.addAll(extractSetterProperties(parameterType)); + } } } @@ -323,14 +316,10 @@ static Set extractSetterProperties(Type type) { // Extract properties from explicit setter methods for (Symbol member : typeSymbol.memberSymbols()) { - if (!member.isMethodSymbol()) { - continue; + if (member.isMethodSymbol()) { + Symbol.MethodSymbol method = (Symbol.MethodSymbol) member; + isSetterLike(method).ifPresent(properties::add); } - - Symbol.MethodSymbol method = (Symbol.MethodSymbol) member; - - // Check if it's a setter and extract a property name - isSetterLike(method).ifPresent(properties::add); } return properties; @@ -345,17 +334,13 @@ private static Set checkForLombokSetters(Symbol.TypeSymbol typeSymbol) { // Extract properties from fields if Lombok generates setters for (Symbol.VariableSymbol field : typeSymbol.memberSymbols().stream().filter(Symbol::isVariableSymbol).map(Symbol.VariableSymbol.class::cast).toList()) { - if (field.isStatic() || field.isFinal()) { - continue; - } + if (!field.isStatic() && !field.isFinal()) { + boolean hasFieldLevelSetter = field.metadata().annotations().stream() + .anyMatch(annotation -> "lombok.Setter".equals(annotation.symbol().type().fullyQualifiedName())); - // Check if field has @Setter annotation at field level - boolean hasFieldLevelSetter = field.metadata().annotations().stream() - .anyMatch(annotation -> "lombok.Setter".equals(annotation.symbol().type().fullyQualifiedName())); - - // Add property if class-level or field-level Lombok setter exists - if (hasLombokSetters || hasFieldLevelSetter) { - properties.add(field.name()); + if (hasLombokSetters || hasFieldLevelSetter) { + properties.add(field.name()); + } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java index 13821758b88..5e2e6605b8a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java @@ -82,16 +82,15 @@ public void visitNode(Tree tree) { for (RedundancyRule rule : REDUNDANCY_RULES) { List redundantAnnotations = annotationsByFqn.get(rule.redundantFqn); - if (redundantAnnotations == null) { - continue; - } - for (AnnotationTree redundantAnnotation : redundantAnnotations) { - for (String impliedByFqn : rule.impliedByFqns) { - List impliedByAnnotations = annotationsByFqn.get(impliedByFqn); - if (impliedByAnnotations != null && !impliedByAnnotations.isEmpty() - && passesSpecialCondition(rule, redundantAnnotation)) { - reportRedundancy(redundantAnnotation, impliedByAnnotations.get(0)); - break; + if (redundantAnnotations != null) { + for (AnnotationTree redundantAnnotation : redundantAnnotations) { + for (String impliedByFqn : rule.impliedByFqns) { + List impliedByAnnotations = annotationsByFqn.get(impliedByFqn); + if (impliedByAnnotations != null && !impliedByAnnotations.isEmpty() + && passesSpecialCondition(rule, redundantAnnotation)) { + reportRedundancy(redundantAnnotation, impliedByAnnotations.get(0)); + break; + } } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java index 8734589db61..35fa9e6bf1b 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java @@ -81,12 +81,10 @@ private static void collectAutowiredFields(Tree tree, Map for(String annotation: AUTOWIRED_ANNOTATIONS) { List annotationValues = metadata.valuesForAnnotation(annotation); if (annotationValues != null) { - if (annotationValues.stream().anyMatch(SpringConfigurationWithAutowiredFieldsCheck::isRequiredFalse) - && variable.initializer() != null) { - // Common pattern used to define a default value. - continue; + if (annotationValues.stream().noneMatch(SpringConfigurationWithAutowiredFieldsCheck::isRequiredFalse) + || variable.initializer() == null) { + autowiredFields.put(variableSymbol, variable); } - autowiredFields.put(variableSymbol, variable); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/ParameterizedTestCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/ParameterizedTestCheck.java index 6a066d60cbe..1b5dcfe4058 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/ParameterizedTestCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/ParameterizedTestCheck.java @@ -73,30 +73,26 @@ public void visitNode(Tree tree) { Set handled = new HashSet<>(); for (int i = 0; i < methods.size(); i++) { MethodTree method = methods.get(i); - if (handled.contains(method)) { - continue; - } - List methodBody = method.block().body(); - // In addition to filtering literals, we want to count the number of differences since they will represent the number of parameter - // that would be required to transform the tests to a single parametrized one. - CollectAndIgnoreLiterals collectAndIgnoreLiterals = new CollectAndIgnoreLiterals(); - - List equivalentMethods = new ArrayList<>(); - - for (int j = i + 1; j < methods.size(); j++) { - MethodTree otherMethod = methods.get(j); - if (!handled.contains(otherMethod)) { - boolean areEquivalent = SyntacticEquivalence.areEquivalent(methodBody, otherMethod.block().body(), collectAndIgnoreLiterals); - if (areEquivalent) { - // If methods where not equivalent, we don't want to pollute the set of node to parameterize. - equivalentMethods.add(otherMethod); - collectAndIgnoreLiterals.finishCollect(); + if (!handled.contains(method)) { + List methodBody = method.block().body(); + CollectAndIgnoreLiterals collectAndIgnoreLiterals = new CollectAndIgnoreLiterals(); + + List equivalentMethods = new ArrayList<>(); + + for (int j = i + 1; j < methods.size(); j++) { + MethodTree otherMethod = methods.get(j); + if (!handled.contains(otherMethod)) { + boolean areEquivalent = SyntacticEquivalence.areEquivalent(methodBody, otherMethod.block().body(), collectAndIgnoreLiterals); + if (areEquivalent) { + equivalentMethods.add(otherMethod); + collectAndIgnoreLiterals.finishCollect(); + } + collectAndIgnoreLiterals.clearCurrentNodes(); } - collectAndIgnoreLiterals.clearCurrentNodes(); } - } - reportIfIssue(handled, method, collectAndIgnoreLiterals, equivalentMethods); + reportIfIssue(handled, method, collectAndIgnoreLiterals, equivalentMethods); + } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java index 49279da87c5..44cc2dba73c 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java @@ -52,11 +52,9 @@ public void visitNode(Tree tree) { VariableTree variableTree = (VariableTree) member; Symbol symbol = variableTree.symbol(); if ((isTestNameOrTemporaryFolderRule(symbol) || hasTempDirAnnotation(symbol)) && symbol.usages().isEmpty()) { - // if class is abstract, then we need to check modifier - if not private, then it's okay - if (isAbstract && !ModifiersUtils.hasModifier(variableTree.modifiers(), Modifier.PRIVATE)) { - continue; + if (!isAbstract || ModifiersUtils.hasModifier(variableTree.modifiers(), Modifier.PRIVATE)) { + reportIssue(variableTree.simpleName(), "Remove this unused \"" + getSymbolType(symbol) + "\"."); } - reportIssue(variableTree.simpleName(), "Remove this unused \"" + getSymbolType(symbol) + "\"."); } } else if (member.is(Tree.Kind.METHOD, Tree.Kind.CONSTRUCTOR)) { checkJUnit5((MethodTree) member); diff --git a/java-frontend/src/main/java/org/sonar/java/cfg/LiveVariables.java b/java-frontend/src/main/java/org/sonar/java/cfg/LiveVariables.java index 478d80e2e33..f31923a333c 100644 --- a/java-frontend/src/main/java/org/sonar/java/cfg/LiveVariables.java +++ b/java-frontend/src/main/java/org/sonar/java/cfg/LiveVariables.java @@ -113,11 +113,10 @@ private void analyzeCFG(Map> in, Map newIn = new HashSet<>(gen.get(block)); newIn.addAll(SetUtils.difference(blockOut, kill.get(block))); - if (newIn.equals(in.get(block))) { - continue; + if (!newIn.equals(in.get(block))) { + in.put(block, newIn); + block.predecessors().forEach(workList::addLast); } - in.put(block, newIn); - block.predecessors().forEach(workList::addLast); } } diff --git a/java-frontend/src/main/java/org/sonar/java/classpath/JavaSdkUtil.java b/java-frontend/src/main/java/org/sonar/java/classpath/JavaSdkUtil.java index 03308f62300..51a72a8715c 100644 --- a/java-frontend/src/main/java/org/sonar/java/classpath/JavaSdkUtil.java +++ b/java-frontend/src/main/java/org/sonar/java/classpath/JavaSdkUtil.java @@ -66,20 +66,16 @@ private static List collectJars(Path home, boolean isMac) { List rootFiles = new ArrayList<>(); Set duplicatePathFilter = new HashSet<>(); for (Path jarDir : collectJarDirs(home, isMac)) { - if (!Files.isDirectory(jarDir)) { - continue; + if (Files.isDirectory(jarDir)) { + listFiles(jarDir, JavaSdkUtil::isJarFile).stream() + .filter(JavaSdkUtil::isNotAlternativeImplementation) + .map(JavaSdkUtil::toRealPath).filter(Optional::isPresent).map(Optional::get) + .forEach(jarFile -> { + if (duplicatePathFilter.add(jarFile)) { + rootFiles.add(jarFile.toFile()); + } + }); } - listFiles(jarDir, JavaSdkUtil::isJarFile).stream() - // filter out alternative implementations - .filter(JavaSdkUtil::isNotAlternativeImplementation) - // filter out duplicate (symbolically linked) .jar files commonly found in OS X JDK distributions - .map(JavaSdkUtil::toRealPath).filter(Optional::isPresent).map(Optional::get) - // make sure there is no duplicates - .forEach(jarFile -> { - if (duplicatePathFilter.add(jarFile)) { - rootFiles.add(jarFile.toFile()); - } - }); } return rootFiles; diff --git a/java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java b/java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java index 5ae3d502130..7a470516627 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JMethodSymbol.java @@ -157,18 +157,14 @@ void findOverridesInParentTypes(Collection accumulator, Predicate< private void findOverridesInTypes(Collection accumulator, Predicate overridesCondition, ITypeBinding... types) { for (ITypeBinding type : types) { - if (type == null) { - // Can happen for unknown reason. - continue; + if (type != null) { + Stream.of(type.getDeclaredMethods()) + .filter(overridesCondition) + .findFirst() + .map(sema::methodSymbol) + .ifPresent(accumulator::add); + findOverridesInParentTypes(accumulator, overridesCondition, type); } - // check current type - Stream.of(type.getDeclaredMethods()) - .filter(overridesCondition) - .findFirst() - .map(sema::methodSymbol) - .ifPresent(accumulator::add); - // check other inheritance levels - findOverridesInParentTypes(accumulator, overridesCondition, type); } } From b2bb97804005789521ba1ac53894b3c548560bcd Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 15:43:59 +0200 Subject: [PATCH 09/10] Fix test failures from S9357 and S6880 changes - Revert anonymous-to-lambda conversions where Mockito spy() is used, since Mockito cannot spy on lambdas - Revert anonymous-to-lambda in DefaultJavaResourceLocatorTest since the test counts generated .class files (lambdas don't generate them) - Add null case to switch expression in JSymbolMetadata to handle null annotation values Co-Authored-By: Claude Opus 4.6 --- .../java/checks/AbstractPrintfChecker.java | 20 +++--- ...ssertThrowsInsteadOfTryCatchFailCheck.java | 34 ++++++---- .../CompilationOrPreparationInLoopCheck.java | 4 +- .../checks/ConditionalRuleCacheUtils.java | 39 +++++------ .../java/checks/DateTimeConversionsCheck.java | 12 ++-- .../checks/EqualsMismatchedMembersCheck.java | 18 ++--- ...tatelessGatherersOmitInitializerCheck.java | 28 ++++---- .../java/checks/HardCodedSecretCheck.java | 2 +- .../sonar/java/checks/HardcodedURICheck.java | 13 +++- .../checks/HashCodeMismatchedFieldsCheck.java | 28 ++++---- .../checks/IdenticalCasesInSwitchCheck.java | 32 ++++----- .../checks/IncorrectOrderOfMembersCheck.java | 16 ++--- .../InterfaceAsConstantContainerCheck.java | 5 +- ...lesShouldNotSpanSwitchCaseGroupsCheck.java | 11 +-- .../checks/LoopExecutingAtMostOnceCheck.java | 20 +++--- ...rridesInRecordWithArrayComponentCheck.java | 19 +++--- .../java/checks/ModifiersOrderCheck.java | 15 +++-- .../java/checks/OptionalAsParameterCheck.java | 12 ++-- .../java/checks/PatternMatchUsingIfCheck.java | 24 +++---- .../sonar/java/checks/PseudoRandomCheck.java | 17 ++--- .../RedundantThrowsDeclarationCheck.java | 33 ++++----- ...witchCasesShouldBeCommaSeparatedCheck.java | 28 ++++---- .../java/checks/TryWithResourcesCheck.java | 12 ++-- ...fSequentialForSequentialGathererCheck.java | 27 ++++---- ...tilityClassWithPublicConstructorCheck.java | 15 +++-- .../java/checks/helpers/StringUtils.java | 13 ++-- ...roidMobileDatabaseEncryptionKeysCheck.java | 4 +- .../SerialVersionUidInRecordCheck.java | 13 ++-- .../MissingPathVariableAnnotationCheck.java | 67 ++++++++++++------- .../RedundantSpringAnnotationCheck.java | 19 +++--- .../checks/spring/SpelExpressionCheck.java | 4 +- ...ConfigurationWithAutowiredFieldsCheck.java | 8 ++- .../checks/tests/ParameterizedTestCheck.java | 38 ++++++----- .../checks/unused/UnusedTestRuleCheck.java | 6 +- .../utils/AnnotationFieldReferenceFinder.java | 2 +- .../org/sonar/java/model/JSymbolMetadata.java | 1 + .../java/DefaultJavaResourceLocatorTest.java | 5 +- .../sonar/java/ast/JavaAstScannerTest.java | 7 +- .../org/sonar/java/model/JParserTest.java | 12 +++- 39 files changed, 386 insertions(+), 297 deletions(-) diff --git a/java-checks/src/main/java/org/sonar/java/checks/AbstractPrintfChecker.java b/java-checks/src/main/java/org/sonar/java/checks/AbstractPrintfChecker.java index 975d8d5bde0..cd783f0c69c 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/AbstractPrintfChecker.java +++ b/java-checks/src/main/java/org/sonar/java/checks/AbstractPrintfChecker.java @@ -201,18 +201,18 @@ protected List getParameters(String formatString, MethodInvocationTree m while (matcher.find()) { if (firstArgumentIsLT(params, matcher.group(2))) { reportMissingPrevious(mit); - } else { - StringBuilder param = new StringBuilder(); - for (int groupIndex : new int[] {1, 2, 5, 6}) { - if (matcher.group(groupIndex) != null) { - param.append(matcher.group(groupIndex)); - } - } - String specifier = param.toString(); - if(!"%".equals(specifier)) { - params.add(specifier); + continue; + } + StringBuilder param = new StringBuilder(); + for (int groupIndex : new int[] {1, 2, 5, 6}) { + if (matcher.group(groupIndex) != null) { + param.append(matcher.group(groupIndex)); } } + String specifier = param.toString(); + if(!"%".equals(specifier)) { + params.add(specifier); + } } return params; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java b/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java index df56716e9fe..f90048c68f9 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/AssertThrowsInsteadOfTryCatchFailCheck.java @@ -169,11 +169,15 @@ private Replacements junitReplacement( ", %s".formatted(contentFor(argument)) ).orElse(""); - String prefix = isTryBlock - ? "assertThrows(%s, () -> ".formatted(typeClass(firstCaughtTypeInTry(tryStatement))) - : "assertDoesNotThrow(() -> "; - String suffix = "%s);".formatted(argumentsSuffix); - return new Replacements(prefix, suffix); + return isTryBlock ? + new Replacements( + "assertThrows(%s, () -> ".formatted(typeClass(firstCaughtTypeInTry(tryStatement))), + "%s);".formatted(argumentsSuffix) + ) : + new Replacements( + "assertDoesNotThrow(() -> ", + "%s);".formatted(argumentsSuffix) + ); } private Replacements assertJReplacement( @@ -181,14 +185,18 @@ private Replacements assertJReplacement( TryStatementTree tryStatement, boolean isTryBlock ) { - var failureMessagePart = failArguments.isEmpty() - ? "" - : ".withFailMessage(%s)".formatted(contentFor(failArguments.get(0))); - String prefix = "assertThatCode(() -> "; - String suffix = isTryBlock - ? ")%s.isInstanceOf(%s);".formatted(failureMessagePart, typeClass(firstCaughtTypeInTry(tryStatement))) - : ")%s.doesNotThrowAnyException();".formatted(failureMessagePart); - return new Replacements(prefix, suffix); + var failureMessagePart = failArguments.isEmpty() ? + "" : + ".withFailMessage(%s)".formatted(contentFor(failArguments.get(0))); + return isTryBlock ? + new Replacements( + "assertThatCode(() -> ", + ")%s.isInstanceOf(%s);".formatted(failureMessagePart, typeClass(firstCaughtTypeInTry(tryStatement))) + ) : + new Replacements( + "assertThatCode(() -> ", + ")%s.doesNotThrowAnyException();".formatted(failureMessagePart) + ); } private String contentFor(Tree tree) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java index 1e293d0cddc..8dfa122b5fd 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java @@ -201,8 +201,8 @@ public void visitUnaryExpression(UnaryExpressionTree tree) { super.visitUnaryExpression(tree); switch (tree.kind()) { case POSTFIX_INCREMENT, POSTFIX_DECREMENT, PREFIX_INCREMENT, PREFIX_DECREMENT -> { - if (tree.expression() instanceof IdentifierTree identifier) { - names.add(identifier.name()); + if (tree.expression().is(Tree.Kind.IDENTIFIER)) { + names.add(((IdentifierTree) tree.expression()).name()); } } default -> { diff --git a/java-checks/src/main/java/org/sonar/java/checks/ConditionalRuleCacheUtils.java b/java-checks/src/main/java/org/sonar/java/checks/ConditionalRuleCacheUtils.java index 41a56c6d0cb..7cc7b26cefe 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/ConditionalRuleCacheUtils.java +++ b/java-checks/src/main/java/org/sonar/java/checks/ConditionalRuleCacheUtils.java @@ -112,26 +112,27 @@ static CachedFileData deserialize(byte[] data) { int issueCount = Integer.parseInt(header[1]); List issues = new ArrayList<>(); for (int i = 1; i < lines.length; i++) { - if (!lines[i].isEmpty()) { - String[] parts = lines[i].split("\\|", -1); - int sl = Integer.parseInt(parts[0]); - int sc = Integer.parseInt(parts[1]); - int el = Integer.parseInt(parts[2]); - int ec = Integer.parseInt(parts[3]); - String message = new String(dec.decode(parts[4]), StandardCharsets.UTF_8); - String replacement = new String(dec.decode(parts[5]), StandardCharsets.UTF_8); - boolean hasImport = "1".equals(parts[6]); - ImportEditData importEdit = null; - if (hasImport) { - int isl = Integer.parseInt(parts[7]); - int isc = Integer.parseInt(parts[8]); - int iel = Integer.parseInt(parts[9]); - int iec = Integer.parseInt(parts[10]); - String importRepl = new String(dec.decode(parts[11]), StandardCharsets.UTF_8); - importEdit = new ImportEditData(isl, isc, iel, iec, importRepl); - } - issues.add(new CachedIssue(sl, sc, el, ec, message, replacement, importEdit)); + if (lines[i].isEmpty()) { + continue; } + String[] parts = lines[i].split("\\|", -1); + int sl = Integer.parseInt(parts[0]); + int sc = Integer.parseInt(parts[1]); + int el = Integer.parseInt(parts[2]); + int ec = Integer.parseInt(parts[3]); + String message = new String(dec.decode(parts[4]), StandardCharsets.UTF_8); + String replacement = new String(dec.decode(parts[5]), StandardCharsets.UTF_8); + boolean hasImport = "1".equals(parts[6]); + ImportEditData importEdit = null; + if (hasImport) { + int isl = Integer.parseInt(parts[7]); + int isc = Integer.parseInt(parts[8]); + int iel = Integer.parseInt(parts[9]); + int iec = Integer.parseInt(parts[10]); + String importRepl = new String(dec.decode(parts[11]), StandardCharsets.UTF_8); + importEdit = new ImportEditData(isl, isc, iel, iec, importRepl); + } + issues.add(new CachedIssue(sl, sc, el, ec, message, replacement, importEdit)); } return new CachedFileData(totalCount, issueCount, issues); } diff --git a/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java index afe6f41ca06..f7fee6daeee 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/DateTimeConversionsCheck.java @@ -99,12 +99,12 @@ private static boolean isLocalDateOrTime(Type type) { private static ExpressionTree skipParenthesesAndCasts(ExpressionTree expression) { ExpressionTree result = expression; while (true) { - switch (result) { - case ParenthesizedTree parenthesizedTree -> result = parenthesizedTree.expression(); - case TypeCastTree typeCastTree -> result = typeCastTree.expression(); - default -> { - return result; - } + if (result instanceof ParenthesizedTree parenthesizedTree) { + result = parenthesizedTree.expression(); + } else if (result instanceof TypeCastTree typeCastTree) { + result = typeCastTree.expression(); + } else { + return result; } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java index 8bebc47b819..47657662d3a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/EqualsMismatchedMembersCheck.java @@ -98,15 +98,17 @@ public void visitNode(Tree tree) { ComparisonCollector collector = new ComparisonCollector(owner); methodTree.block().accept(collector); for (ComparisonSite comparison : collector.comparisons) { - if (!collector.pairsByStatement.get(comparison.statement).contains(comparison.pair().reversed())) { - reportIssue( - comparison.tree, - String.format(ISSUE_MESSAGE, comparison.thisMember.displayName, comparison.otherMember.displayName), - List.of( - new JavaFileScannerContext.Location(SECONDARY_THIS, comparison.thisMember.tree), - new JavaFileScannerContext.Location(SECONDARY_OTHER, comparison.otherMember.tree)), - null); + // Order-independent equality: (a, b) || (b, a) in the same statement is not a mismatch. + if (collector.pairsByStatement.get(comparison.statement).contains(comparison.pair().reversed())) { + continue; } + reportIssue( + comparison.tree, + String.format(ISSUE_MESSAGE, comparison.thisMember.displayName, comparison.otherMember.displayName), + List.of( + new JavaFileScannerContext.Location(SECONDARY_THIS, comparison.thisMember.tree), + new JavaFileScannerContext.Location(SECONDARY_OTHER, comparison.otherMember.tree)), + null); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java index 0544777335d..aef0bfb474e 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/ForStatelessGatherersOmitInitializerCheck.java @@ -94,19 +94,21 @@ public void visitNode(Tree tree) { MethodInvocationTree mit = (MethodInvocationTree) tree; for (Case c : CASES) { - if (c.matcher.matches(mit)) { - var argPredicate = c.pred; - var issues = argPredicate.predicate.apply(mit.arguments().get(argPredicate.argIdx)); - if (!issues.isEmpty()) { - - var secondaries = issues.subList(1, issues.size()) - .stream() - .map(element -> new JavaFileScannerContext.Location("", element)) - .toList(); - - context.reportIssue(this, issues.get(0), c.msg, secondaries, null); - return; - } + if (!c.matcher.matches(mit)) { + continue; + } + + var argPredicate = c.pred; + var issues = argPredicate.predicate.apply(mit.arguments().get(argPredicate.argIdx)); + if (!issues.isEmpty()) { + + var secondaries = issues.subList(1, issues.size()) + .stream() + .map(element -> new JavaFileScannerContext.Location("", element)) + .toList(); + + context.reportIssue(this, issues.get(0), c.msg, secondaries, null); + return; } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/HardCodedSecretCheck.java b/java-checks/src/main/java/org/sonar/java/checks/HardCodedSecretCheck.java index eada3b0ec50..19cef5bfb1e 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/HardCodedSecretCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/HardCodedSecretCheck.java @@ -35,7 +35,7 @@ import static org.sonar.java.checks.HardcodedIpCheck.IP_V6_ALONE; @Rule(key = "S6418") -public final class HardCodedSecretCheck extends AbstractHardCodedCredentialChecker { +public class HardCodedSecretCheck extends AbstractHardCodedCredentialChecker { private static final String DEFAULT_SECRET_WORDS = "api[_.-]?key,auth,credential,secret,token"; private static final String DEFAULT_RANDOMNESS_SENSIBILITY= "5.0"; diff --git a/java-checks/src/main/java/org/sonar/java/checks/HardcodedURICheck.java b/java-checks/src/main/java/org/sonar/java/checks/HardcodedURICheck.java index 48af76f2686..d78de95b441 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/HardcodedURICheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/HardcodedURICheck.java @@ -118,10 +118,17 @@ public void leaveFile(JavaFileScannerContext context) { } for(VariableData v : hardCodedUri) { - if (!idNamesWithoutSemantic.contains(v.identifier()) - && !(idNamesWithSemantic.contains(v.identifier()) && idSymbols.contains(v.symbol()))) { - reportHardcodedURI(v.initializer()); + // equals to an identifier with unknown semantic, we cannot compare their symbols + if (idNamesWithoutSemantic.contains(v.identifier())) { + continue; } + + // idNamesWithSemantic is used to only compare the symbols when their string identifier are the same + // as comparing symbols is costly + if (idNamesWithSemantic.contains(v.identifier()) && idSymbols.contains(v.symbol())) { + continue; + } + reportHardcodedURI(v.initializer()); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/HashCodeMismatchedFieldsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/HashCodeMismatchedFieldsCheck.java index 11726053b3d..e2ca285604e 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/HashCodeMismatchedFieldsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/HashCodeMismatchedFieldsCheck.java @@ -118,14 +118,15 @@ private static EqualsAndHashCode find(ClassTree classTree) { MethodTree hashCodeMethod = null; List otherMethods = new ArrayList<>(); for (Tree member : classTree.members()) { - if (member instanceof MethodTree methodTree && methodTree.block() != null) { - if (MethodTreeUtils.isEqualsMethod(methodTree)) { - equalsMethod = methodTree; - } else if (MethodTreeUtils.isHashCodeMethod(methodTree)) { - hashCodeMethod = methodTree; - } else { - otherMethods.add(methodTree); - } + if (!(member instanceof MethodTree methodTree) || methodTree.block() == null) { + continue; + } + if (MethodTreeUtils.isEqualsMethod(methodTree)) { + equalsMethod = methodTree; + } else if (MethodTreeUtils.isHashCodeMethod(methodTree)) { + hashCodeMethod = methodTree; + } else { + otherMethods.add(methodTree); } } if (equalsMethod == null || hashCodeMethod == null) { @@ -139,11 +140,12 @@ private static Map> collectHelperFields(S Map> fieldsByHelper = new HashMap<>(); for (MethodTree helper : otherMethods) { Symbol.MethodSymbol helperSymbol = helper.symbol(); - if (!helperSymbol.isUnknown() && helper.parameters().isEmpty()) { - ReadAndAssignedFields helperFields = collectReadFields(helper, owner, Map.of(), Role.HELPER); - if (!helperFields.failed()) { - fieldsByHelper.put(helperSymbol, helperFields.readFields()); - } + if (helperSymbol.isUnknown() || !helper.parameters().isEmpty()) { + continue; + } + ReadAndAssignedFields helperFields = collectReadFields(helper, owner, Map.of(), Role.HELPER); + if (!helperFields.failed()) { + fieldsByHelper.put(helperSymbol, helperFields.readFields()); } } return fieldsByHelper; diff --git a/java-checks/src/main/java/org/sonar/java/checks/IdenticalCasesInSwitchCheck.java b/java-checks/src/main/java/org/sonar/java/checks/IdenticalCasesInSwitchCheck.java index 2f219f746db..695f60a3a14 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/IdenticalCasesInSwitchCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/IdenticalCasesInSwitchCheck.java @@ -85,13 +85,14 @@ protected Map> checkSwitchStatement(SwitchTree Set duplicates = new HashSet<>(); for (CaseGroupTree caseGroupTree : cases) { index++; - if (!duplicates.contains(caseGroupTree)) { - for (int i = index; i < cases.size(); i++) { - CaseGroupTree caseI = cases.get(i); - if (SyntacticEquivalence.areEquivalent(caseGroupTree.body(), caseI.body())) { - duplicates.add(caseI); - identicalBranches.computeIfAbsent(caseGroupTree, k -> new HashSet<>()).add(caseI); - } + if (duplicates.contains(caseGroupTree)) { + continue; + } + for (int i = index; i < cases.size(); i++) { + CaseGroupTree caseI = cases.get(i); + if (SyntacticEquivalence.areEquivalent(caseGroupTree.body(), caseI.body())) { + duplicates.add(caseI); + identicalBranches.computeIfAbsent(caseGroupTree, k -> new HashSet<>()).add(caseI); } } } @@ -124,14 +125,15 @@ private static IfElseChain collectIdenticalBranches(List allBranc IfElseChain ifElseChain = new IfElseChain(); Set duplicates = new HashSet<>(); for (int i = 0; i < allBranches.size(); i++) { - if (!duplicates.contains(allBranches.get(i))) { - for (int j = i + 1; j < allBranches.size(); j++) { - StatementTree statement1 = allBranches.get(i); - StatementTree statement2 = allBranches.get(j); - if (SyntacticEquivalence.areEquivalentIncludingSameVariables(statement1, statement2)) { - duplicates.add(statement2); - ifElseChain.branches.computeIfAbsent(statement1, k -> new HashSet<>()).add(statement2); - } + if (duplicates.contains(allBranches.get(i))) { + continue; + } + for (int j = i + 1; j < allBranches.size(); j++) { + StatementTree statement1 = allBranches.get(i); + StatementTree statement2 = allBranches.get(j); + if (SyntacticEquivalence.areEquivalentIncludingSameVariables(statement1, statement2)) { + duplicates.add(statement2); + ifElseChain.branches.computeIfAbsent(statement1, k -> new HashSet<>()).add(statement2); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/IncorrectOrderOfMembersCheck.java b/java-checks/src/main/java/org/sonar/java/checks/IncorrectOrderOfMembersCheck.java index db97a503ccd..4ef43722d7a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/IncorrectOrderOfMembersCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/IncorrectOrderOfMembersCheck.java @@ -44,8 +44,8 @@ public void visitClass(ClassTree tree) { int prev = 0; for (int i = 0; i < tree.members().size(); i++) { final Tree member = tree.members().get(i); - int priority = -1; - IdentifierTree identifier = null; + final int priority; + IdentifierTree identifier; if (member.is(Tree.Kind.VARIABLE)) { VariableTree variable = ((VariableTree) member); if (variable.symbol().isStatic()) { @@ -60,13 +60,13 @@ public void visitClass(ClassTree tree) { } else if (member.is(Tree.Kind.METHOD)) { priority = 3; identifier = ((MethodTree) member).simpleName(); + } else { + continue; } - if (identifier != null) { - if (priority < prev) { - context.reportIssue(this, identifier, "Move this " + NAMES[priority] + " to comply with Java Code Conventions."); - } else { - prev = priority; - } + if (priority < prev) { + context.reportIssue(this, identifier, "Move this " + NAMES[priority] + " to comply with Java Code Conventions."); + } else { + prev = priority; } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/InterfaceAsConstantContainerCheck.java b/java-checks/src/main/java/org/sonar/java/checks/InterfaceAsConstantContainerCheck.java index e7b8871e863..7be9d479bf6 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/InterfaceAsConstantContainerCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/InterfaceAsConstantContainerCheck.java @@ -53,9 +53,10 @@ private static List collectConstantsLocation(Cl // the interface doesn't hold only constants return Collections.emptyList(); } - if (!member.is(Tree.Kind.EMPTY_STATEMENT)) { - constantLocations.add(new JavaFileScannerContext.Location("", ((VariableTree) member).simpleName())); + if (member.is(Tree.Kind.EMPTY_STATEMENT)) { + continue; } + constantLocations.add(new JavaFileScannerContext.Location("", ((VariableTree) member).simpleName())); } return constantLocations; } diff --git a/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java index 46517468300..7896e744823 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java @@ -51,11 +51,12 @@ public void visitNode(Tree tree) { List caseGroups = ((SwitchTree) tree).cases(); for (int index = 0; index < caseGroups.size(); index++) { CaseGroupTree caseGroup = caseGroups.get(index); - if (caseGroup.labels().get(0).isFallThrough()) { - for (StatementTree statement : caseGroup.body()) { - if (statement instanceof VariableTree variable) { - reportIfAccessedFromLaterGroup(variable, caseGroups, index + 1); - } + if (!caseGroup.labels().get(0).isFallThrough()) { + continue; + } + for (StatementTree statement : caseGroup.body()) { + if (statement instanceof VariableTree variable) { + reportIfAccessedFromLaterGroup(variable, caseGroups, index + 1); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/LoopExecutingAtMostOnceCheck.java b/java-checks/src/main/java/org/sonar/java/checks/LoopExecutingAtMostOnceCheck.java index 98052349dc6..8ce88221c58 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/LoopExecutingAtMostOnceCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/LoopExecutingAtMostOnceCheck.java @@ -160,14 +160,18 @@ private static boolean hasPredecessorInBlock(CFG.Block block, Tree loop) { } else { Tree predecessorFirstElement = predecessorElements.get(0); - if (!isForStatementInitializer(predecessorFirstElement, loop)) { - if (isForStatementUpdate(predecessorFirstElement, loop)) { - return !predecessor.predecessors().isEmpty(); - } - - if (isDescendant(predecessorFirstElement, loop)) { - return true; - } + if (isForStatementInitializer(predecessorFirstElement, loop)) { + // skip 'for' loops initializers + continue; + } + + if (isForStatementUpdate(predecessorFirstElement, loop)) { + // there is no way to reach the 'for' loop update + return !predecessor.predecessors().isEmpty(); + } + + if (isDescendant(predecessorFirstElement, loop)) { + return true; } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/MissingOverridesInRecordWithArrayComponentCheck.java b/java-checks/src/main/java/org/sonar/java/checks/MissingOverridesInRecordWithArrayComponentCheck.java index e6143589e5f..13e2e2e44c5 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/MissingOverridesInRecordWithArrayComponentCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/MissingOverridesInRecordWithArrayComponentCheck.java @@ -83,15 +83,16 @@ public static Optional inspectRecord(ClassTree tree) { boolean hashCodeIsOverridden = false; boolean toStringIsOverridden = false; for (Tree member : tree.members()) { - if (member.is(Tree.Kind.METHOD)) { - MethodTree method = (MethodTree) member; - if (EQUALS_MATCHER.matches(method)) { - equalsIsOverridden = true; - } else if (HASH_CODE_MATCHER.matches(method)) { - hashCodeIsOverridden = true; - } else if (TO_STRING_MATCHER.matches(method)) { - toStringIsOverridden = true; - } + if (!member.is(Tree.Kind.METHOD)) { + continue; + } + MethodTree method = (MethodTree) member; + if (EQUALS_MATCHER.matches(method)) { + equalsIsOverridden = true; + } else if (HASH_CODE_MATCHER.matches(method)) { + hashCodeIsOverridden = true; + } else if (TO_STRING_MATCHER.matches(method)) { + toStringIsOverridden = true; } } return computeMessage(equalsIsOverridden, hashCodeIsOverridden, toStringIsOverridden); diff --git a/java-checks/src/main/java/org/sonar/java/checks/ModifiersOrderCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ModifiersOrderCheck.java index 1575d68cd2d..f588de06576 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/ModifiersOrderCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/ModifiersOrderCheck.java @@ -146,12 +146,15 @@ private static List removalOfAllModifiers(ModifiersTre int numberModifiers = modifiersTree.size(); for (int i = 0; i < numberModifiers; i++) { ModifierTree current = modifiersTree.get(i); - if (!current.is(Tree.Kind.ANNOTATION)) { - if (i == (numberModifiers - 1)) { - removals.add(AnalyzerMessage.textSpanBetween(current, true, QuickFixHelper.nextToken(modifiersTree), false)); - } else { - removals.add(AnalyzerMessage.textSpanBetween(current, true, modifiersTree.get(i + 1), false)); - } + if (current.is(Tree.Kind.ANNOTATION)) { + continue; + } + if (i == (numberModifiers - 1)) { + // Last: remove last token and potential space + removals.add(AnalyzerMessage.textSpanBetween(current, true, QuickFixHelper.nextToken(modifiersTree), false)); + } else { + // Take into account neighboring modifiers (can be on different lines) + removals.add(AnalyzerMessage.textSpanBetween(current, true, modifiersTree.get(i + 1), false)); } } return removals; diff --git a/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java b/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java index f2f90eb7f11..89e4390b720 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/OptionalAsParameterCheck.java @@ -52,12 +52,14 @@ public void visitNode(Tree tree) { for (VariableTree parameter : methodTree.parameters()) { SymbolMetadata parameterMetadata = parameter.symbol().metadata(); - if (!parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.RequestParam") - && !parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.PathVariable")) { - TypeTree typeTree = parameter.type(); - Optional msg = expectedTypeInsteadOfOptional(typeTree.symbolType()); - msg.ifPresent(s -> reportIssue(typeTree, s)); + if (parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.RequestParam") + || parameterMetadata.isAnnotatedWith("org.springframework.web.bind.annotation.PathVariable")) { + continue; } + + TypeTree typeTree = parameter.type(); + Optional msg = expectedTypeInsteadOfOptional(typeTree.symbolType()); + msg.ifPresent(s -> reportIssue(typeTree, s)); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/PatternMatchUsingIfCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PatternMatchUsingIfCheck.java index 18942fb45df..ce1d538a9cb 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PatternMatchUsingIfCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PatternMatchUsingIfCheck.java @@ -217,20 +217,18 @@ private JavaQuickFix computeQuickFix(List cases, IfStatementTree topLevelI } private void writeCase(Case caze, StringBuilder sb, int baseIndent, boolean canLiftReturn) { - switch (caze) { - case PatternMatchCase patternMatchCase -> { - sb.append("case ").append(QuickFixHelper.contentForTree(patternMatchCase.pattern, context)); - if (!patternMatchCase.guards().isEmpty()) { - List guards = patternMatchCase.guards(); - sb.append(" when "); - join(guards, " && ", sb); - } + if (caze instanceof PatternMatchCase patternMatchCase) { + sb.append("case ").append(QuickFixHelper.contentForTree(patternMatchCase.pattern, context)); + if (!patternMatchCase.guards().isEmpty()) { + List guards = patternMatchCase.guards(); + sb.append(" when "); + join(guards, " && ", sb); } - case EqualityCase equalityCase -> { - sb.append("case "); - join(equalityCase.constants, ", ", sb); - } - default -> sb.append("default"); + } else if (caze instanceof EqualityCase equalityCase) { + sb.append("case "); + join(equalityCase.constants, ", ", sb); + } else { + sb.append("default"); } sb.append(" -> "); if (canLiftReturn) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java index fb07d4b38de..58e55a231c9 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PseudoRandomCheck.java @@ -194,14 +194,15 @@ static List tokenizeIdentifier(String identifier) { List words = new ArrayList<>(); Pattern splitPattern = Pattern.compile("(?=[A-Z])"); for (String part : identifier.split("_")) { - if (!part.isEmpty()) { - if (isAllUppercaseWithLetter(part)) { - words.add(part.toLowerCase(Locale.ROOT)); - } else { - for (String sub : splitPattern.split(part)) { - if (!sub.isEmpty()) { - words.add(sub.toLowerCase(Locale.ROOT)); - } + if (part.isEmpty()) { + continue; + } + if (isAllUppercaseWithLetter(part)) { + words.add(part.toLowerCase(Locale.ROOT)); + } else { + for (String sub : splitPattern.split(part)) { + if (!sub.isEmpty()) { + words.add(sub.toLowerCase(Locale.ROOT)); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/RedundantThrowsDeclarationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/RedundantThrowsDeclarationCheck.java index 169fdb7747a..e8ca9170644 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/RedundantThrowsDeclarationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/RedundantThrowsDeclarationCheck.java @@ -83,23 +83,24 @@ private void checkMethodThrownList(MethodTree methodTree, ListTree thr for (TypeTree typeTree : thrownList) { Type exceptionType = typeTree.symbolType(); - if (!exceptionType.isUnknown()) { - String fullyQualifiedName = exceptionType.fullyQualifiedName(); - if (!reported.contains(fullyQualifiedName)) { - String superTypeName = isSubclassOfAny(exceptionType, thrownList); - if (superTypeName != null && !exceptionType.isSubtypeOf("java.lang.RuntimeException")) { - reportIssueWithQuickfix(methodTree, typeTree, String.format( - "Remove the declaration of thrown exception '%s' which is a subclass of '%s'.", fullyQualifiedName, superTypeName)); - } else if (declaredMoreThanOnce(fullyQualifiedName, thrownList)) { - reportIssueWithQuickfix(methodTree, typeTree, String.format( - "Remove the redundant '%s' thrown exception declaration(s).", fullyQualifiedName)); - } else if (canNotBeThrown(methodTree, exceptionType, thrownExceptions) && (!isOverridableMethod || undocumentedExceptionNames.contains(exceptionType.name()))) { - reportIssueWithQuickfix(methodTree, typeTree, String.format( - "Remove the declaration of thrown exception '%s', as it cannot be thrown from %s's body.", fullyQualifiedName, - methodTreeType(methodTree))); - } - reported.add(fullyQualifiedName); + if (exceptionType.isUnknown()) { + continue; + } + String fullyQualifiedName = exceptionType.fullyQualifiedName(); + if (!reported.contains(fullyQualifiedName)) { + String superTypeName = isSubclassOfAny(exceptionType, thrownList); + if (superTypeName != null && !exceptionType.isSubtypeOf("java.lang.RuntimeException")) { + reportIssueWithQuickfix(methodTree, typeTree, String.format( + "Remove the declaration of thrown exception '%s' which is a subclass of '%s'.", fullyQualifiedName, superTypeName)); + } else if (declaredMoreThanOnce(fullyQualifiedName, thrownList)) { + reportIssueWithQuickfix(methodTree, typeTree, String.format( + "Remove the redundant '%s' thrown exception declaration(s).", fullyQualifiedName)); + } else if (canNotBeThrown(methodTree, exceptionType, thrownExceptions) && (!isOverridableMethod || undocumentedExceptionNames.contains(exceptionType.name()))) { + reportIssueWithQuickfix(methodTree, typeTree, String.format( + "Remove the declaration of thrown exception '%s', as it cannot be thrown from %s's body.", fullyQualifiedName, + methodTreeType(methodTree))); } + reported.add(fullyQualifiedName); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/SwitchCasesShouldBeCommaSeparatedCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SwitchCasesShouldBeCommaSeparatedCheck.java index d971968bbca..afcdbd158e7 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/SwitchCasesShouldBeCommaSeparatedCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/SwitchCasesShouldBeCommaSeparatedCheck.java @@ -53,20 +53,22 @@ public void visitNode(Tree tree) { for (CaseGroupTree aCase : switchExpression.cases()) { List labels = aCase.labels(); int size = labels.size(); - if (size > 1) { - Deque caseLabels = labels.stream() - .filter(label -> "case".equals(label.caseOrDefaultKeyword().text())) - .collect(Collectors.toCollection(ArrayDeque::new)); + if (size == 1) { + continue; + } + + Deque caseLabels = labels.stream() + .filter(label -> "case".equals(label.caseOrDefaultKeyword().text())) + .collect(Collectors.toCollection(ArrayDeque::new)); - if (caseLabels.size() > 1) { - CaseLabelTree lastLabel = caseLabels.removeLast(); - ((DefaultJavaFileScannerContext) context).newIssue() - .forRule(this) - .onTree(lastLabel) - .withMessage(MESSAGE) - .withSecondaries(caseLabels.stream().map(label -> new JavaFileScannerContext.Location("", label)).toList()) - .report(); - } + if (caseLabels.size() > 1) { + CaseLabelTree lastLabel = caseLabels.removeLast(); + ((DefaultJavaFileScannerContext) context).newIssue() + .forRule(this) + .onTree(lastLabel) + .withMessage(MESSAGE) + .withSecondaries(caseLabels.stream().map(label -> new JavaFileScannerContext.Location("", label)).toList()) + .report(); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java b/java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java index d85cf3cd341..5696f4be99f 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java @@ -97,13 +97,15 @@ public void visitNode(Tree tree) { } private static boolean isNewAutocloseableOrBuilder(Tree tree, JavaFileScannerContext context) { - return switch (tree) { - case NewClassTree newClass -> newClass.symbolType().isSubtypeOf("java.lang.AutoCloseable"); - case MethodInvocationTree mit -> AUTOCLOSEABLE_FACTORY_MATCHER.matches(mit) || + if (tree instanceof NewClassTree newClass) { + return newClass.symbolType().isSubtypeOf("java.lang.AutoCloseable"); + } else if (tree instanceof MethodInvocationTree mit) { + return AUTOCLOSEABLE_FACTORY_MATCHER.matches(mit) || (context.getJavaVersion().isJava21Compatible() && AUTOCLOSEABLE_JAVA21_MATCHER.matches(mit)) || (context.getJavaVersion().isJava26Compatible() && AUTOCLOSEABLE_JAVA26_MATCHER.matches(mit)); - default -> false; - }; + } else { + return false; + } } private static boolean isFollowedByTryWithFinally(Tree tree) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/UseOfSequentialForSequentialGathererCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UseOfSequentialForSequentialGathererCheck.java index 4ee14530c71..245724e40d7 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UseOfSequentialForSequentialGathererCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UseOfSequentialForSequentialGathererCheck.java @@ -88,19 +88,20 @@ public void visitNode(Tree tree) { MethodInvocationTree mit = (MethodInvocationTree) tree; for (Case caze : CASES) { - if (caze.matcher.matches(mit)) { - var argumentPredicate = caze.pred; - var issues = argumentPredicate.predicate.apply(mit.arguments().get(argumentPredicate.argIdx)); - if (!issues.isEmpty()) { - - var secondaries = issues.subList(1, issues.size()) - .stream() - .map(element -> new JavaFileScannerContext.Location("", element)) - .toList(); - - context.reportIssue(this, issues.get(0), caze.msg, secondaries, null); - return; - } + if (!caze.matcher.matches(mit)) { + continue; + } + var argumentPredicate = caze.pred; + var issues = argumentPredicate.predicate.apply(mit.arguments().get(argumentPredicate.argIdx)); + if (!issues.isEmpty()) { + + var secondaries = issues.subList(1, issues.size()) + .stream() + .map(element -> new JavaFileScannerContext.Location("", element)) + .toList(); + + context.reportIssue(this, issues.get(0), caze.msg, secondaries, null); + return; } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java b/java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java index 9f80f174d32..22fa4f5384a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/UtilityClassWithPublicConstructorCheck.java @@ -128,12 +128,15 @@ private static boolean hasPublicAccess(AnnotationTree annotation) { } private static boolean isAccessLevelNotPublic(ExpressionTree tree) { - String valueName = switch (tree) { - case MemberSelectExpressionTree mset -> mset.identifier().name(); - case IdentifierTree identifier -> identifier.name(); - default -> null; - }; - return valueName != null && !"PUBLIC".equals(valueName); + String valueName; + if (tree instanceof MemberSelectExpressionTree mset) { + valueName = mset.identifier().name(); + } else if (tree instanceof IdentifierTree identifier) { + valueName = identifier.name(); + } else { + return false; + } + return !"PUBLIC".equals(valueName); } private static List computeQuickFixes(ClassTree classTree) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java index 9def709ec63..2c2e755afcb 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java +++ b/java-checks/src/main/java/org/sonar/java/checks/helpers/StringUtils.java @@ -60,11 +60,14 @@ public static int countMatches(@Nullable String string, @Nullable String pattern public static String[] flatten(Object ... args) { List result = new ArrayList<>(); for (Object arg : args) { - switch (arg) { - case String s -> result.add(s); - case String[] arr -> Collections.addAll(result, arr); - case Collection col -> result.addAll((Collection) col); - default -> throw new IllegalArgumentException("Unsupported argument type: " + arg.getClass()); + if (arg instanceof String s) { + result.add(s); + } else if (arg instanceof String[] arr) { + Collections.addAll(result, arr); + } else if (arg instanceof Collection col) { + result.addAll((Collection) col); + } else { + throw new IllegalArgumentException("Unsupported argument type: " + arg.getClass()); } } return result.toArray(new String[0]); diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/AndroidMobileDatabaseEncryptionKeysCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/AndroidMobileDatabaseEncryptionKeysCheck.java index a9cd3d379bc..d26a20ee86b 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/AndroidMobileDatabaseEncryptionKeysCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/AndroidMobileDatabaseEncryptionKeysCheck.java @@ -92,8 +92,8 @@ public void visitNode(Tree tree) { private void reportIssueIfHardCoded(MethodInvocationTree mit, String argName) { Arguments arguments = mit.arguments(); - int argIndex = arguments.size() == 1 ? 0 : 1; - reportIssueIfHardCoded(arguments.get(argIndex), argName); + ExpressionTree passwordArg = arguments.size() == 1 ? arguments.get(0) : arguments.get(1); + reportIssueIfHardCoded(passwordArg, argName); } private void reportIssueIfHardCoded(ExpressionTree expressionTree, String messageArg) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/serialization/SerialVersionUidInRecordCheck.java b/java-checks/src/main/java/org/sonar/java/checks/serialization/SerialVersionUidInRecordCheck.java index 5883deabcd5..c8fb397629f 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/serialization/SerialVersionUidInRecordCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/serialization/SerialVersionUidInRecordCheck.java @@ -41,12 +41,13 @@ public void visitNode(Tree tree) { return; } for (Tree member : targetRecord.members()) { - if (member.is(Tree.Kind.VARIABLE)) { - VariableTree variable = (VariableTree) member; - if (isSerialVersionUIDField(variable) && setsTheValueToZero(variable)) { - reportIssue(variable, "Remove this redundant \"serialVersionUID\" field"); - return; - } + if (!member.is(Tree.Kind.VARIABLE)) { + continue; + } + VariableTree variable = (VariableTree) member; + if (isSerialVersionUIDField(variable) && setsTheValueToZero(variable)) { + reportIssue(variable, "Remove this redundant \"serialVersionUID\" field"); + return; } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java index e708bfad1fa..31f90cfd176 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/MissingPathVariableAnnotationCheck.java @@ -102,13 +102,14 @@ public void visitNode(Tree tree) { private static Set extractModelAttributeMethodParameter(List methods){ Set modelAttributeMethodParameter = new HashSet<>(); for (var method : methods) { - if (method.symbol().metadata().isAnnotatedWith(MODEL_ATTRIBUTE_ANNOTATION)) { - for (var parameter : method.parameters()) { - SymbolMetadata metadata = parameter.symbol().metadata(); - var arguments = metadata.valuesForAnnotation(PATH_VARIABLE_ANNOTATION); - if (arguments != null) { - modelAttributeMethodParameter.add(extractPathMethodParameters(parameter, arguments).value); - } + if (!method.symbol().metadata().isAnnotatedWith(MODEL_ATTRIBUTE_ANNOTATION)) { + continue; + } + for (var parameter : method.parameters()) { + SymbolMetadata metadata = parameter.symbol().metadata(); + var arguments = metadata.valuesForAnnotation(PATH_VARIABLE_ANNOTATION); + if (arguments != null) { + modelAttributeMethodParameter.add(extractPathMethodParameters(parameter, arguments).value); } } } @@ -172,9 +173,11 @@ private void checkParametersAndPathTemplate(MethodTree method, Set model String fullyQualifiedName = ann.annotationType().symbolType().fullyQualifiedName(); var values = method.symbol().metadata().valuesForAnnotation(fullyQualifiedName); - if (values != null && MAPPING_ANNOTATIONS.contains(fullyQualifiedName)) { - templateVariables.add(new UriInfo<>(ann, templateVariablesFromMapping(values))); + if (values == null || !MAPPING_ANNOTATIONS.contains(fullyQualifiedName)) { + continue; } + + templateVariables.add(new UriInfo<>(ann, templateVariablesFromMapping(values))); } // we handle the case where a path variable doesn't match to uri parameter (/{aParam}/) @@ -293,14 +296,18 @@ private Set extractClassAndRecordProperties(MethodTree method) { for (var parameter : method.parameters()) { Type parameterType = parameter.type().symbolType(); - if (!parameterType.isUnknown() - && !isStandardDataType(parameterType) && !parameterType.isSubtypeOf(MAP) - && !requiresModelAttributeAnnotation(parameter.symbol().metadata())) { - if (parameterType.isSubtypeOf("java.lang.Record") && springWebVersion != SpringWebVersion.LESS_THAN_5_3) { - properties.addAll(extractRecordProperties(parameterType)); - } else if (parameterType.isClass()) { - properties.addAll(extractSetterProperties(parameterType)); - } + if (parameterType.isUnknown() + || isStandardDataType(parameterType) || parameterType.isSubtypeOf(MAP) + || requiresModelAttributeAnnotation(parameter.symbol().metadata())) { + continue; + } + + if (parameterType.isSubtypeOf("java.lang.Record") && springWebVersion != SpringWebVersion.LESS_THAN_5_3) { + // Extract record's components + properties.addAll(extractRecordProperties(parameterType)); + } else if (parameterType.isClass()) { + // Extract setter properties from the class + properties.addAll(extractSetterProperties(parameterType)); } } @@ -316,10 +323,14 @@ static Set extractSetterProperties(Type type) { // Extract properties from explicit setter methods for (Symbol member : typeSymbol.memberSymbols()) { - if (member.isMethodSymbol()) { - Symbol.MethodSymbol method = (Symbol.MethodSymbol) member; - isSetterLike(method).ifPresent(properties::add); + if (!member.isMethodSymbol()) { + continue; } + + Symbol.MethodSymbol method = (Symbol.MethodSymbol) member; + + // Check if it's a setter and extract a property name + isSetterLike(method).ifPresent(properties::add); } return properties; @@ -334,13 +345,17 @@ private static Set checkForLombokSetters(Symbol.TypeSymbol typeSymbol) { // Extract properties from fields if Lombok generates setters for (Symbol.VariableSymbol field : typeSymbol.memberSymbols().stream().filter(Symbol::isVariableSymbol).map(Symbol.VariableSymbol.class::cast).toList()) { - if (!field.isStatic() && !field.isFinal()) { - boolean hasFieldLevelSetter = field.metadata().annotations().stream() - .anyMatch(annotation -> "lombok.Setter".equals(annotation.symbol().type().fullyQualifiedName())); + if (field.isStatic() || field.isFinal()) { + continue; + } - if (hasLombokSetters || hasFieldLevelSetter) { - properties.add(field.name()); - } + // Check if field has @Setter annotation at field level + boolean hasFieldLevelSetter = field.metadata().annotations().stream() + .anyMatch(annotation -> "lombok.Setter".equals(annotation.symbol().type().fullyQualifiedName())); + + // Add property if class-level or field-level Lombok setter exists + if (hasLombokSetters || hasFieldLevelSetter) { + properties.add(field.name()); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java index 5e2e6605b8a..13821758b88 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/RedundantSpringAnnotationCheck.java @@ -82,15 +82,16 @@ public void visitNode(Tree tree) { for (RedundancyRule rule : REDUNDANCY_RULES) { List redundantAnnotations = annotationsByFqn.get(rule.redundantFqn); - if (redundantAnnotations != null) { - for (AnnotationTree redundantAnnotation : redundantAnnotations) { - for (String impliedByFqn : rule.impliedByFqns) { - List impliedByAnnotations = annotationsByFqn.get(impliedByFqn); - if (impliedByAnnotations != null && !impliedByAnnotations.isEmpty() - && passesSpecialCondition(rule, redundantAnnotation)) { - reportRedundancy(redundantAnnotation, impliedByAnnotations.get(0)); - break; - } + if (redundantAnnotations == null) { + continue; + } + for (AnnotationTree redundantAnnotation : redundantAnnotations) { + for (String impliedByFqn : rule.impliedByFqns) { + List impliedByAnnotations = annotationsByFqn.get(impliedByFqn); + if (impliedByAnnotations != null && !impliedByAnnotations.isEmpty() + && passesSpecialCondition(rule, redundantAnnotation)) { + reportRedundancy(redundantAnnotation, impliedByAnnotations.get(0)); + break; } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java index 89e78aad884..3a677ba3f22 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java @@ -246,8 +246,8 @@ static Placeholder parse(ParseCtx ctx, int startIdx) { } else { state = new DefaultValue(0, expr, idx + 1); } - } else if (state instanceof DefaultValue(int nestingLevel, String expr, int startDefault) && current == '}' && nestingLevel == 0) { - return new Placeholder(ctx.offset(), new Range(startIdx, idx + 1), expr, expressionSource.substring(startDefault, idx).trim()); + } else if (state instanceof DefaultValue d && current == '}' && d.nestingLevel == 0) { + return new Placeholder(ctx.offset(), new Range(startIdx, idx + 1), d.expr, expressionSource.substring(d.startDefault, idx).trim()); } else if (state instanceof DefaultValue d) { if (SpEL.matchPrefix(expressionSource, idx)) { SpEL.parse(ctx, idx); diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java index 35fa9e6bf1b..8734589db61 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java @@ -81,10 +81,12 @@ private static void collectAutowiredFields(Tree tree, Map for(String annotation: AUTOWIRED_ANNOTATIONS) { List annotationValues = metadata.valuesForAnnotation(annotation); if (annotationValues != null) { - if (annotationValues.stream().noneMatch(SpringConfigurationWithAutowiredFieldsCheck::isRequiredFalse) - || variable.initializer() == null) { - autowiredFields.put(variableSymbol, variable); + if (annotationValues.stream().anyMatch(SpringConfigurationWithAutowiredFieldsCheck::isRequiredFalse) + && variable.initializer() != null) { + // Common pattern used to define a default value. + continue; } + autowiredFields.put(variableSymbol, variable); } } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/tests/ParameterizedTestCheck.java b/java-checks/src/main/java/org/sonar/java/checks/tests/ParameterizedTestCheck.java index 1b5dcfe4058..6a066d60cbe 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/tests/ParameterizedTestCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/tests/ParameterizedTestCheck.java @@ -73,26 +73,30 @@ public void visitNode(Tree tree) { Set handled = new HashSet<>(); for (int i = 0; i < methods.size(); i++) { MethodTree method = methods.get(i); - if (!handled.contains(method)) { - List methodBody = method.block().body(); - CollectAndIgnoreLiterals collectAndIgnoreLiterals = new CollectAndIgnoreLiterals(); - - List equivalentMethods = new ArrayList<>(); - - for (int j = i + 1; j < methods.size(); j++) { - MethodTree otherMethod = methods.get(j); - if (!handled.contains(otherMethod)) { - boolean areEquivalent = SyntacticEquivalence.areEquivalent(methodBody, otherMethod.block().body(), collectAndIgnoreLiterals); - if (areEquivalent) { - equivalentMethods.add(otherMethod); - collectAndIgnoreLiterals.finishCollect(); - } - collectAndIgnoreLiterals.clearCurrentNodes(); + if (handled.contains(method)) { + continue; + } + List methodBody = method.block().body(); + // In addition to filtering literals, we want to count the number of differences since they will represent the number of parameter + // that would be required to transform the tests to a single parametrized one. + CollectAndIgnoreLiterals collectAndIgnoreLiterals = new CollectAndIgnoreLiterals(); + + List equivalentMethods = new ArrayList<>(); + + for (int j = i + 1; j < methods.size(); j++) { + MethodTree otherMethod = methods.get(j); + if (!handled.contains(otherMethod)) { + boolean areEquivalent = SyntacticEquivalence.areEquivalent(methodBody, otherMethod.block().body(), collectAndIgnoreLiterals); + if (areEquivalent) { + // If methods where not equivalent, we don't want to pollute the set of node to parameterize. + equivalentMethods.add(otherMethod); + collectAndIgnoreLiterals.finishCollect(); } + collectAndIgnoreLiterals.clearCurrentNodes(); } - - reportIfIssue(handled, method, collectAndIgnoreLiterals, equivalentMethods); } + + reportIfIssue(handled, method, collectAndIgnoreLiterals, equivalentMethods); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java index 44cc2dba73c..49279da87c5 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java @@ -52,9 +52,11 @@ public void visitNode(Tree tree) { VariableTree variableTree = (VariableTree) member; Symbol symbol = variableTree.symbol(); if ((isTestNameOrTemporaryFolderRule(symbol) || hasTempDirAnnotation(symbol)) && symbol.usages().isEmpty()) { - if (!isAbstract || ModifiersUtils.hasModifier(variableTree.modifiers(), Modifier.PRIVATE)) { - reportIssue(variableTree.simpleName(), "Remove this unused \"" + getSymbolType(symbol) + "\"."); + // if class is abstract, then we need to check modifier - if not private, then it's okay + if (isAbstract && !ModifiersUtils.hasModifier(variableTree.modifiers(), Modifier.PRIVATE)) { + continue; } + reportIssue(variableTree.simpleName(), "Remove this unused \"" + getSymbolType(symbol) + "\"."); } } else if (member.is(Tree.Kind.METHOD, Tree.Kind.CONSTRUCTOR)) { checkJUnit5((MethodTree) member); diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/utils/AnnotationFieldReferenceFinder.java b/java-checks/src/main/java/org/sonar/java/checks/unused/utils/AnnotationFieldReferenceFinder.java index ab706332f1f..f7c12a44233 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/utils/AnnotationFieldReferenceFinder.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/utils/AnnotationFieldReferenceFinder.java @@ -72,7 +72,7 @@ private AnnotationFieldReferenceFinder(HashMap fieldNameTo * Constructs an instance of this visitor that looks for references of the given fields inside annotations. */ public static AnnotationFieldReferenceFinder findReferencesTo(Collection fields) { - var fieldNameToVariableTree = HashMap.newHashMap(fields.size()); + var fieldNameToVariableTree = new HashMap(fields.size()); for (var variable : fields) { var fieldName = variable.simpleName().name(); diff --git a/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java b/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java index 488771bde49..9318e47bd33 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java @@ -409,6 +409,7 @@ public List values() { private Object convertAnnotationValue(Object value) { return switch (value) { + case null -> value; case IVariableBinding iVariableBinding -> sema.variableSymbol(iVariableBinding); case ITypeBinding iTypeBinding -> sema.typeSymbol(iTypeBinding); case IAnnotationBinding iAnnotationBinding -> sema.annotation(iAnnotationBinding); diff --git a/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java b/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java index 304e792cece..2471b5c823d 100644 --- a/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java +++ b/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java @@ -127,7 +127,10 @@ interface I { private void method() { class B { - Object obj = (I) () -> { + Object obj = new I() { + @Override + public void foo() { + } }; } } diff --git a/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java b/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java index 6102da4c3c5..7b56b74a69d 100644 --- a/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java +++ b/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java @@ -164,7 +164,12 @@ void should_not_fail_whole_analysis_upon_parse_error_and_notify_audit_listeners( @Test void should_handle_analysis_cancellation() { - JavaFileScanner visitor = spy((JavaFileScanner) context -> JavaAstScannerTest.this.context.setCancelled(true)); + JavaFileScanner visitor = spy(new JavaFileScanner() { + @Override + public void scanFile(JavaFileScannerContext context) { + JavaAstScannerTest.this.context.setCancelled(true); + } + }); scanTwoFilesWithVisitor(visitor, false, false); diff --git a/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java b/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java index 8c6cf1cbad7..c74794de66b 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java @@ -856,9 +856,17 @@ private void assertResultsOfParsing(List results, List inputFiles = Arrays.asList(TestUtils.inputFile("src/test/files/metrics/Classes.java"), TestUtils.inputFile("src/test/files/metrics/Methods.java")); - BiConsumer action = spy((BiConsumer) (inputFile, result) -> { + BiConsumer action = spy(new BiConsumer() { + @Override + public void accept(InputFile inputFile, JParserConfig.Result result) { + } + }); + BooleanSupplier isCanceled = spy(new BooleanSupplier() { + @Override + public boolean getAsBoolean() { + return false; + } }); - BooleanSupplier isCanceled = spy((BooleanSupplier) () -> false); FILE_BY_FILE .create(MAXIMUM_SUPPORTED_JAVA_VERSION, DEFAULT_CLASSPATH) From 4a4b22f90f107d81755fc710964f0e3ceb018fbb Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Thu, 27 Aug 2026 16:02:38 +0200 Subject: [PATCH 10/10] Fix SonarQube Quality Gate issues - Rename local 'context' variable to 'scannerContext' to fix S1117 (variable shadowing) - Remove unused JavaFileScannerContext import to fix S1128 - Restore load-bearing comments for S1186 exemptions in test files - Merge redundant 'case null' and 'default' switch arms in JSymbolMetadata Co-Authored-By: Claude Opus 4.6 --- .../java/checks/verifier/internal/JavaCheckVerifierTest.java | 1 - .../src/main/java/org/sonar/java/model/JSymbolMetadata.java | 3 +-- .../java/org/sonar/java/DefaultJavaResourceLocatorTest.java | 1 + .../src/test/java/org/sonar/java/ast/JavaAstScannerTest.java | 4 ++-- .../src/test/java/org/sonar/java/model/JParserTest.java | 1 + 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java b/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java index 5baac685309..d732eff40a2 100644 --- a/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java +++ b/java-checks-testkit/src/test/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifierTest.java @@ -44,7 +44,6 @@ import org.sonar.java.reporting.JavaQuickFix; import org.sonar.java.reporting.JavaTextEdit; import org.sonar.plugins.java.api.JavaFileScanner; -import org.sonar.plugins.java.api.JavaFileScannerContext; import org.sonar.plugins.java.api.caching.CacheContext; import org.sonar.plugins.java.api.caching.JavaReadCache; import org.sonar.plugins.java.api.caching.JavaWriteCache; diff --git a/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java b/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java index 9318e47bd33..2b9c5b6b862 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JSymbolMetadata.java @@ -409,7 +409,6 @@ public List values() { private Object convertAnnotationValue(Object value) { return switch (value) { - case null -> value; case IVariableBinding iVariableBinding -> sema.variableSymbol(iVariableBinding); case ITypeBinding iTypeBinding -> sema.typeSymbol(iTypeBinding); case IAnnotationBinding iAnnotationBinding -> sema.annotation(iAnnotationBinding); @@ -420,7 +419,7 @@ private Object convertAnnotationValue(Object value) { } yield result; } - default -> value; + case null, default -> value; }; } } diff --git a/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java b/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java index 2471b5c823d..d01c4f3c101 100644 --- a/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java +++ b/java-frontend/src/test/java/org/sonar/java/DefaultJavaResourceLocatorTest.java @@ -130,6 +130,7 @@ class B { Object obj = new I() { @Override public void foo() { + // empty implementation } }; } diff --git a/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java b/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java index 7b56b74a69d..9c7d66bc68e 100644 --- a/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java +++ b/java-frontend/src/test/java/org/sonar/java/ast/JavaAstScannerTest.java @@ -451,8 +451,8 @@ void scanWithoutParsing_filters_out_the_files_that_could_be_successfully_scanned @Test void test_modifyCompilationUnit_modify_ast() { - var check = (JavaFileScanner) context -> { - CompilationUnitTree tree = context.getTree(); + var check = (JavaFileScanner) scannerContext -> { + CompilationUnitTree tree = scannerContext.getTree(); ClassTreeImpl classTree = (ClassTreeImpl) tree.types().get(0); assertThat(classTree.simpleName().symbol().isUnknown()).isTrue(); }; diff --git a/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java b/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java index c74794de66b..57d8dfeb19b 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/JParserTest.java @@ -859,6 +859,7 @@ void test_is_canceled_is_called_before_each_action_file_by_file() { BiConsumer action = spy(new BiConsumer() { @Override public void accept(InputFile inputFile, JParserConfig.Result result) { + // Do nothing } }); BooleanSupplier isCanceled = spy(new BooleanSupplier() {