diff --git a/kotlin-checks-test-sources/src/main/kotlin/checks/CollectionShouldBeImmutableCheckSample.kt b/kotlin-checks-test-sources/src/main/kotlin/checks/CollectionShouldBeImmutableCheckSample.kt new file mode 100644 index 000000000..c6a460ca3 --- /dev/null +++ b/kotlin-checks-test-sources/src/main/kotlin/checks/CollectionShouldBeImmutableCheckSample.kt @@ -0,0 +1,228 @@ +package checks + +class CollectionShouldBeImmutableCheckSample { + fun MutableCollection.doSomething(): Unit {} // Noncompliant + + //let also apply run with + fun nonCompliant( + x: MutableList, // Noncompliant {{Make this collection immutable.}} + y: MutableSet, // Noncompliant +// ^^^^^^^^^^^^^^^^^^^^^ + z: MutableMap, // Noncompliant + a: MutableList, // compliant, FN + b: MutableList, // Noncompliant + c: MutableList, // compliant, FN + ): Unit { + baz(x.toList().toMutableList()) + baz(doNothing(x).toMutableList()) + doNothing(x) + x.reduce { acc, it -> acc + it } + x + 1 + x.size + for (i in x) { + println(i) + } + x.foo() + id(x + listOf(1, 2, 3)) + y.contains("a") + y.map { it + "a" } + z[1] + doNothing(x, y, z) + a.let { it.size } + a.let { it.also { with(it, { this.size }) } } + Pair(1, b.reduce { acc, i -> i + acc }) + id(c) + } + + fun List.toList(): List = this // compliant + fun baz(list : MutableList): Unit {} // Noncompliant + + fun List.foo(): Unit {} // Compliant + fun doNothing(a : List, b : Set, c : Map): Unit {} + fun doNothing(a : List): List { return a } // Compliant + + fun id(a : A): A = a // Compliant + + fun compliantScopedFunctions( + a: MutableList, // Compliant + b: MutableList, // Compliant + c: MutableList, // Compliant + d: MutableList, // Compliant + e: MutableList, // Compliant + f: MutableList, // Compliant + crazy: MutableList, + ): Unit { // Compliant + a.let { it.add(1) } + b.also { it.add(1) } + c.apply { add(1) } + d.run { add(1) } + with(e) { add(1) } + f.let { it.let { it.let { it.add(1) } } } + crazy.let { it.also { it.apply { it.run { with(this) { it.add(1) } } } } } + } + + fun MutableList.doSomething2(): MutableList { return this } // Noncompliant + + fun compliantFunctionsCalledOn( + a: MutableList, // Compliant + b: MutableList, // Compliant + c: List, // Compliant + d: MutableList, // Compliant + e: MutableMap, // Compliant + f: MutableList, // Compliant + g: MutableList, // Compliant + h: MutableSet, // Compliant + i: MutableList, // Compliant + j: MutableList, // Compliant + k: MutableList, // Compliant + l: MutableList, // Compliant + m: MutableList, // Compliant + n: MutableList, // Compliant + ): Unit { + a.add(1) + b.iterator() + c.map { it + 1} + d.add(1) + e.remove(1) + f.doSomething() + g.doSomething2() + h.doSomething() + ((i)).doSomething() + j.doSomething2().doSomething2().doSomething2() + when (k) { + is MutableList -> k.doSomething() + } + l!!.doSomething() + m?.doSomething() + if(true){n}else{n}.doSomething() + } + + + fun compliantBinaryExprCalledOn( + c: List, // Compliant + d: MutableList, // Compliant + e: MutableList, // Compliant + f: MutableList, // Compliant + g: MutableList, // Compliant + ): Unit { + c[0] + d += 1 + e -= 1 + f[0] = 1 + (g)[0] = 1 + } + + + + fun compliantCalledInFun( + a: MutableList, // Compliant + b: MutableList, // Compliant + c: MutableList, // Compliant + d: MutableList, // Compliant + e: MutableList, // Compliant + f: MutableList // Compliant + ): Unit { + foo_(a) + foo2(b) + foo_(((c))) + foo_(d!!) + foo_(if(true){e}else{e}) + Pair(f, 1) + } + + fun foo_(x : MutableList): Unit {} // Noncompliant {{Make this collection immutable.}} + fun foo2(x : MutableCollection): Unit {} // Noncompliant + + + fun nonCompliantParameter(list: MutableList): Int { // Noncompliant + return list.reduce { acc, it -> acc + it} + } + + fun compliantParameter(list: List): Int { // Compliant + return list.reduce { acc, it -> acc + it} + } + + fun MutableList.compliantDelegate(): Int { // Compliant + this.add(1) + return reduce { acc, it -> acc + it } + } + + + fun MutableList.compliantDelegate2(): Unit { // Compliant + foo_(this) + } + + fun MutableList.compliantDelegate3(): Unit { // Compliant + foo_(this) + add(1) + } + + fun MutableList.compliantDelegate4(): Unit { // Compliant + if(add(1)) {} + } + + fun MutableList.noncompliantDelegate(): Int { // Noncompliant +// ^^^^^^^^^^^^^^^^ + return reduce { acc, it -> acc + it} + } + + fun MutableList.noncompliantDelegate2(): Int { // compliant, FN + var list = mutableListOf(1, 2, 3) // we should change noReciever to check that it is called on this + list.add(1) + return reduce { acc, it -> acc + it} + } + + + + class AMutableCollections : MutableCollection { + override val size: Int + get() = TODO() + + override fun contains(element: Int): Boolean { + TODO() + } + + override fun containsAll(elements: Collection): Boolean { + TODO() + } + + override fun isEmpty(): Boolean { + TODO() + } + + override fun add(element: Int): Boolean { + TODO() + } + + override fun addAll(elements: Collection): Boolean { + TODO() + } + + override fun clear() { + TODO() + } + + override fun iterator(): MutableIterator { + TODO() + } + + override fun remove(element: Int): Boolean { + TODO() + } + + override fun removeAll(elements: Collection): Boolean { + TODO() + } + + override fun retainAll(elements: Collection): Boolean { + TODO() + } + } + + fun id(x : AMutableCollections): AMutableCollections = x // compliant, for now don't know how to check that is subtype of MutableCollection + + + fun foo(configure: (MutableMap) -> Unit): Unit { // compliant + } + +} \ No newline at end of file diff --git a/kotlin-checks-test-sources/src/main/kotlin/checks/CollectionShouldBeImmutableCheckSampleNoSemantics.kt b/kotlin-checks-test-sources/src/main/kotlin/checks/CollectionShouldBeImmutableCheckSampleNoSemantics.kt new file mode 100644 index 000000000..a7202c085 --- /dev/null +++ b/kotlin-checks-test-sources/src/main/kotlin/checks/CollectionShouldBeImmutableCheckSampleNoSemantics.kt @@ -0,0 +1,215 @@ +package checks + +class CollectionShouldBeImmutableCheckSampleNoSemantics { + fun MutableCollection.doSomething(): Unit {} + + //let also apply run with + fun nonCompliant( + x: MutableList, + y: MutableSet, + z: MutableMap, + a: MutableList, + b: MutableList, + c: MutableList, + ): Unit { + baz(x.toList().toMutableList()) + baz(doNothing(x).toMutableList()) + doNothing(x) + x.reduce { acc, it -> acc + it } + x + 1 + x.size + for (i in x) { + println(i) + } + x.foo() + id(x + listOf(1, 2, 3)) + y.contains("a") + y.map { it + "a" } + z[1] + doNothing(x, y, z) + a.let { it.size } + a.let { it.also { with(it, { this.size }) } } + Pair(1, b.reduce { acc, i -> i + acc }) + id(c) + } + + fun List.toList(): List = this // compliant + fun baz(list : MutableList): Unit {} + + fun List.foo(): Unit {} // Compliant + fun doNothing(a : List, b : Set, c : Map): Unit {} + fun doNothing(a : List): List { return a } // Compliant + + fun id(a : A): A = a // Compliant + + fun compliantScopedFunctions( + a: MutableList, // Compliant + b: MutableList, // Compliant + c: MutableList, // Compliant + d: MutableList, // Compliant + e: MutableList, // Compliant + f: MutableList, // Compliant + crazy: MutableList, + ): Unit { // Compliant + a.let { it.add(1) } + b.also { it.add(1) } + c.apply { add(1) } + d.run { add(1) } + with(e) { add(1) } + f.let { it.let { it.let { it.add(1) } } } + crazy.let { it.also { it.apply { it.run { with(this) { it.add(1) } } } } } + } + + fun MutableList.doSomething2(): MutableList { return this } + + fun compliantFunctionsCalledOn( + a: MutableList, // Compliant + b: MutableList, // Compliant + c: List, // Compliant + d: MutableList, // Compliant + e: MutableMap, // Compliant + f: MutableList, // Compliant + g: MutableList, // Compliant + h: MutableSet, // Compliant + i: MutableList, // Compliant + j: MutableList, // Compliant + k: MutableList, // Compliant + l: MutableList, // Compliant + m: MutableList, // Compliant + n: MutableList, // Compliant + ): Unit { + a.add(1) + b.iterator() + c.map { it + 1} + d.add(1) + e.remove(1) + f.doSomething() + g.doSomething2() + h.doSomething() + ((i)).doSomething() + j.doSomething2().doSomething2().doSomething2() + when (k) { + is MutableList -> k.doSomething() + } + l!!.doSomething() + m?.doSomething() + if(true){n}else{n}.doSomething() + } + + + fun compliantBinaryExprCalledOn( + c: List, // Compliant + d: MutableList, // Compliant + e: MutableList, // Compliant + f: MutableList, // Compliant + g: MutableList, // Compliant + ): Unit { + c[0] + d += 1 + e -= 1 + f[0] = 1 + (g)[0] = 1 + } + + + + fun compliantCalledInFun( + a: MutableList, // Compliant + b: MutableList, // Compliant + c: MutableList, // Compliant + d: MutableList, // Compliant + e: MutableList, // Compliant + f: MutableList // Compliant + ): Unit { + foo_(a) + foo2(b) + foo_(((c))) + foo_(d!!) + foo_(if(true){e}else{e}) + Pair(f, 1) + } + + fun foo_(x : MutableList): Unit {} + fun foo2(x : MutableCollection): Unit {} + + + fun nonCompliantParameter(list: MutableList): Int { + return list.reduce { acc, it -> acc + it} + } + + fun compliantParameter(list: List): Int { // Compliant + return list.reduce { acc, it -> acc + it} + } + + fun MutableList.compliantDelegate(): Int { // Compliant + this.add(1) + return reduce { acc, it -> acc + it } + } + + + fun MutableList.compliantDelegate2(): Unit { // Compliant + foo_(this) + } + + fun MutableList.compliantDelegate3(): Unit { // Compliant + foo_(this) + add(1) + } + + fun MutableList.noncompliantDelegate(): Int { + return reduce { acc, it -> acc + it} + } + + + class AMutableCollections : MutableCollection { + override val size: Int + get() = TODO() + + override fun contains(element: Int): Boolean { + TODO() + } + + override fun containsAll(elements: Collection): Boolean { + TODO() + } + + override fun isEmpty(): Boolean { + TODO() + } + + override fun add(element: Int): Boolean { + TODO() + } + + override fun addAll(elements: Collection): Boolean { + TODO() + } + + override fun clear() { + TODO() + } + + override fun iterator(): MutableIterator { + TODO() + } + + override fun remove(element: Int): Boolean { + TODO() + } + + override fun removeAll(elements: Collection): Boolean { + TODO() + } + + override fun retainAll(elements: Collection): Boolean { + TODO() + } + } + + fun id(x : AMutableCollections): AMutableCollections = x // compliant, for now don't know how to check that is subtype of MutableCollection + + + fun foo(configure: (MutableMap) -> Unit): Unit { // compliant + } + +} \ No newline at end of file diff --git a/sonar-kotlin-api/src/main/java/org/sonarsource/kotlin/api/checks/ApiExtensions.kt b/sonar-kotlin-api/src/main/java/org/sonarsource/kotlin/api/checks/ApiExtensions.kt index 24251072c..c1e15f002 100644 --- a/sonar-kotlin-api/src/main/java/org/sonarsource/kotlin/api/checks/ApiExtensions.kt +++ b/sonar-kotlin-api/src/main/java/org/sonarsource/kotlin/api/checks/ApiExtensions.kt @@ -328,6 +328,9 @@ private fun KtProperty.determineType(bindingContext: BindingContext) = fun KtProperty.determineTypeAsString(bindingContext: BindingContext, printTypeArguments: Boolean = false) = determineType(bindingContext)?.getKotlinTypeFqName(printTypeArguments) +fun KtExpression.determineTypeAsString(bindingContext: BindingContext, printTypeArguments: Boolean = false) = + determineType(bindingContext)?.getKotlinTypeFqName(printTypeArguments) + private fun KtParameter.determineType(bindingContext: BindingContext) = bindingContext[BindingContext.TYPE, typeReference] diff --git a/sonar-kotlin-api/src/test/java/org/sonarsource/kotlin/api/checks/ApiExtensionsKtTest.kt b/sonar-kotlin-api/src/test/java/org/sonarsource/kotlin/api/checks/ApiExtensionsKtTest.kt index 7b67ffd92..02f494021 100644 --- a/sonar-kotlin-api/src/test/java/org/sonarsource/kotlin/api/checks/ApiExtensionsKtTest.kt +++ b/sonar-kotlin-api/src/test/java/org/sonarsource/kotlin/api/checks/ApiExtensionsKtTest.kt @@ -424,6 +424,10 @@ class ApiExtensionsKtDetermineTypeTest { val expr = ktFile.findDescendantOfType { it.text == "{ 1 }" }!! assertThat(expr.determineType(bindingContext)!!.getKotlinTypeFqName(false)) .isEqualTo("kotlin.Int") + assertThat(expr.determineTypeAsString(bindingContext)) + .isEqualTo("kotlin.Int") + val missingContext = BindingContext.EMPTY + assertThat(expr.determineTypeAsString(missingContext)).isNull() } @Test diff --git a/sonar-kotlin-checks/src/main/java/org/sonarsource/kotlin/checks/CollectionShouldBeImmutableCheck.kt b/sonar-kotlin-checks/src/main/java/org/sonarsource/kotlin/checks/CollectionShouldBeImmutableCheck.kt new file mode 100644 index 000000000..e81184972 --- /dev/null +++ b/sonar-kotlin-checks/src/main/java/org/sonarsource/kotlin/checks/CollectionShouldBeImmutableCheck.kt @@ -0,0 +1,184 @@ +/* + * SonarSource Kotlin + * Copyright (C) 2018-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * 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 GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonarsource.kotlin.checks + +import org.jetbrains.kotlin.com.intellij.psi.PsiElement +import org.jetbrains.kotlin.descriptors.CallableDescriptor +import org.jetbrains.kotlin.js.descriptorUtils.getKotlinTypeFqName +import org.jetbrains.kotlin.psi.KtBinaryExpression +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtDotQualifiedExpression +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtExpression +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtNamedDeclaration +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtQualifiedExpression +import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.BindingContext.DECLARATION_TO_DESCRIPTOR +import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall +import org.jetbrains.kotlin.resolve.descriptorUtil.isExtension +import org.jetbrains.kotlin.types.KotlinType +import org.sonar.check.Rule +import org.sonarsource.kotlin.api.checks.AbstractCheck +import org.sonarsource.kotlin.api.checks.determineTypeAsString +import org.sonarsource.kotlin.api.frontend.KotlinFileContext + +@Rule(key = "S6524") +class CollectionShouldBeImmutableCheck : AbstractCheck() { + private val mutableCollections = + setOf( + "kotlin.collections.MutableList", + "kotlin.collections.MutableSet", + "kotlin.collections.MutableMap", + "kotlin.collections.MutableCollection" + ) + + + override fun visitNamedFunction(function: KtNamedFunction, context: KotlinFileContext) { + val bindingContext = context.bindingContext + + val referencesToMutableCollections = collectReferenceToMutatedCollections(function, bindingContext) + val mutatedNames = referencesToMutableCollections.map { it.getReferencedName() }.toSet() + val mutatedDeclarations = + referencesToMutableCollections.mapNotNull { it.getResolvedCall(bindingContext)?.resultingDescriptor?.original }.toSet() + + reportPropertiesAndParameters(function, mutatedDeclarations, bindingContext, context) + + if (function.receiverTypeReference?.determineTypeAsString(bindingContext) in mutableCollections) { + reportForExtensionFunction(function, bindingContext, mutatedNames, context) + } + } + + private fun reportPropertiesAndParameters( + function: KtNamedFunction, + mutatedDeclarations: Set, + bindingContext: BindingContext, + context: KotlinFileContext, + ) { + function.valueParameters + .filter { it.isMutableCollection(bindingContext) } + .filter { it.isNotReferenced(mutatedDeclarations, bindingContext) } + .forEach { context.reportIssue(it, "Make this collection immutable.") } + } + + private fun KtNamedDeclaration.isMutableCollection(bindingContext: BindingContext): Boolean = + this.determineTypeAsString(bindingContext) in mutableCollections + + private fun KtNamedDeclaration.isNotReferenced( + mutatedDeclarations: Set, + bindingContext: BindingContext, + ): Boolean { + val descriptor = bindingContext[DECLARATION_TO_DESCRIPTOR, this] + //descriptor is never null because we need the descriptor to know if we will consider the declaration + //no need to return false if descriptor is null + return !mutatedDeclarations.contains(descriptor) + } + + private fun reportForExtensionFunction( + function: KtNamedFunction, + bindingContext: BindingContext, + mutatedNames: Set, + context: KotlinFileContext, + ) { + val functionsCalledOnThis = function + .collectDescendantsOfType { it.noReceiver() } + + if (functionsCalledOnThis.none { it.mutatesCollection(bindingContext) } && "this" !in mutatedNames) { + context.reportIssue(function.receiverTypeReference!!.navigationElement, "Make this collection immutable.") + } + } + + private fun KtCallExpression.noReceiver(): Boolean { + return this.parent !is KtQualifiedExpression + } + + private fun collectReferenceToMutatedCollections( + function: KtNamedFunction, + bindingContext: BindingContext, + ): List { + val mutablePropertiesReceiver: List = function + .collectDescendantsOfType { it.mutatesCollection(bindingContext) } + .flatMap { findReferenceExpressions(it.receiverExpression) } + + val mutablePropertiesInBinaryExpression: List = function + .collectDescendantsOfType { it.mutatesCollection(bindingContext) } + .flatMap { findReferenceExpressions(it.left!!) } + + val mutablePropertiesInFunctionCalls: List = function + .collectDescendantsOfType() + .flatMap { call -> + val functionDescriptor = call.getResolvedCall(bindingContext)?.resultingDescriptor + val parameterTypes = functionDescriptor?.valueParameters + if (parameterTypes != null) { + val fullyQualifiedTypes = parameterTypes.map { it.type.getKotlinTypeFqName(false) } + call.valueArguments.zip(fullyQualifiedTypes).filter { it.second in mutableCollections }.map { it.first } + } else { + call.valueArguments + } + } + .flatMap { findReferenceExpressions(it.getArgumentExpression()!!) } + + return mutablePropertiesReceiver + mutablePropertiesInBinaryExpression + mutablePropertiesInFunctionCalls + } + + private fun findReferenceExpressions( + expression: KtExpression, + ): List { + return expression.collectDescendantsOfType(::considerElement) { true } + } + + private fun considerElement(element: PsiElement): Boolean { + return when (element) { + is KtCallExpression -> false + is KtDotQualifiedExpression -> false + else -> true + } + } + + private fun KtQualifiedExpression.mutatesCollection(bindingContext: BindingContext): Boolean { + return if (this.selectorExpression is KtCallExpression) { + val selectorExpression = this.selectorExpression as KtCallExpression + selectorExpression.mutatesCollection(bindingContext) + } else { + false + } + } + + private fun KtElement.mutatesCollection(bindingContext: BindingContext): Boolean { + val functionDescriptor = this.getResolvedCall(bindingContext)?.resultingDescriptor + val receiverType = functionDescriptor?.receiverType() + return if (receiverType?.constructor?.declarationDescriptor != null) { + receiverType.getKotlinTypeFqName(false) in mutableCollections + } else { + true + } + } + + private fun CallableDescriptor.receiverType(): KotlinType? { + return if (this.isExtension) { + this.extensionReceiverParameter?.type + } else { + this.dispatchReceiverParameter?.type + } + } + +} \ No newline at end of file diff --git a/sonar-kotlin-checks/src/test/java/org/sonarsource/kotlin/checks/CollectionShouldBeImmutableCheckTest.kt b/sonar-kotlin-checks/src/test/java/org/sonarsource/kotlin/checks/CollectionShouldBeImmutableCheckTest.kt new file mode 100644 index 000000000..6bb503eac --- /dev/null +++ b/sonar-kotlin-checks/src/test/java/org/sonarsource/kotlin/checks/CollectionShouldBeImmutableCheckTest.kt @@ -0,0 +1,22 @@ +/* + * SonarSource Kotlin + * Copyright (C) 2018-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * 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 GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonarsource.kotlin.checks + +internal class CollectionShouldBeImmutableCheckTest : CheckTestWithNoSemantics(CollectionShouldBeImmutableCheck()) \ No newline at end of file diff --git a/sonar-kotlin-plugin/src/main/java/org/sonarsource/kotlin/plugin/KotlinCheckList.kt b/sonar-kotlin-plugin/src/main/java/org/sonarsource/kotlin/plugin/KotlinCheckList.kt index fb2049261..271991d12 100644 --- a/sonar-kotlin-plugin/src/main/java/org/sonarsource/kotlin/plugin/KotlinCheckList.kt +++ b/sonar-kotlin-plugin/src/main/java/org/sonarsource/kotlin/plugin/KotlinCheckList.kt @@ -37,6 +37,7 @@ import org.sonarsource.kotlin.checks.CodeAfterJumpCheck import org.sonarsource.kotlin.checks.CollapsibleIfStatementsCheck import org.sonarsource.kotlin.checks.CollectionCallingItselfCheck import org.sonarsource.kotlin.checks.CollectionInappropriateCallsCheck +import org.sonarsource.kotlin.checks.CollectionShouldBeImmutableCheck import org.sonarsource.kotlin.checks.CollectionSizeAndArrayLengthCheck import org.sonarsource.kotlin.checks.CommentedCodeCheck import org.sonarsource.kotlin.checks.CoroutineScopeFunSuspendingCheck @@ -169,6 +170,7 @@ val KOTLIN_CHECKS = listOf( CodeAfterJumpCheck::class.java, CollapsibleIfStatementsCheck::class.java, CollectionCallingItselfCheck::class.java, + CollectionShouldBeImmutableCheck::class.java, CollectionSizeAndArrayLengthCheck::class.java, CollectionInappropriateCallsCheck::class.java, CoroutineScopeFunSuspendingCheck::class.java, diff --git a/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/S6524.html b/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/S6524.html new file mode 100644 index 000000000..d47a3734e --- /dev/null +++ b/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/S6524.html @@ -0,0 +1,65 @@ +

Why is this an issue?

+

If a mutable collection type is used but no mutating functions such as add or remove are ever called, and the collection +instance does not leave the scope of the function, it can be replaced with the corresponding immutable collection type.

+

This is similar to why val should be used instead of var for local variables that are never re-assigned.

+

What is the potential impact?

+

Readability and Understanding

+

If an immutable collection type is used, it is evident to the readers that its content is never changed. This makes it easier to understand the +code because readers do not need to keep track of possible state changes of the collection.

+

Performance

+

In some cases, optimized implementation variants of collection classes can be used when the collection is immutable.

+

Wrong code

+

Developers might intend for a collection to remain unchanged and have their code relying on that constraint. For example, a map could be expected +to contain specific elements. Changing the contents of a collection breaks that constraint. Also, users of an API might otherwise downcast an +immutable collection they got from a library into a mutable collection, and so cause unforeseen side effects.

+

Declare collections that remain unchanged as immutable to avoid these mistakes.

+

How to fix it

+

Replace mutable collection type names such as MutableList or MutableMap with their immutable equivalents, such as +List or map.

+

Replace builder functions that return mutable collection instances, such as mutableListOf with their immutable counterparts, such as +listOf.

+

Code examples

+

Noncompliant code example

+
+fun sum123(): Int {
+    val list = mutableListOf(1,2,3) // Noncompliant, can be immutable
+    return list.reduce { acc, it -> acc + it}
+}
+
+

Compliant solution

+
+fun sum123(): Int {
+    val list = listOf(1,2,3) // Compliant
+    return list.reduce { acc, it -> acc + it}
+}
+
+

Noncompliant code example

+
+fun sumList(list: MutableList<Int>): Int { // Noncompliant, can be immutable
+    return list.reduce { acc, it -> acc + it}
+}
+
+

Compliant solution

+
+fun sumList(list: List<Int>): Int { // Compliant
+    return list.reduce { acc, it -> acc + it}
+}
+
+

Noncompliant code example

+
+fun MutableList<Int>.sum(): Int { // Noncompliant, can be immutable
+    return reduce { acc, it -> acc + it}
+}
+
+

Compliant solution

+
+fun List<Int>.sum(): Int { // Compliant
+    return reduce { acc, it -> acc + it}
+}
+
+

Resources

+

Articles & blog posts

+
+ diff --git a/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/S6524.json b/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/S6524.json new file mode 100644 index 000000000..a8574b8b2 --- /dev/null +++ b/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/S6524.json @@ -0,0 +1,21 @@ +{ + "title": "Collection should be immutable if contents is not changed", + "type": "CODE_SMELL", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CLEAR" + }, + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-6524", + "sqKey": "S6524", + "scope": "All", + "quickfix": "unknown" +} diff --git a/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/Sonar_way_profile.json b/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/Sonar_way_profile.json index b7776631e..dbc6310ad 100644 --- a/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/Sonar_way_profile.json +++ b/sonar-kotlin-plugin/src/main/resources/org/sonar/l10n/kotlin/rules/kotlin/Sonar_way_profile.json @@ -106,6 +106,7 @@ "S6517", "S6518", "S6519", + "S6524", "S6526", "S6527", "S6528",