Skip to content

Commit 5dd102b

Browse files
margarita-nedzelska-sonarsourceleveretkaGodin
authored
SONARKT-101 Replace rule S1481 with binding context diagnostics
Co-authored-by: Margarita Nedzelska <[email protected]> Co-authored-by: Evgeny Mandrikov <[email protected]>
1 parent fead5c0 commit 5dd102b

File tree

9 files changed

+88
-200
lines changed

9 files changed

+88
-200
lines changed

‎its/plugin/src/test/java/org/sonarsource/slang/SonarLintTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,7 @@ public void test_kotlin() throws Exception {
115115

116116
assertThat(issues).extracting(Issue::getRuleKey, Issue::getStartLine, issue -> issue.getInputFile().getPath(), Issue::getSeverity).containsOnly(
117117
tuple("kotlin:S100", 1, inputFile.getPath(), "MINOR"),
118-
tuple("kotlin:S1145", 2, inputFile.getPath(), "MAJOR"),
119-
tuple("kotlin:S1481", 3, inputFile.getPath(), "MINOR"));
118+
tuple("kotlin:S1145", 2, inputFile.getPath(), "MAJOR"));
120119
}
121120

122121
private ClientInputFile prepareInputFile(String relativePath, String content, final boolean isTest, String language) throws IOException {

‎its/ruling/src/test/resources/expected/kotlin/corda/kotlin-S1481.json‎

Lines changed: 0 additions & 59 deletions
This file was deleted.

‎its/ruling/src/test/resources/expected/kotlin/kotlin/kotlin-S1481.json‎

Lines changed: 0 additions & 93 deletions
This file was deleted.

‎kotlin-checks-test-sources/src/main/kotlin/checks/UnusedLocalVariableCheckSample.kt‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class UnusedLocalVariableCheckSample {
1111
val hcek = 0 // Compliant, is not local
1212

1313
init {
14-
var i: Int // Compliant
14+
var i: Int // Noncompliant
1515
}
1616

1717
constructor() {
@@ -64,3 +64,15 @@ class HappyPath {
6464
print(i)
6565
}
6666
}
67+
68+
data class D(val p: Int)
69+
70+
fun duplicatedNameInInitializer(d: D) {
71+
val p = d.p // Noncompliant {{Remove this unused "p" local variable.}}
72+
// ^
73+
}
74+
75+
fun destructuringDeclaration(d: D) {
76+
val (p) = d // Noncompliant {{Remove this unused "p" local variable.}}
77+
// ^
78+
}

‎kotlin-checks-test-sources/src/main/kotlin/checks/UnusedLocalVariableCheckSampleNoSemantics.kt‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package checks
2+
3+
// No issues without semantics
4+
class UnusedLocalVariableCheckSampleNoSemantics {
5+
6+
val f = { i: Int ->
7+
if (i == 0) {
8+
val toExpr = " "
9+
}
10+
}
11+
12+
val hcek = 0
13+
14+
init {
15+
var i: Int
16+
}
17+
18+
constructor() {
19+
val i = 90
20+
}
21+
var global = 0
22+
23+
fun fooBar() {
24+
val a = 0
25+
26+
val b: String
27+
28+
var c: String
29+
30+
var d: Int
31+
d = 0
32+
33+
var e: Int
34+
e = d + a
35+
36+
37+
val f = { i: Int ->
38+
if (i == 0) {
39+
val toExpr = " "
40+
}
41+
}
42+
43+
f(0)
44+
45+
fun fff() {
46+
var b = 0
47+
}
48+
}
49+
50+
fun String.extension() {
51+
val unused = 0
52+
}
53+
}
54+

‎sonar-kotlin-plugin/src/main/java/org/sonarsource/kotlin/checks/UnusedLocalVariableCheck.kt‎

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -19,46 +19,22 @@
1919
*/
2020
package org.sonarsource.kotlin.checks
2121

22-
import org.jetbrains.kotlin.psi.KtElement
23-
import org.jetbrains.kotlin.psi.KtFunction
24-
import org.jetbrains.kotlin.psi.KtLambdaExpression
25-
import org.jetbrains.kotlin.psi.KtNameReferenceExpression
26-
import org.jetbrains.kotlin.psi.KtNamedFunction
27-
import org.jetbrains.kotlin.psi.KtProperty
28-
import org.jetbrains.kotlin.psi.KtSecondaryConstructor
29-
import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
30-
import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType
31-
import org.jetbrains.kotlin.util.containingNonLocalDeclaration
22+
import org.jetbrains.kotlin.diagnostics.Errors
23+
import org.jetbrains.kotlin.psi.KtFile
24+
import org.jetbrains.kotlin.psi.KtNamedDeclaration
3225
import org.sonar.check.Rule
3326
import org.sonarsource.kotlin.api.AbstractCheck
3427
import org.sonarsource.kotlin.plugin.KotlinFileContext
3528

3629
@Rule(key = "S1481")
3730
class UnusedLocalVariableCheck : AbstractCheck() {
3831

39-
override fun visitNamedFunction(function: KtNamedFunction, context: KotlinFileContext) {
40-
if (function.isLocal) return
41-
function.checkUnusedVariables(context)
42-
}
43-
44-
override fun visitSecondaryConstructor(constructor: KtSecondaryConstructor, context: KotlinFileContext) {
45-
constructor.checkUnusedVariables(context)
46-
}
47-
48-
override fun visitLambdaExpression(expression: KtLambdaExpression, context: KotlinFileContext) {
49-
if (expression.containingNonLocalDeclaration() !is KtFunction) {
50-
expression.checkUnusedVariables(context)
51-
}
52-
}
53-
54-
private fun KtElement.checkUnusedVariables(context: KotlinFileContext) {
55-
forEachDescendantOfType<KtProperty> { property ->
56-
val nameIdentifier = property.nameIdentifier!!
57-
if (property.isLocal && !anyDescendantOfType<KtNameReferenceExpression> {
58-
reference -> reference.getReferencedName() == property.name
59-
}) {
60-
context.reportIssue(nameIdentifier, """Remove this unused "${nameIdentifier.text}" local variable.""")
32+
override fun visitKtFile(file: KtFile, context: KotlinFileContext) {
33+
context.bindingContext.diagnostics.noSuppression()
34+
.filter { it.factory == Errors.UNUSED_VARIABLE }
35+
.map { it.psiElement as KtNamedDeclaration }
36+
.forEach {
37+
context.reportIssue(it.nameIdentifier!!, """Remove this unused "${it.name}" local variable.""")
6138
}
62-
}
6339
}
6440
}

‎sonar-kotlin-plugin/src/test/java/org/sonarsource/kotlin/checks/UnusedLocalVariableCheckTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,4 @@
1919
*/
2020
package org.sonarsource.kotlin.checks
2121

22-
class UnusedLocalVariableCheckTest : CheckTest(UnusedLocalVariableCheck())
22+
class UnusedLocalVariableCheckTest : CheckTestWithNoSemantics(UnusedLocalVariableCheck())

‎sonar-kotlin-plugin/src/test/java/org/sonarsource/kotlin/plugin/IssueSuppressionVisitorTest.kt‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,15 @@ import org.sonarsource.kotlin.verifier.TestContext
1717
import java.nio.charset.StandardCharsets
1818
import java.nio.file.Files
1919
import java.nio.file.Path
20+
import org.sonarsource.kotlin.verifier.DEFAULT_KOTLIN_CLASSPATH
21+
import org.sonarsource.kotlin.verifier.KOTLIN_BASE_DIR
2022

2123
class IssueSuppressionVisitorTest {
2224
@Test
2325
fun `verify we actually suppress issues on various AST nodes`() {
24-
val withSuppressionTestFile = KotlinVerifier.KOTLIN_BASE_DIR.resolve("../sample/IssueSuppressionSample.kt")
25-
val forNoSuppressionTestFile = KotlinVerifier.KOTLIN_BASE_DIR.resolve("../sample/IssueNonSuppressionSample.kt")
26-
val withoutSuppressionTestFile = KotlinVerifier.KOTLIN_BASE_DIR.resolve("../sample/IssueWithoutSuppressionSample.kt")
26+
val withSuppressionTestFile = KOTLIN_BASE_DIR.resolve("../sample/IssueSuppressionSample.kt")
27+
val forNoSuppressionTestFile = KOTLIN_BASE_DIR.resolve("../sample/IssueNonSuppressionSample.kt")
28+
val withoutSuppressionTestFile = KOTLIN_BASE_DIR.resolve("../sample/IssueWithoutSuppressionSample.kt")
2729
scanWithSuppression(withSuppressionTestFile).assertOneOrMoreIssues()
2830
scanWithSuppression(withoutSuppressionTestFile).assertOneOrMoreIssues()
2931
scanWithoutSuppression(forNoSuppressionTestFile).assertOneOrMoreIssues()
@@ -36,7 +38,7 @@ class IssueSuppressionVisitorTest {
3638
scanFile(path, false, BadClassNameCheck(), BadFunctionNameCheck(), VariableAndParameterNameCheck(), UnusedLocalVariableCheck())
3739

3840
private fun scanFile(path: Path, suppress: Boolean, check: AbstractCheck, vararg checks: AbstractCheck): SingleFileVerifier {
39-
val env = Environment(emptyList())
41+
val env = Environment(DEFAULT_KOTLIN_CLASSPATH)
4042
val verifier = SingleFileVerifier.create(path, StandardCharsets.UTF_8)
4143
val testFileContent = String(Files.readAllBytes(path), StandardCharsets.UTF_8)
4244
val inputFile = TestInputFileBuilder("moduleKey", "src/org/foo/kotlin")

‎sonar-kotlin-plugin/src/test/java/org/sonarsource/kotlin/verifier/KotlinVerifier.kt‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,14 @@ import java.nio.file.Path
3939
import java.nio.file.Paths
4040
import java.nio.file.SimpleFileVisitor
4141
import java.nio.file.attribute.BasicFileAttributes
42+
val KOTLIN_BASE_DIR = Paths.get("..", "kotlin-checks-test-sources", "src", "main", "kotlin", "checks")
43+
val DEFAULT_KOTLIN_CLASSPATH = listOf("../kotlin-checks-test-sources/build/classes/kotlin/main", "../kotlin-checks-test-sources/build/classes/java/main")
44+
private val DEFAULT_TEST_JARS_DIRECTORY = "../kotlin-checks-test-sources/build/test-jars"
4245

4346
class KotlinVerifier(private val check: AbstractCheck) {
4447

45-
companion object {
46-
val KOTLIN_BASE_DIR = Paths.get("..", "kotlin-checks-test-sources", "src", "main", "kotlin", "checks")
47-
private val KOTLIN_CLASSPATH = listOf("../kotlin-checks-test-sources/build/classes/kotlin/main", "../kotlin-checks-test-sources/build/classes/java/main")
48-
private val DEFAULT_TEST_JARS_DIRECTORY = "../kotlin-checks-test-sources/build/test-jars"
49-
}
50-
5148
var fileName: String = ""
52-
var classpath: List<String> = System.getProperty("java.class.path").split(":") + KOTLIN_CLASSPATH
49+
var classpath: List<String> = System.getProperty("java.class.path").split(":") + DEFAULT_KOTLIN_CLASSPATH
5350
var deps: List<String> = getClassPath(DEFAULT_TEST_JARS_DIRECTORY)
5451
var isAndroid = false
5552

0 commit comments

Comments
 (0)