Skip to content

Commit

Permalink
Fix: CommentsRule should not treat presence of import keyword alone…
Browse files Browse the repository at this point in the history
… as code (#1096)

### What's done:
* Fix in logic
* Add tests

Closes #1091
  • Loading branch information
petertrr authored Nov 8, 2021
1 parent 2dc8e8b commit 631c10e
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import org.cqfn.diktat.ruleset.constants.ListOfPairs
import org.cqfn.diktat.ruleset.constants.Warnings.COMMENTED_OUT_CODE
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.findAllDescendantsWithSpecificType
import org.cqfn.diktat.ruleset.utils.getFilePath

import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT
import com.pinterest.ktlint.core.ast.ElementType.EOL_COMMENT
Expand All @@ -17,6 +18,7 @@ import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.KtPsiFactory
import org.jetbrains.kotlin.resolve.ImportPath
import org.slf4j.LoggerFactory

/**
* This rule performs checks if there is any commented code.
Expand Down Expand Up @@ -47,7 +49,11 @@ class CommentsRule(configRules: List<RulesConfig>) : DiktatRule(
* with '// ' with whitespace, while automatic commenting in, e.g., IDEA creates slashes in the beginning of the line
*
*/
@Suppress("UnsafeCallOnNullableType", "TOO_LONG_FUNCTION")
@Suppress(
"UnsafeCallOnNullableType",
"TOO_LONG_FUNCTION",
"AVOID_NULL_CHECKS"
)
private fun checkCommentedCode(node: ASTNode) {
val errorNodesWithText: ListOfPairs = mutableListOf()
val eolCommentsOffsetToText = getOffsetsToTextBlocksFromEolComments(node, errorNodesWithText)
Expand All @@ -68,10 +74,10 @@ class CommentsRule(configRules: List<RulesConfig>) : DiktatRule(
.map { (offset, text) -> offset to text.trim() }
.mapNotNull { (offset, text) ->
when {
text.contains(importKeyword) ->
offset to ktPsiFactory.createImportDirective(ImportPath.fromString(text.substringAfter("$importKeyword "))).node
text.contains(packageKeyword) ->
offset to ktPsiFactory.createPackageDirective(FqName(text.substringAfter("$packageKeyword "))).node
text.isPossibleImport() ->
offset to ktPsiFactory.createImportDirective(ImportPath.fromString(text.substringAfter(importKeywordWithSpace, ""))).node
text.trimStart().startsWith(packageKeywordWithSpace) ->
offset to ktPsiFactory.createPackageDirective(FqName(text.substringAfter(packageKeywordWithSpace, ""))).node
else -> if (isContainingRequiredPartOfCode(text)) {
offset to ktPsiFactory.createBlockCodeFragment(text, null).node
} else {
Expand All @@ -85,8 +91,23 @@ class CommentsRule(configRules: List<RulesConfig>) : DiktatRule(
.isEmpty()
}
.forEach { (offset, parsedNode) ->
COMMENTED_OUT_CODE.warn(configRules, emitWarn, isFixMode, parsedNode.text.substringBefore("\n").trim(), offset,
errorNodesWithText.find { it.second.trim().contains(parsedNode.text, false) || parsedNode.text.contains(it.second.trim(), false) }?.first!!)
val invalidNode = errorNodesWithText.find {
it.second.trim().contains(parsedNode.text, false) ||
parsedNode.text.contains(it.second.trim(), false)
}?.first
if (invalidNode == null) {
logger.warn("Text [${parsedNode.text}] is a piece of code, created from comment; " +
"but no matching text in comments has been found in the file ${node.getFilePath()}")
} else {
COMMENTED_OUT_CODE.warn(
configRules,
emitWarn,
isFixMode,
parsedNode.text.substringBefore("\n").trim(),
offset,
invalidNode
)
}
}
}

Expand Down Expand Up @@ -139,11 +160,22 @@ class CommentsRule(configRules: List<RulesConfig>) : DiktatRule(
private fun isContainingRequiredPartOfCode(text: String): Boolean =
text.contains("val ", true) || text.contains("var ", true) || text.contains("=", true) || (text.contains("{", true) && text.substringAfter("{").contains("}", true))

/**
* Some weak checks to see if this string can be used as a part of import statement.
* Only string surrounded in backticks or a dot-qualified expression (i.e., containing words maybe separated by dots)
* are considered for this case.
*/
private fun String.isPossibleImport(): Boolean = trimStart().startsWith(importKeywordWithSpace) &&
substringAfter(importKeywordWithSpace, "").run {
startsWith('`') && endsWith('`') || !contains(' ')
}

@Suppress("MaxLineLength")
companion object {
private val importKeyword = KtTokens.IMPORT_KEYWORD.value
private val packageKeyword = KtTokens.PACKAGE_KEYWORD.value
private val importOrPackage = """($importKeyword|$packageKeyword) """.toRegex()
private val logger = LoggerFactory.getLogger(CommentsRule::class.java)
private val importKeywordWithSpace = "${KtTokens.IMPORT_KEYWORD.value} "
private val packageKeywordWithSpace = "${KtTokens.PACKAGE_KEYWORD.value} "
private val importOrPackage = """($importKeywordWithSpace|$packageKeywordWithSpace)""".toRegex()
private val classRegex =
"""^\s*(public|private|protected)*\s*(internal)*\s*(open|data|sealed)*\s*(internal)*\s*(class|object)\s+(\w+)(\(.*\))*(\s*:\s*\w+(\(.*\))*)?\s*\{*$""".toRegex()
private val importOrPackageRegex = """^(import|package)?\s+([a-zA-Z.])+;*$""".toRegex()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,4 +453,21 @@ class CommentedCodeTest : LintTestBase(::CommentsRule) {
""".trimMargin()
)
}

@Test
@Tag(WarningNames.COMMENTED_OUT_CODE)
fun `should not trigger on 'imports'`() {
lintMethod(
"""
/* Checks if specified imports can be found in classpath. */
class Example
/* Checks if specified import can be found in classpath. */
class Example2
/* import this and you died. */
class Example3
""".trimMargin()
)
}
}

0 comments on commit 631c10e

Please sign in to comment.