Skip to content

Commit ed935c5

Browse files
authored
Fix false-positive of ComentsRule on default copyright text (#740)
### What's done: * Fixed bug, added comment and test * Changed isCopyrightMandatory to true in default config
1 parent 3293e8d commit ed935c5

File tree

5 files changed

+82
-18
lines changed

5 files changed

+82
-18
lines changed

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ typealias EmitType = ((offset: Int, errorMessage: String, canBeAutoCorrected: Bo
1010

1111
typealias ListOfList = MutableList<MutableList<ASTNode>>
1212

13+
typealias ListOfPairs = MutableList<Pair<ASTNode, String>>
14+
1315
/**
1416
* This class represent individual inspections of diktat code style.
1517
* A [Warnings] entry contains rule name, warning message and is used in code check.

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter2/comments/CommentsRule.kt

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package org.cqfn.diktat.ruleset.rules.chapter2.comments
22

33
import org.cqfn.diktat.common.config.rules.RulesConfig
44
import org.cqfn.diktat.ruleset.constants.EmitType
5+
import org.cqfn.diktat.ruleset.constants.ListOfPairs
56
import org.cqfn.diktat.ruleset.constants.Warnings.COMMENTED_OUT_CODE
67
import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType
78

@@ -52,26 +53,35 @@ class CommentsRule(private val configRules: List<RulesConfig>) : Rule("comments"
5253
* with '// ' with whitespace, while automatic commenting in, e.g., IDEA creates slashes in the beginning of the line
5354
*
5455
*/
56+
@Suppress("UnsafeCallOnNullableType", "TOO_LONG_FUNCTION")
5557
private fun checkCommentedCode(node: ASTNode) {
56-
val eolCommentsOffsetToText = getOffsetsToTextBlocksFromEolComments(node)
58+
val errorNodesWithText: ListOfPairs = mutableListOf()
59+
val eolCommentsOffsetToText = getOffsetsToTextBlocksFromEolComments(node, errorNodesWithText)
5760
val blockCommentsOffsetToText = node
5861
.findAllNodesWithSpecificType(BLOCK_COMMENT)
59-
.map { it.startOffset to it.text.trim().removeSurrounding("/*", "*/") }
60-
62+
.map {
63+
errorNodesWithText.add(it to it.text.trim().removeSurrounding("/*", "*/"))
64+
it.startOffset to it.text.trim().removeSurrounding("/*", "*/")
65+
}
6166
(eolCommentsOffsetToText + blockCommentsOffsetToText)
6267
.flatMap { (offset, text) ->
6368
val (singleLines, blockLines) = text.lines().partition { it.contains(importOrPackage) }
6469
val block = if (blockLines.isNotEmpty()) listOf(blockLines.joinToString("\n")) else emptyList()
65-
(singleLines + block).map { offset to it }
70+
(singleLines + block).map {
71+
offset to it
72+
}
6673
}
67-
.map { (offset, text) ->
74+
.mapNotNull { (offset, text) ->
6875
when {
6976
text.contains(importKeyword) ->
7077
offset to ktPsiFactory.createImportDirective(ImportPath.fromString(text.substringAfter("$importKeyword "))).node
7178
text.contains(packageKeyword) ->
7279
offset to ktPsiFactory.createPackageDirective(FqName(text.substringAfter("$packageKeyword "))).node
73-
else ->
80+
else -> if (text.contains(requirePartOfCode)) {
7481
offset to ktPsiFactory.createBlockCodeFragment(text, null).node
82+
} else {
83+
null
84+
}
7585
}
7686
}
7787
.filter { (_, parsedNode) ->
@@ -80,7 +90,8 @@ class CommentsRule(private val configRules: List<RulesConfig>) : Rule("comments"
8090
.isEmpty()
8191
}
8292
.forEach { (offset, parsedNode) ->
83-
COMMENTED_OUT_CODE.warn(configRules, emitWarn, isFixMode, parsedNode.text.substringBefore("\n").trim(), offset, parsedNode)
93+
COMMENTED_OUT_CODE.warn(configRules, emitWarn, isFixMode, parsedNode.text.substringBefore("\n").trim(), offset,
94+
errorNodesWithText.find { it.second == parsedNode.text }?.first ?: parsedNode)
8495
}
8596
}
8697

@@ -90,7 +101,7 @@ class CommentsRule(private val configRules: List<RulesConfig>) : Rule("comments"
90101
* Splitting back into lines, if necessary, will be done outside of this method, for both text from EOL and block.
91102
* fixme: in this case offset is lost for lines which will be split once more
92103
*/
93-
private fun getOffsetsToTextBlocksFromEolComments(node: ASTNode): List<Pair<Int, String>> {
104+
private fun getOffsetsToTextBlocksFromEolComments(node: ASTNode, errorNodesWithText: ListOfPairs): List<Pair<Int, String>> {
94105
val comments = node
95106
.findAllNodesWithSpecificType(EOL_COMMENT)
96107
.filter { !it.text.contains(eolCommentStart) || isCodeAfterCommentStart(it.text) }
@@ -109,6 +120,7 @@ class CommentsRule(private val configRules: List<RulesConfig>) : Rule("comments"
109120
acc
110121
}
111122
.map { list ->
123+
list.forEach { errorNodesWithText.add(it to it.text.removePrefix("//")) }
112124
list.first().startOffset to list.joinToString("\n") { it.text.removePrefix("//") }
113125
}
114126
} else {
@@ -139,6 +151,7 @@ class CommentsRule(private val configRules: List<RulesConfig>) : Rule("comments"
139151
private val importOrPackageRegex = """^(import|package)?\s+([a-zA-Z.])+;*$""".toRegex()
140152
private val functionRegex = """^(public|private|protected)*\s*(override|abstract|actual|expect)*\s?fun\s+\w+(\(.*\))?(\s*:\s*\w+)?\s*[{=]${'$'}""".toRegex()
141153
private val rightBraceRegex = """^\s*}$""".toRegex()
154+
private val requirePartOfCode = """val |var |=|(\{((.|\n)*)\})""".toRegex()
142155
private val codeFileStartCases = listOf(classRegex, importOrPackageRegex, functionRegex, rightBraceRegex)
143156
private val eolCommentStart = """// \S""".toRegex()
144157
}

diktat-rules/src/main/resources/diktat-analysis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@
131131
- name: HEADER_MISSING_OR_WRONG_COPYRIGHT
132132
enabled: true
133133
configuration:
134-
isCopyrightMandatory: true
134+
isCopyrightMandatory: false
135135
copyrightText: 'Copyright (c) Your Company Name Here. 2010-2021'
136136
# Checks that header kdoc is located before package directive
137137
- name: HEADER_NOT_BEFORE_PACKAGE

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter2/comments/CommentedCodeTest.kt

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ class CommentedCodeTest : LintTestBase(::CommentsRule) {
5252
|fun foo(a: Int): Int {
5353
| /* println(a + 42)
5454
| println("This is a test string")
55+
| val b = a*10
5556
| */
5657
| return 0
5758
|}
@@ -72,9 +73,7 @@ class CommentedCodeTest : LintTestBase(::CommentsRule) {
7273
|// println("This is a test string")
7374
| return 0
7475
|}
75-
""".trimMargin(),
76-
LintError(4, 1, ruleId, "${COMMENTED_OUT_CODE.warnText()} println(a + 42)", false)
77-
)
76+
""".trimMargin())
7877
}
7978

8079
@Test
@@ -179,8 +178,7 @@ class CommentedCodeTest : LintTestBase(::CommentsRule) {
179178
lintMethod(
180179
"""
181180
|// class Test: Exception()
182-
""".trimMargin(),
183-
LintError(1, 1, ruleId, "${COMMENTED_OUT_CODE.warnText()} class Test: Exception()", false))
181+
""".trimMargin())
184182
}
185183

186184
@Test
@@ -199,8 +197,7 @@ class CommentedCodeTest : LintTestBase(::CommentsRule) {
199197
lintMethod(
200198
"""
201199
|// internal sealed class Test: Exception()
202-
""".trimMargin(),
203-
LintError(1, 1, ruleId, "${COMMENTED_OUT_CODE.warnText()} internal sealed class Test: Exception()", false))
200+
""".trimMargin())
204201
}
205202

206203
@Test
@@ -293,4 +290,57 @@ class CommentedCodeTest : LintTestBase(::CommentsRule) {
293290
| */
294291
""".trimMargin())
295292
}
293+
294+
@Test
295+
@Tag(WarningNames.COMMENTED_OUT_CODE)
296+
fun `should not trigger on Copyright and another comment`() {
297+
lintMethod(
298+
"""
299+
/*
300+
Copyright (c) Your Company Name Here. 2010-2021
301+
*/
302+
303+
package org.cqfn.diktat
304+
305+
/*
306+
x = 2 + 4 + 1
307+
*/
308+
// x = 2+4
309+
310+
// if true make this
311+
312+
/*
313+
class A {
314+
315+
fun foo()
316+
317+
}
318+
319+
*/
320+
""".trimMargin(),
321+
LintError(7, 13, ruleId, "${COMMENTED_OUT_CODE.warnText()} ", false),
322+
LintError(14, 13, ruleId, "${COMMENTED_OUT_CODE.warnText()} ", false)
323+
)
324+
}
325+
326+
@Test
327+
@Tag(WarningNames.COMMENTED_OUT_CODE)
328+
fun `should not trigger with suppress`() {
329+
lintMethod(
330+
"""
331+
@Suppress("UnsafeCallOnNullableType", "COMMENTED_OUT_CODE")
332+
private fun handleProperty(property: KtProperty) {
333+
334+
/*
335+
x = 1
336+
*/
337+
}
338+
339+
@Suppress("COMMENTED_OUT_CODE")
340+
class A {
341+
// val x = 10
342+
}
343+
""".trimMargin()
344+
)
345+
}
296346
}

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,7 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin",
160160
fun `smoke test #2`() {
161161
fixAndCompare("Example2Expected.kt", "Example2Test.kt")
162162
unfixedLintErrors.assertEquals(
163-
LintError(1, 1, "$DIKTAT_RULE_SET_ID:header-comment", "${HEADER_MISSING_IN_NON_SINGLE_CLASS_FILE.warnText()} there are 2 declared classes and/or objects", false),
164-
LintError(12, 26, "$DIKTAT_RULE_SET_ID:comments", "${Warnings.COMMENTED_OUT_CODE.warnText()} private class Test : RuntimeException()", false)
163+
LintError(1, 1, "$DIKTAT_RULE_SET_ID:header-comment", "${HEADER_MISSING_IN_NON_SINGLE_CLASS_FILE.warnText()} there are 2 declared classes and/or objects", false)
165164
)
166165
}
167166

0 commit comments

Comments
 (0)