diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8445.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8445.json new file mode 100644 index 00000000000..7339e14441a --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8445.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S8445", + "hasTruePositives": true, + "falseNegatives": 0, + "falsePositives": 0 +} \ No newline at end of file diff --git a/java-checks-common/src/test/java/org/sonar/java/checks/helpers/ExpressionsHelperTest.java b/java-checks-common/src/test/java/org/sonar/java/checks/helpers/ExpressionsHelperTest.java index e42975e3fb3..c2b1fed85e6 100644 --- a/java-checks-common/src/test/java/org/sonar/java/checks/helpers/ExpressionsHelperTest.java +++ b/java-checks-common/src/test/java/org/sonar/java/checks/helpers/ExpressionsHelperTest.java @@ -19,10 +19,14 @@ import java.lang.reflect.Constructor; import javax.annotation.Nullable; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.CompilationUnitTree; import org.sonar.plugins.java.api.tree.ExpressionStatementTree; import org.sonar.plugins.java.api.tree.ExpressionTree; import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.ImportTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.MethodTree; @@ -40,6 +44,33 @@ void private_constructor() throws Exception { constructor.newInstance(); } + @Test + void concatenate_null() { + assertThat(ExpressionsHelper.concatenate(null)).isEmpty(); + } + + @ParameterizedTest + @CsvSource({ + "import A;, A", + "import java.util.List;, java.util.List", + "import java.util.regex.Pattern;, java.util.regex.Pattern", + "import java.util.*;, java.util.*", + "import static java.util.Collections.emptyList;, java.util.Collections.emptyList", + "import static java.util.Collections.*;, java.util.Collections.*", + "import module java.base;, java.base" + }) + void concatenate(String importStatement, String expected) { + String code = importStatement + "\nclass C {}"; + ExpressionTree qualifiedId = getImportQualifiedIdentifier(code, 0); + assertThat(ExpressionsHelper.concatenate(qualifiedId)).isEqualTo(expected); + } + + private ExpressionTree getImportQualifiedIdentifier(String code, int importIndex) { + CompilationUnitTree compilationUnit = parse(code); + ImportTree importTree = (ImportTree) compilationUnit.imports().get(importIndex); + return (ExpressionTree) importTree.qualifiedIdentifier(); + } + @Test void simpleAssignment() { String code = newCode( "int foo() {", diff --git a/java-checks-test-sources/default/src/main/java/checks/ImportDeclarationOrderCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/ImportDeclarationOrderCheckSample.java new file mode 100644 index 00000000000..d6bd9730a84 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/ImportDeclarationOrderCheckSample.java @@ -0,0 +1,22 @@ +package checks; + +// Basic violation: package import after single-type import +import java.util.List; // Compliant +import java.io.*; // Noncompliant {{Reorder this on-demand package import to come before single-type imports.}} +// ^^^^^^^^^ +import java.util.Map; // Compliant + +// Static import violations +import static java.lang.Math.PI; // Compliant +import static java.util.Collections.*; // Noncompliant {{Reorder this static on-demand package import to come before static single-type imports.}} +// ^^^^^^^^^^^^^^^^^^^^^^^ +// Module import violation - comes after static imports +import module java.base; // Noncompliant {{Reorder this module import to come before static on-demand package imports.}} + +// These are compliant since they come after module import (which resets the ordering) +import java.sql.*; // Compliant +import java.time.Instant; // Compliant +import static java.lang.System.out; // Compliant + +class ImportDeclarationOrderCheckSample { +} diff --git a/java-checks-test-sources/default/src/main/java/checks/ImportDeclarationOrderCheckSampleNoIssuesSample.java b/java-checks-test-sources/default/src/main/java/checks/ImportDeclarationOrderCheckSampleNoIssuesSample.java new file mode 100644 index 00000000000..ee7bfdb173e --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/ImportDeclarationOrderCheckSampleNoIssuesSample.java @@ -0,0 +1,26 @@ +package checks; + +// All imports in correct order - no violations expected + +// Module imports +import module java.base; + +// Package imports (on-demand) +import java.io.*; +import java.sql.*; + +// Single-type imports +import java.util.List; +import java.util.Map; +import java.time.Instant; + +// Static package imports (on-demand) +import static java.util.Collections.*; + +// Static single-type imports +import static java.lang.Math.PI; +import static java.lang.System.out; + + +class ImportDeclarationOrderCheckSampleNoIssuesSample { +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/ImportDeclarationOrderCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ImportDeclarationOrderCheck.java new file mode 100644 index 00000000000..7abd4b69168 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/ImportDeclarationOrderCheck.java @@ -0,0 +1,118 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2025 SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaVersion; +import org.sonar.plugins.java.api.JavaVersionAwareVisitor; +import org.sonar.plugins.java.api.tree.CompilationUnitTree; +import org.sonar.plugins.java.api.tree.ImportTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S8445") +public class ImportDeclarationOrderCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor { + + @Override + public boolean isCompatibleWithJavaVersion(JavaVersion version) { + return version.isJava25Compatible(); + } + + @Override + public List nodesToVisit() { + return Collections.singletonList(Tree.Kind.COMPILATION_UNIT); + } + + @Override + public void visitNode(Tree tree) { + CompilationUnitTree compilationUnit = (CompilationUnitTree) tree; + + List imports = new ArrayList<>(); + // Collect all import statements + compilationUnit.imports() + .stream() + .filter(importTree -> importTree.is(Tree.Kind.IMPORT)) + .map(ImportTree.class::cast) + .forEach(imports::add); + + analyzeImportOrder(imports); + } + + private void analyzeImportOrder(List imports) { + if (imports.size() <= 1) { + return; + } + + ImportType previousType = ImportType.SENTINEL_IMPORT; + for (ImportTree importTree : imports) { + ImportType currentType = classifyImport(importTree); + + if (currentType.ordinal() < previousType.ordinal()) { + String message = buildMessage(currentType, previousType); + reportIssue(importTree.qualifiedIdentifier(), message); + } + + previousType = currentType; + } + } + + private static String buildMessage(ImportType currentType, ImportType previousType) { + return String.format("Reorder this %s import to come before %s imports.", + currentType.getDescription(), + previousType.getDescription()); + } + + private static ImportType classifyImport(ImportTree importTree) { + boolean isWildcard = isWildcardImport(importTree); + + if (importTree.isModule()) { + return ImportType.MODULE_IMPORT; + } + + if (importTree.isStatic()) { + return isWildcard ? ImportType.STATIC_PACKAGE_IMPORT : ImportType.STATIC_SINGLE_TYPE_IMPORT; + } + + return isWildcard ? ImportType.PACKAGE_IMPORT : ImportType.SINGLE_TYPE_IMPORT; + } + + private static boolean isWildcardImport(ImportTree importTree) { + return "*".equals(importTree.qualifiedIdentifier().lastToken().text()); + } + + enum ImportType { + SENTINEL_IMPORT("sentinel"), + MODULE_IMPORT("module"), + PACKAGE_IMPORT("on-demand package"), + SINGLE_TYPE_IMPORT("single-type"), + STATIC_PACKAGE_IMPORT("static on-demand package"), + STATIC_SINGLE_TYPE_IMPORT("static single-type"); + + private final String description; + + ImportType(String description) { + this.description = description; + } + + public String getDescription() { + return description; + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/WildcardImportsShouldNotBeUsedCheck.java b/java-checks/src/main/java/org/sonar/java/checks/WildcardImportsShouldNotBeUsedCheck.java index f837763f25a..0ccdfb16fd8 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/WildcardImportsShouldNotBeUsedCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/WildcardImportsShouldNotBeUsedCheck.java @@ -17,10 +17,10 @@ package org.sonar.java.checks; import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.ExpressionsHelper; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; -import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; import org.sonar.plugins.java.api.tree.ImportTree; -import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.Tree.Kind; @@ -40,18 +40,9 @@ public void visitNode(Tree tree) { ImportTree importTree = (ImportTree) tree; // See RSPEC-2208 : exception with static imports. - if (fullQualifiedName(importTree.qualifiedIdentifier()).endsWith(".*") && !importTree.isStatic()) { + String qualifiedName = ExpressionsHelper.concatenate((ExpressionTree) importTree.qualifiedIdentifier()); + if (qualifiedName.endsWith(".*") && !importTree.isStatic()) { reportIssue(importTree.qualifiedIdentifier(), "Explicitly import the specific classes needed."); } } - - private static String fullQualifiedName(Tree tree) { - if (tree.is(Tree.Kind.IDENTIFIER)) { - return ((IdentifierTree) tree).name(); - } else if (tree.is(Tree.Kind.MEMBER_SELECT)) { - MemberSelectExpressionTree m = (MemberSelectExpressionTree) tree; - return fullQualifiedName(m.expression()) + "." + m.identifier().name(); - } - throw new UnsupportedOperationException(String.format("Kind/Class '%s' not supported", tree.getClass())); - } } diff --git a/java-checks/src/test/java/org/sonar/java/checks/ImportDeclarationOrderCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/ImportDeclarationOrderCheckTest.java new file mode 100644 index 00000000000..53bd5620a6c --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/ImportDeclarationOrderCheckTest.java @@ -0,0 +1,55 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2025 SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class ImportDeclarationOrderCheckTest { + + @Test + void basic_import_ordering() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/ImportDeclarationOrderCheckSample.java")) + .withCheck(new ImportDeclarationOrderCheck()) + .withJavaVersion(25) + .verifyIssues(); + } + + @Test + void no_issue_on_java_24() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/ImportDeclarationOrderCheckSample.java")) + .withCheck(new ImportDeclarationOrderCheck()) + .withJavaVersion(24) + .verifyNoIssues(); + } + + @Test + @DisplayName("The same imports but correctly ordered") + void correctly_ordered_imports() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/ImportDeclarationOrderCheckSampleNoIssuesSample.java")) + .withCheck(new ImportDeclarationOrderCheck()) + .withJavaVersion(25) + .verifyNoIssues(); + } + +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8445.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8445.html new file mode 100644 index 00000000000..daef907884b --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8445.html @@ -0,0 +1,63 @@ +

This rule raises an issue when import declarations are not organized into distinct groups based on their kind and specificity level.

+

Why is this an issue?

+

Import declarations should be organized into distinct groups based on their kind to improve readability and make shadowing behavior explicit. The +grouping order should reflect specificity levels: module imports first (least specific), followed by on-demand package imports (intermediate +specificity), single-type imports (most specific), then static on-demand imports, and finally single-static imports.

+

When imports are mixed or ordered arbitrarily, it becomes harder to understand which declarations might shadow others. Java’s import shadowing +rules mean that more specific imports can override less specific ones, so organizing imports by specificity makes these relationships clearer and +improves code maintainability by providing a consistent, predictable structure.

+

How to fix it

+

Organize import declarations into the following groups, in this order:

+
    +
  1. Module imports (e.g., import module java.base;)
    2. +
  2. On-demand package imports (e.g., import javax.swing.text.*;)
    3. +
  3. Single-type imports (e.g., import java.util.List;)
    4. +
  4. Static on-demand imports (e.g., import static java.util.Collections.*;)
    5. +
  5. Single-static imports (e.g., import static java.util.regex.Pattern.compile;)
    6. +
+

Separate each group with a blank line for better visual organization.

+

Code examples

+

Noncompliant code example

+
+import java.sql.Date;
+import module java.base;
+import static java.util.Collections.*;
+import javax.swing.text.*;
+import module java.desktop;
+import static java.util.regex.Pattern.compile;
+import java.util.List;
+
+class Foo {
+    // ...
+}
+
+

Compliant solution

+
+// Module imports
+import module java.base;
+import module java.desktop;
+
+// On-demand package imports
+import javax.swing.text.*; // resolves the ambiguity of the simple name Element
+
+// Single-type imports
+import java.sql.Date; // resolves the ambiguity of the simple name Date
+import java.util.List;
+
+// Static on-demand imports
+import static java.util.Collections.*;
+
+// Single-static imports
+import static java.util.regex.Pattern.compile;
+
+class Foo {
+    // ...
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8445.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8445.json new file mode 100644 index 00000000000..fd4bc8c8507 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8445.json @@ -0,0 +1,24 @@ +{ + "title": "Import declarations should be grouped by specificity", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "2min" + }, + "tags": [ + "convention", + "java25" + ], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-8445", + "sqKey": "S8445", + "scope": "All", + "quickfix": "targeted", + "code": { + "impacts": { + "MAINTAINABILITY": "LOW" + }, + "attribute": "CONVENTIONAL" + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index c6ee6637bd6..ddecba36ba4 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -520,6 +520,7 @@ "S8346", "S8432", "S8433", - "S8444" + "S8444", + "S8445" ] }