From 2fc8dce90d20c866281e92ef338c2d2212bef634 Mon Sep 17 00:00:00 2001 From: Romain Brenguier Date: Mon, 3 Aug 2026 14:14:55 +0200 Subject: [PATCH] Implement new rule S2330 Detect array covariance where an array of a derived type is assigned to a variable declared as an array of its base type, which can lead to ArrayStoreException at runtime. --- .../checks/ArrayCovarianceCheckSample.java | 199 ++++++++++++++++++ .../java/checks/ArrayCovarianceCheck.java | 133 ++++++++++++ .../java/checks/ArrayCovarianceCheckTest.java | 33 +++ .../org/sonar/l10n/java/rules/java/S2330.html | 51 +++++ .../org/sonar/l10n/java/rules/java/S2330.json | 23 ++ .../main/resources/profiles/Sonar_way/S2330 | 0 6 files changed, 439 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/ArrayCovarianceCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/ArrayCovarianceCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/ArrayCovarianceCheckTest.java create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2330.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2330.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S2330 diff --git a/java-checks-test-sources/default/src/main/java/checks/ArrayCovarianceCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/ArrayCovarianceCheckSample.java new file mode 100644 index 00000000000..60829c3d858 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/ArrayCovarianceCheckSample.java @@ -0,0 +1,199 @@ +package checks; + +import java.io.Serializable; +import java.util.function.Supplier; + +class ArrayCovarianceCheckSample { + + abstract static class Fruit {} + static class Apple extends Fruit {} + static class Orange extends Fruit {} + + abstract static class Shape {} + static class Circle extends Shape {} + + static class A {} + static class B extends A {} + static class C extends B {} + + // --- Noncompliant: variable declarations with new array creation --- + + void variableDeclarations() { + Fruit[] fruits1 = new Apple[10]; // Noncompliant {{Use the type of the actual array element here; array covariance can lead to ArrayStoreException at runtime.}} +// ^^^^^^^^^^^^^ + Object[] objects = new String[5]; // Noncompliant + Number[] numbers = new Integer[4]; // Noncompliant + CharSequence[] seqs = new String[3]; // Noncompliant + Serializable[] items = new String[2]; // Noncompliant + A[] deepHierarchy = new C[3]; // Noncompliant + } + + // --- Noncompliant: variable initialized from existing subtype array reference --- + + void variableFromReference() { + Apple[] apples = new Apple[5]; + Fruit[] fruits = apples; // Noncompliant + } + + // --- Noncompliant: field-level covariance --- + + Shape[] shapeField = new Circle[2]; // Noncompliant + + // --- Noncompliant: assignments --- + + void assignments() { + Fruit[] fruits; + fruits = new Apple[5]; // Noncompliant + Object[] objects; + objects = new Integer[3]; // Noncompliant + } + + // --- Noncompliant: return statements --- + + Fruit[] returnCovariant() { + return new Apple[1]; // Noncompliant + } + + Object[] returnCovariantJdk() { + return new String[2]; // Noncompliant + } + + // --- Noncompliant: method arguments --- + + void acceptFruits(Fruit[] fruits) {} + void acceptObjects(Object[] objects) {} + + void methodArguments() { + acceptFruits(new Apple[1]); // Noncompliant + acceptObjects(new String[1]); // Noncompliant + } + + // --- Noncompliant: constructor arguments --- + + static class Container { + Container(Fruit[] fruits) {} + Container(int x, Object[] objects) {} + } + + void constructorArguments() { + new Container(new Apple[1]); // Noncompliant + new Container(1, new String[2]); // Noncompliant + } + + // --- Noncompliant: lambda return --- + + void lambdaReturn() { + Supplier s = () -> { + return new Apple[1]; // Noncompliant + }; + } + + // --- Noncompliant: switch expression --- + + void switchExpression(int code) { + Apple[] apples = new Apple[1]; + Fruit[] result = switch (code) { + case 0 -> apples; // Noncompliant + default -> null; + }; + } + + // --- Noncompliant: yield statement --- + + void yieldStatement(int code) { + Apple[] apples = new Apple[1]; + Fruit[] result = switch (code) { + case 0 -> null; + default -> { + yield apples; // Noncompliant + } + }; + } + + // --- Compliant: same-type arrays --- + + void sameType() { + Apple[] apples = new Apple[10]; // Compliant + String[] strings = new String[5]; // Compliant + Fruit[] fruits = new Fruit[3]; // Compliant + } + + // --- Compliant: primitive arrays --- + + void primitiveArrays() { + int[] numbers = new int[4]; // Compliant + double[] doubles = new double[3]; // Compliant + } + + // --- Compliant: null assignment --- + + void nullAssignment() { + Object[] objects = null; // Compliant + } + + // --- Compliant: method return matching declared type --- + + Apple[] returnSameType() { + return new Apple[1]; // Compliant + } + + // --- Compliant: method argument matching parameter type --- + + void acceptApples(Apple[] apples) {} + + void methodArgumentsSameType() { + acceptApples(new Apple[1]); // Compliant + } + + // --- Compliant: multi-dimensional same-type --- + + void multiDimensional() { + String[][] matrix = new String[3][3]; // Compliant + } + + // --- Compliant: base type array creation with initializer --- + + void baseTypeInitializer() { + Number[] numbers = new Number[] { 1, 2.0 }; // Compliant + } + + // --- Compliant: void return --- + + void voidReturn() { + return; // Compliant + } + + // --- Compliant: non-array types --- + + void nonArrayTypes() { + Object o = "hello"; // Compliant + Number n = 42; // Compliant + } + + // --- Compliant: lambda returning matching type --- + + void lambdaCompliant() { + Supplier s = () -> { + return new Apple[1]; // Compliant + }; + } + + // --- Compliant: switch expression with matching types --- + + void switchCompliant(int code, Fruit[] fruits) { + Fruit[] result = switch (code) { + case 0 -> fruits; // Compliant + default -> null; + }; + } + + // --- Compliant: switch statement yield (not expression) --- + + void switchStatement(int code) { + switch (code) { + default -> doNothing(); + }; + } + + private void doNothing() {} +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/ArrayCovarianceCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ArrayCovarianceCheck.java new file mode 100644 index 00000000000..35d7748df8b --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/ArrayCovarianceCheck.java @@ -0,0 +1,133 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * 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.List; +import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.Type; +import org.sonar.plugins.java.api.tree.Arguments; +import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.LambdaExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.NewClassTree; +import org.sonar.plugins.java.api.tree.ReturnStatementTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.VariableTree; +import org.sonar.plugins.java.api.tree.YieldStatementTree; + +@Rule(key = "S2330") +public class ArrayCovarianceCheck extends IssuableSubscriptionVisitor { + + private static final String MESSAGE = "Use the type of the actual array element here; array covariance can lead to ArrayStoreException at runtime."; + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.VARIABLE, Tree.Kind.ASSIGNMENT, Tree.Kind.RETURN_STATEMENT, + Tree.Kind.YIELD_STATEMENT, Tree.Kind.METHOD_INVOCATION, Tree.Kind.NEW_CLASS); + } + + @Override + public void visitNode(Tree tree) { + switch (tree.kind()) { + case VARIABLE -> visitVariable((VariableTree) tree); + case ASSIGNMENT -> { + var assignment = (AssignmentExpressionTree) tree; + checkArrayCovariance(assignment.variable().symbolType(), assignment.expression()); + } + case RETURN_STATEMENT -> visitReturnStatement((ReturnStatementTree) tree); + case YIELD_STATEMENT -> visitYieldStatement((YieldStatementTree) tree); + case METHOD_INVOCATION -> { + var invocation = (MethodInvocationTree) tree; + visitInvocation(invocation.methodSymbol(), invocation.arguments()); + } + case NEW_CLASS -> { + var invocation = (NewClassTree) tree; + visitInvocation(invocation.methodSymbol(), invocation.arguments()); + } + default -> { + // do nothing + } + } + } + + private void visitVariable(VariableTree tree) { + var initializer = tree.initializer(); + if (initializer != null) { + checkArrayCovariance(tree.symbol().type(), initializer); + } + } + + private void visitReturnStatement(ReturnStatementTree tree) { + var expression = tree.expression(); + if (expression == null) { + return; + } + Tree enclosing = ExpressionUtils.getEnclosingTree(tree, Tree.Kind.METHOD, Tree.Kind.LAMBDA_EXPRESSION); + if (enclosing != null) { + var lhsType = enclosing instanceof LambdaExpressionTree lambda + ? lambda.symbol().returnType().type() + : ((MethodTree) enclosing).returnType().symbolType(); + checkArrayCovariance(lhsType, expression); + } + } + + private void visitYieldStatement(YieldStatementTree tree) { + Tree enclosing = ExpressionUtils.getEnclosingTree(tree, Tree.Kind.SWITCH_EXPRESSION, Tree.Kind.SWITCH_STATEMENT); + if (enclosing == null || enclosing.is(Tree.Kind.SWITCH_STATEMENT)) { + return; + } + var lhsType = ((ExpressionTree) enclosing).symbolType(); + checkArrayCovariance(lhsType, tree.expression()); + } + + private void visitInvocation(Symbol.MethodSymbol methodSymbol, Arguments arguments) { + List parameterTypes = methodSymbol.parameterTypes(); + var nonVarargCount = parameterTypes.size() - (methodSymbol.isVarArgsMethod() ? 1 : 0); + for (int i = 0; i < nonVarargCount && i < arguments.size(); i++) { + checkArrayCovariance(parameterTypes.get(i), arguments.get(i)); + } + if (!methodSymbol.isVarArgsMethod() || arguments.size() == nonVarargCount) { + return; + } + var varargType = (Type.ArrayType) parameterTypes.get(nonVarargCount); + checkArrayCovariance(varargType, arguments.get(nonVarargCount)); + var elementType = varargType.elementType(); + for (int i = nonVarargCount; i < arguments.size(); i++) { + checkArrayCovariance(elementType, arguments.get(i)); + } + } + + private void checkArrayCovariance(Type lhsType, ExpressionTree rhsExpression) { + var rhsType = rhsExpression.symbolType(); + if (!lhsType.isArray() || !rhsType.isArray() || rhsType.isNullType() || rhsType.isUnknown() || lhsType.isUnknown()) { + return; + } + var lhsElementType = ((Type.ArrayType) lhsType).elementType(); + var rhsElementType = ((Type.ArrayType) rhsType).elementType(); + if (lhsElementType.isUnknown() || rhsElementType.isUnknown() || lhsElementType.isPrimitive() || rhsElementType.isPrimitive()) { + return; + } + if (rhsElementType.isSubtypeOf(lhsElementType) && !lhsElementType.isSubtypeOf(rhsElementType)) { + context.reportIssue(this, rhsExpression, MESSAGE); + } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/ArrayCovarianceCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/ArrayCovarianceCheckTest.java new file mode 100644 index 00000000000..b76af1a9f39 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/ArrayCovarianceCheckTest.java @@ -0,0 +1,33 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * 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.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class ArrayCovarianceCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/ArrayCovarianceCheckSample.java")) + .withCheck(new ArrayCovarianceCheck()) + .verifyIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2330.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2330.html new file mode 100644 index 00000000000..42c657d0092 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2330.html @@ -0,0 +1,51 @@ +

Why is this an issue?

+

Array covariance is a feature in Java where if type A is a subtype of type B, then A[] is also considered +a subtype of B[]. This means you can assign an array of a more specific type to a variable of a more general array type.

+

While this can be convenient for read-only operations, it creates a significant risk when writing to the array. The Java compiler allows these +assignments because they are type-safe from a compile-time perspective. However, the runtime type system still tracks the actual array type.

+

When you try to store an element into a covariant array, the JVM performs a runtime check to ensure the element is compatible with the array's +actual component type. If the types don't match, an ArrayStoreException is thrown.

+

Noncompliant code example

+
+abstract class Fruit { }
+class Apple extends Fruit { }
+class Orange extends Fruit { }
+
+class Program {
+  public static void main(String[] args) {
+    Fruit[] fruits = new Apple[1]; // Noncompliant
+    fillWithOranges(fruits);
+  }
+
+  static void fillWithOranges(Fruit[] fruits) {
+    for (int i = 0; i < fruits.length; i++) {
+      fruits[i] = new Orange(); // Will throw ArrayStoreException
+    }
+  }
+}
+
+

Compliant solution

+
+abstract class Fruit { }
+class Apple extends Fruit { }
+class Orange extends Fruit { }
+
+class Program {
+  public static void main(String[] args) {
+    Orange[] fruits = new Orange[1]; // Compliant
+    fillWithOranges(fruits);
+  }
+
+  static void fillWithOranges(Orange[] fruits) {
+    for (int i = 0; i < fruits.length; i++) {
+      fruits[i] = new Orange();
+    }
+  }
+}
+
+

Resources

+

Documentation

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2330.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2330.json new file mode 100644 index 00000000000..4881458d22f --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S2330.json @@ -0,0 +1,23 @@ +{ + "title": "Array covariance should not be used", + "type": "CODE_SMELL", + "code": { + "impacts": { + "RELIABILITY": "MEDIUM" + }, + "attribute": "LOGICAL" + }, + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "10min" + }, + "tags": [ + "pitfall" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-2330", + "sqKey": "S2330", + "scope": "All", + "quickfix": "unknown" +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S2330 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S2330 new file mode 100644 index 00000000000..e69de29bb2d