-
Notifications
You must be signed in to change notification settings - Fork 30
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Create rule S6524: Collection should be immutable if contents is not …
…changed (#1637)
- Loading branch information
1 parent
30d8955
commit af8db31
Showing
3 changed files
with
125 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
{ | ||
"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", | ||
"defaultQualityProfiles": ["Sonar way"], | ||
"quickfix": "unknown" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
== 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 | ||
|
||
[source,kotlin,diff-id=1,diff-type=noncompliant] | ||
---- | ||
fun sum123(): Int { | ||
val list = mutableListOf(1,2,3) // Noncompliant, can be immutable | ||
return list.reduce { acc, it -> acc + it} | ||
} | ||
---- | ||
|
||
==== Compliant solution | ||
|
||
[source,kotlin,diff-id=1,diff-type=compliant] | ||
---- | ||
fun sum123(): Int { | ||
val list = listOf(1,2,3) // Compliant | ||
return list.reduce { acc, it -> acc + it} | ||
} | ||
---- | ||
|
||
==== Noncompliant code example | ||
|
||
[source,kotlin,diff-id=2,diff-type=noncompliant] | ||
---- | ||
fun sumList(list: MutableList<Int>): Int { // Noncompliant, can be immutable | ||
return list.reduce { acc, it -> acc + it} | ||
} | ||
---- | ||
|
||
==== Compliant solution | ||
|
||
[source,kotlin,diff-id=2,diff-type=compliant] | ||
---- | ||
fun sumList(list: List<Int>): Int { // Compliant | ||
return list.reduce { acc, it -> acc + it} | ||
} | ||
---- | ||
|
||
==== Noncompliant code example | ||
|
||
[source,kotlin,diff-id=3,diff-type=noncompliant] | ||
---- | ||
fun MutableList<Int>.sum(): Int { // Noncompliant, can be immutable | ||
return reduce { acc, it -> acc + it} | ||
} | ||
---- | ||
|
||
==== Compliant solution | ||
|
||
[source,kotlin,diff-id=3,diff-type=compliant] | ||
---- | ||
fun List<Int>.sum(): Int { // Compliant | ||
return reduce { acc, it -> acc + it} | ||
} | ||
---- | ||
|
||
== Resources | ||
|
||
=== Articles & blog posts | ||
|
||
* https://www.baeldung.com/kotlin/immutable-collections[Baeldung, Kotlin Immutable Collections] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
{ | ||
} |