Skip to content

Commit 207cac2

Browse files
SONARJAVA-6096 Group import declarations by specificity (#5457)
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent b421292 commit 207cac2

10 files changed

Lines changed: 351 additions & 14 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S8445",
3+
"hasTruePositives": true,
4+
"falseNegatives": 0,
5+
"falsePositives": 0
6+
}

java-checks-common/src/test/java/org/sonar/java/checks/helpers/ExpressionsHelperTest.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@
1919
import java.lang.reflect.Constructor;
2020
import javax.annotation.Nullable;
2121
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.CsvSource;
2224
import org.sonar.plugins.java.api.semantic.Symbol;
25+
import org.sonar.plugins.java.api.tree.CompilationUnitTree;
2326
import org.sonar.plugins.java.api.tree.ExpressionStatementTree;
2427
import org.sonar.plugins.java.api.tree.ExpressionTree;
2528
import org.sonar.plugins.java.api.tree.IdentifierTree;
29+
import org.sonar.plugins.java.api.tree.ImportTree;
2630
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
2731
import org.sonar.plugins.java.api.tree.MethodTree;
2832

@@ -40,6 +44,33 @@ void private_constructor() throws Exception {
4044
constructor.newInstance();
4145
}
4246

47+
@Test
48+
void concatenate_null() {
49+
assertThat(ExpressionsHelper.concatenate(null)).isEmpty();
50+
}
51+
52+
@ParameterizedTest
53+
@CsvSource({
54+
"import A;, A",
55+
"import java.util.List;, java.util.List",
56+
"import java.util.regex.Pattern;, java.util.regex.Pattern",
57+
"import java.util.*;, java.util.*",
58+
"import static java.util.Collections.emptyList;, java.util.Collections.emptyList",
59+
"import static java.util.Collections.*;, java.util.Collections.*",
60+
"import module java.base;, java.base"
61+
})
62+
void concatenate(String importStatement, String expected) {
63+
String code = importStatement + "\nclass C {}";
64+
ExpressionTree qualifiedId = getImportQualifiedIdentifier(code, 0);
65+
assertThat(ExpressionsHelper.concatenate(qualifiedId)).isEqualTo(expected);
66+
}
67+
68+
private ExpressionTree getImportQualifiedIdentifier(String code, int importIndex) {
69+
CompilationUnitTree compilationUnit = parse(code);
70+
ImportTree importTree = (ImportTree) compilationUnit.imports().get(importIndex);
71+
return (ExpressionTree) importTree.qualifiedIdentifier();
72+
}
73+
4374
@Test
4475
void simpleAssignment() {
4576
String code = newCode( "int foo() {",
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package checks;
2+
3+
// Basic violation: package import after single-type import
4+
import java.util.List; // Compliant
5+
import java.io.*; // Noncompliant {{Reorder this on-demand package import to come before single-type imports.}}
6+
// ^^^^^^^^^
7+
import java.util.Map; // Compliant
8+
9+
// Static import violations
10+
import static java.lang.Math.PI; // Compliant
11+
import static java.util.Collections.*; // Noncompliant {{Reorder this static on-demand package import to come before static single-type imports.}}
12+
// ^^^^^^^^^^^^^^^^^^^^^^^
13+
// Module import violation - comes after static imports
14+
import module java.base; // Noncompliant {{Reorder this module import to come before static on-demand package imports.}}
15+
16+
// These are compliant since they come after module import (which resets the ordering)
17+
import java.sql.*; // Compliant
18+
import java.time.Instant; // Compliant
19+
import static java.lang.System.out; // Compliant
20+
21+
class ImportDeclarationOrderCheckSample {
22+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package checks;
2+
3+
// All imports in correct order - no violations expected
4+
5+
// Module imports
6+
import module java.base;
7+
8+
// Package imports (on-demand)
9+
import java.io.*;
10+
import java.sql.*;
11+
12+
// Single-type imports
13+
import java.util.List;
14+
import java.util.Map;
15+
import java.time.Instant;
16+
17+
// Static package imports (on-demand)
18+
import static java.util.Collections.*;
19+
20+
// Static single-type imports
21+
import static java.lang.Math.PI;
22+
import static java.lang.System.out;
23+
24+
25+
class ImportDeclarationOrderCheckSampleNoIssuesSample {
26+
}
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import java.util.ArrayList;
20+
import java.util.Collections;
21+
import java.util.List;
22+
import org.sonar.check.Rule;
23+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
24+
import org.sonar.plugins.java.api.JavaVersion;
25+
import org.sonar.plugins.java.api.JavaVersionAwareVisitor;
26+
import org.sonar.plugins.java.api.tree.CompilationUnitTree;
27+
import org.sonar.plugins.java.api.tree.ImportTree;
28+
import org.sonar.plugins.java.api.tree.Tree;
29+
30+
@Rule(key = "S8445")
31+
public class ImportDeclarationOrderCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
32+
33+
@Override
34+
public boolean isCompatibleWithJavaVersion(JavaVersion version) {
35+
return version.isJava25Compatible();
36+
}
37+
38+
@Override
39+
public List<Tree.Kind> nodesToVisit() {
40+
return Collections.singletonList(Tree.Kind.COMPILATION_UNIT);
41+
}
42+
43+
@Override
44+
public void visitNode(Tree tree) {
45+
CompilationUnitTree compilationUnit = (CompilationUnitTree) tree;
46+
47+
List<ImportTree> imports = new ArrayList<>();
48+
// Collect all import statements
49+
compilationUnit.imports()
50+
.stream()
51+
.filter(importTree -> importTree.is(Tree.Kind.IMPORT))
52+
.map(ImportTree.class::cast)
53+
.forEach(imports::add);
54+
55+
analyzeImportOrder(imports);
56+
}
57+
58+
private void analyzeImportOrder(List<ImportTree> imports) {
59+
if (imports.size() <= 1) {
60+
return;
61+
}
62+
63+
ImportType previousType = ImportType.SENTINEL_IMPORT;
64+
for (ImportTree importTree : imports) {
65+
ImportType currentType = classifyImport(importTree);
66+
67+
if (currentType.ordinal() < previousType.ordinal()) {
68+
String message = buildMessage(currentType, previousType);
69+
reportIssue(importTree.qualifiedIdentifier(), message);
70+
}
71+
72+
previousType = currentType;
73+
}
74+
}
75+
76+
private static String buildMessage(ImportType currentType, ImportType previousType) {
77+
return String.format("Reorder this %s import to come before %s imports.",
78+
currentType.getDescription(),
79+
previousType.getDescription());
80+
}
81+
82+
private static ImportType classifyImport(ImportTree importTree) {
83+
boolean isWildcard = isWildcardImport(importTree);
84+
85+
if (importTree.isModule()) {
86+
return ImportType.MODULE_IMPORT;
87+
}
88+
89+
if (importTree.isStatic()) {
90+
return isWildcard ? ImportType.STATIC_PACKAGE_IMPORT : ImportType.STATIC_SINGLE_TYPE_IMPORT;
91+
}
92+
93+
return isWildcard ? ImportType.PACKAGE_IMPORT : ImportType.SINGLE_TYPE_IMPORT;
94+
}
95+
96+
private static boolean isWildcardImport(ImportTree importTree) {
97+
return "*".equals(importTree.qualifiedIdentifier().lastToken().text());
98+
}
99+
100+
enum ImportType {
101+
SENTINEL_IMPORT("sentinel"),
102+
MODULE_IMPORT("module"),
103+
PACKAGE_IMPORT("on-demand package"),
104+
SINGLE_TYPE_IMPORT("single-type"),
105+
STATIC_PACKAGE_IMPORT("static on-demand package"),
106+
STATIC_SINGLE_TYPE_IMPORT("static single-type");
107+
108+
private final String description;
109+
110+
ImportType(String description) {
111+
this.description = description;
112+
}
113+
114+
public String getDescription() {
115+
return description;
116+
}
117+
}
118+
}

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
package org.sonar.java.checks;
1818

1919
import org.sonar.check.Rule;
20+
import org.sonar.java.checks.helpers.ExpressionsHelper;
2021
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
21-
import org.sonar.plugins.java.api.tree.IdentifierTree;
22+
import org.sonar.plugins.java.api.tree.ExpressionTree;
2223
import org.sonar.plugins.java.api.tree.ImportTree;
23-
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
2424
import org.sonar.plugins.java.api.tree.Tree;
2525
import org.sonar.plugins.java.api.tree.Tree.Kind;
2626

@@ -40,18 +40,9 @@ public void visitNode(Tree tree) {
4040
ImportTree importTree = (ImportTree) tree;
4141

4242
// See RSPEC-2208 : exception with static imports.
43-
if (fullQualifiedName(importTree.qualifiedIdentifier()).endsWith(".*") && !importTree.isStatic()) {
43+
String qualifiedName = ExpressionsHelper.concatenate((ExpressionTree) importTree.qualifiedIdentifier());
44+
if (qualifiedName.endsWith(".*") && !importTree.isStatic()) {
4445
reportIssue(importTree.qualifiedIdentifier(), "Explicitly import the specific classes needed.");
4546
}
4647
}
47-
48-
private static String fullQualifiedName(Tree tree) {
49-
if (tree.is(Tree.Kind.IDENTIFIER)) {
50-
return ((IdentifierTree) tree).name();
51-
} else if (tree.is(Tree.Kind.MEMBER_SELECT)) {
52-
MemberSelectExpressionTree m = (MemberSelectExpressionTree) tree;
53-
return fullQualifiedName(m.expression()) + "." + m.identifier().name();
54-
}
55-
throw new UnsupportedOperationException(String.format("Kind/Class '%s' not supported", tree.getClass()));
56-
}
5748
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks;
18+
19+
import org.junit.jupiter.api.DisplayName;
20+
import org.junit.jupiter.api.Test;
21+
import org.sonar.java.checks.verifier.CheckVerifier;
22+
23+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
24+
25+
class ImportDeclarationOrderCheckTest {
26+
27+
@Test
28+
void basic_import_ordering() {
29+
CheckVerifier.newVerifier()
30+
.onFile(mainCodeSourcesPath("checks/ImportDeclarationOrderCheckSample.java"))
31+
.withCheck(new ImportDeclarationOrderCheck())
32+
.withJavaVersion(25)
33+
.verifyIssues();
34+
}
35+
36+
@Test
37+
void no_issue_on_java_24() {
38+
CheckVerifier.newVerifier()
39+
.onFile(mainCodeSourcesPath("checks/ImportDeclarationOrderCheckSample.java"))
40+
.withCheck(new ImportDeclarationOrderCheck())
41+
.withJavaVersion(24)
42+
.verifyNoIssues();
43+
}
44+
45+
@Test
46+
@DisplayName("The same imports but correctly ordered")
47+
void correctly_ordered_imports() {
48+
CheckVerifier.newVerifier()
49+
.onFile(mainCodeSourcesPath("checks/ImportDeclarationOrderCheckSampleNoIssuesSample.java"))
50+
.withCheck(new ImportDeclarationOrderCheck())
51+
.withJavaVersion(25)
52+
.verifyNoIssues();
53+
}
54+
55+
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<p>This rule raises an issue when import declarations are not organized into distinct groups based on their kind and specificity level.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>Import declarations should be organized into distinct groups based on their kind to improve readability and make shadowing behavior explicit. The
4+
grouping order should reflect specificity levels: module imports first (least specific), followed by on-demand package imports (intermediate
5+
specificity), single-type imports (most specific), then static on-demand imports, and finally single-static imports.</p>
6+
<p>When imports are mixed or ordered arbitrarily, it becomes harder to understand which declarations might shadow others. Java’s import shadowing
7+
rules mean that more specific imports can override less specific ones, so organizing imports by specificity makes these relationships clearer and
8+
improves code maintainability by providing a consistent, predictable structure.</p>
9+
<h2>How to fix it</h2>
10+
<p>Organize import declarations into the following groups, in this order:</p>
11+
<ol>
12+
<li> Module imports (e.g., <code>import module java.base;</code>) </li>
13+
<li> On-demand package imports (e.g., <code>import javax.swing.text.*;</code>) </li>
14+
<li> Single-type imports (e.g., <code>import java.util.List;</code>) </li>
15+
<li> Static on-demand imports (e.g., <code>import static java.util.Collections.*;</code>) </li>
16+
<li> Single-static imports (e.g., <code>import static java.util.regex.Pattern.compile;</code>) </li>
17+
</ol>
18+
<p>Separate each group with a blank line for better visual organization.</p>
19+
<h3>Code examples</h3>
20+
<h4>Noncompliant code example</h4>
21+
<pre data-diff-id="1" data-diff-type="noncompliant">
22+
import java.sql.Date;
23+
import module java.base;
24+
import static java.util.Collections.*;
25+
import javax.swing.text.*;
26+
import module java.desktop;
27+
import static java.util.regex.Pattern.compile;
28+
import java.util.List;
29+
30+
class Foo {
31+
// ...
32+
}
33+
</pre>
34+
<h4>Compliant solution</h4>
35+
<pre data-diff-id="1" data-diff-type="compliant">
36+
// Module imports
37+
import module java.base;
38+
import module java.desktop;
39+
40+
// On-demand package imports
41+
import javax.swing.text.*; // resolves the ambiguity of the simple name Element
42+
43+
// Single-type imports
44+
import java.sql.Date; // resolves the ambiguity of the simple name Date
45+
import java.util.List;
46+
47+
// Static on-demand imports
48+
import static java.util.Collections.*;
49+
50+
// Single-static imports
51+
import static java.util.regex.Pattern.compile;
52+
53+
class Foo {
54+
// ...
55+
}
56+
</pre>
57+
<h2>Resources</h2>
58+
<h3>Documentation</h3>
59+
<ul>
60+
<li> <a href="https://docs.oracle.com/javase/specs/jls/se25/html/jls-7.html#jls-7.5">Java Language Specification - Import Declarations</a> </li>
61+
<li> <a href="https://openjdk.org/jeps/511">JEP 511: Module Import Declarations</a> </li>
62+
</ul>
63+

0 commit comments

Comments
 (0)