Skip to content

Commit

Permalink
Merge branch 'master' into bugfix/incorrect_package_name_fix_with_ann…
Browse files Browse the repository at this point in the history
…otations
  • Loading branch information
kgevorkyan authored May 12, 2021
2 parents e838549 + 2b17b31 commit 1e14fda
Show file tree
Hide file tree
Showing 13 changed files with 245 additions and 25 deletions.
4 changes: 4 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@
- name: MAGIC_NUMBER
enabled: true
configuration:
# Ignore numbers from test
ignoreTest: "true"
# Ignore numbers
ignoreNumbers: "-1, 1, 0, 2"
# Is ignore override hashCode function
Expand All @@ -312,6 +314,8 @@
ignorePropertyDeclaration: "false"
# Is ignore local variable
ignoreLocalVariableDeclaration: "false"
# Is ignore value parameter
ignoreValueParameter: "true"
# Is ignore constant
ignoreConstantDeclaration: "true"
# Is ignore property in companion object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType
import org.cqfn.diktat.ruleset.utils.getIdentifierName
import org.cqfn.diktat.ruleset.utils.hasChildMatching
import org.cqfn.diktat.ruleset.utils.hasTrailingNewlineInTagBody
import org.cqfn.diktat.ruleset.utils.kDocTags
import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine
import org.cqfn.diktat.ruleset.utils.reversedChildren

import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.KDOC
Expand All @@ -39,7 +41,6 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType
import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet
import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
import org.jetbrains.kotlin.kdoc.psi.impl.KDocTag
import org.jetbrains.kotlin.lexer.KtTokens
Expand Down Expand Up @@ -238,11 +239,9 @@ class KdocFormatting(configRules: List<RulesConfig>) : DiktatRule(
previousTag.addChild(treePrev.clone() as ASTNode, null)
previousTag.addChild(this.clone() as ASTNode, null)
}
?: run {
firstBasicTag.node.applyToPrevSibling(KDOC_LEADING_ASTERISK) {
treeParent.addChild(treePrev.clone() as ASTNode, this)
treeParent.addChild(this.clone() as ASTNode, treePrev)
}
?: firstBasicTag.node.applyToPrevSibling(KDOC_LEADING_ASTERISK) {
treeParent.addChild(treePrev.clone() as ASTNode, this)
treeParent.addChild(this.clone() as ASTNode, treePrev)
}
} else {
firstBasicTag.node.apply {
Expand All @@ -256,27 +255,26 @@ class KdocFormatting(configRules: List<RulesConfig>) : DiktatRule(
}

private fun checkEmptyLinesBetweenBasicTags(basicTags: List<KDocTag>) {
val tagsWithRedundantEmptyLines = basicTags.dropLast(1).filterNot { tag ->
val tagsWithRedundantEmptyLines = basicTags.dropLast(1).filter { tag ->
val nextWhiteSpace = tag.node.nextSibling { it.elementType == WHITE_SPACE }
val noEmptyKdocLines = tag
.node
.getChildren(TokenSet.create(KDOC_LEADING_ASTERISK))
.filter { it.treeNext == null || it.treeNext.elementType == WHITE_SPACE }
.count() == 0
nextWhiteSpace?.text?.count { it == '\n' } == 1 && noEmptyKdocLines
// either there is a trailing blank line in tag's body OR there are empty lines right after this tag
tag.hasTrailingNewlineInTagBody() || nextWhiteSpace?.text?.count { it == '\n' } != 1
}

tagsWithRedundantEmptyLines.forEach { tag ->
KDOC_NO_NEWLINES_BETWEEN_BASIC_TAGS.warnAndFix(configRules, emitWarn, isFixMode,
"@${tag.name}", tag.startOffset, tag.node) {
tag.node.nextSibling { it.elementType == WHITE_SPACE }?.leaveOnlyOneNewLine()
// the first asterisk before tag is not included inside KDOC_TAG node
// we look for the second and take its previous which should be WHITE_SPACE with newline
tag
.node
.getAllChildrenWithType(KDOC_LEADING_ASTERISK)
.firstOrNull()
?.let { tag.node.removeRange(it.treePrev, null) }
if (tag.hasTrailingNewlineInTagBody()) {
// if there is a blank line in tag's body, we remove it and everything after it, so that the next white space is kept in place
// we look for the last LEADING_ASTERISK and take its previous node which should be WHITE_SPACE with newline
tag.node.reversedChildren()
.takeWhile { it.elementType == WHITE_SPACE || it.elementType == KDOC_LEADING_ASTERISK }
.firstOrNull { it.elementType == KDOC_LEADING_ASTERISK }
?.let { tag.node.removeRange(it.treePrev, null) }
} else {
// otherwise we remove redundant blank lines from white space node after tag
tag.node.nextSibling { it.elementType == WHITE_SPACE }?.leaveOnlyOneNewLine()
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.cqfn.diktat.ruleset.rules.chapter3

import org.cqfn.diktat.common.config.rules.RuleConfiguration
import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.common.config.rules.getCommonConfiguration
import org.cqfn.diktat.common.config.rules.getRuleConfig
import org.cqfn.diktat.ruleset.constants.Warnings.MAGIC_NUMBER
import org.cqfn.diktat.ruleset.rules.DiktatRule
Expand All @@ -16,6 +17,7 @@ import com.pinterest.ktlint.core.ast.ElementType.MINUS
import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE
import com.pinterest.ktlint.core.ast.ElementType.PROPERTY
import com.pinterest.ktlint.core.ast.ElementType.RANGE
import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER
import com.pinterest.ktlint.core.ast.parent
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.psi.KtFunction
Expand All @@ -36,9 +38,14 @@ class MagicNumberRule(configRules: List<RulesConfig>) : DiktatRule(
configRules.getRuleConfig(MAGIC_NUMBER)?.configuration ?: emptyMap()
)
}
@Suppress("COLLAPSE_IF_STATEMENTS")
override fun logic(node: ASTNode) {
val filePath = node.getRootNode().getFilePath()
val config = configRules.getCommonConfiguration()
if (node.elementType == INTEGER_CONSTANT || node.elementType == FLOAT_CONSTANT) {
checkNumber(node, configuration)
if (!isLocatedInTest(filePath.splitPathToDirs(), config.testAnchors) || !configuration.isIgnoreTest) {
checkNumber(node, configuration)
}
}
}

Expand All @@ -49,7 +56,8 @@ class MagicNumberRule(configRules: List<RulesConfig>) : DiktatRule(
val isHashFunction = node.parent({ it.elementType == FUN && it.isHashFun() }) != null
val isConstant = node.parent({ it.elementType == PROPERTY && it.isConstant() }) != null
val isPropertyDeclaration = !isConstant && node.parent({ it.elementType == PROPERTY && !it.isNodeFromCompanionObject() }) != null
val isLocalVariable = node.parent({ it.isVarProperty() && (it.psi as KtProperty).isLocal }) != null
val isLocalVariable = node.parent({ it.elementType == PROPERTY && it.isVarProperty() && (it.psi as KtProperty).isLocal }) != null
val isValueParameter = node.parent({ it.elementType == VALUE_PARAMETER }) != null
val isCompanionObjectProperty = node.parent({ it.elementType == PROPERTY && it.isNodeFromCompanionObject() }) != null
val isEnums = node.parent({ it.elementType == ENUM_ENTRY }) != null
val isRanges = node.treeParent.run {
Expand All @@ -58,7 +66,7 @@ class MagicNumberRule(configRules: List<RulesConfig>) : DiktatRule(
}
val isExtensionFunctions = node.parent({ it.elementType == FUN && (it.psi as KtFunction).isExtensionDeclaration() }) != null &&
node.parents().find { it.elementType == PROPERTY } == null
val result = listOf(isHashFunction, isPropertyDeclaration, isLocalVariable, isConstant,
val result = listOf(isHashFunction, isPropertyDeclaration, isLocalVariable, isValueParameter, isConstant,
isCompanionObjectProperty, isEnums, isRanges, isExtensionFunctions).zip(mapConfiguration.map { configuration.getParameter(it.key) })
if (result.any { it.first && it.first != it.second } && !isIgnoreNumber) {
MAGIC_NUMBER.warn(configRules, emitWarn, isFixMode, nodeText, node.startOffset, node)
Expand All @@ -74,6 +82,11 @@ class MagicNumberRule(configRules: List<RulesConfig>) : DiktatRule(
* [RuleConfiguration] for configuration
*/
class MagicNumberConfiguration(config: Map<String, String>) : RuleConfiguration(config) {
/**
* Flag to ignore numbers from test
*/
val isIgnoreTest = config["ignoreTest"]?.toBoolean() ?: IGNORE_TEST

/**
* List of ignored numbers
*/
Expand All @@ -93,11 +106,13 @@ class MagicNumberRule(configRules: List<RulesConfig>) : DiktatRule(
}

companion object {
const val IGNORE_TEST = true
val ignoreNumbersList = listOf("-1", "1", "0", "2")
val mapConfiguration = mapOf(
"ignoreHashCodeFunction" to true,
"ignorePropertyDeclaration" to false,
"ignoreLocalVariableDeclaration" to false,
"ignoreValueParameter" to true,
"ignoreConstantDeclaration" to true,
"ignoreCompanionObjectPropertyDeclaration" to true,
"ignoreEnums" to false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.cqfn.diktat.ruleset.rules.chapter1.PackageNaming

import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.ANNOTATED_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT
import com.pinterest.ktlint.core.ast.ElementType.CONST_KEYWORD
Expand All @@ -29,6 +30,7 @@ import com.pinterest.ktlint.core.ast.ElementType.LBRACE
import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
import com.pinterest.ktlint.core.ast.ElementType.OVERRIDE_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.PRIVATE_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.PROPERTY
import com.pinterest.ktlint.core.ast.ElementType.PROTECTED_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.PUBLIC_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
Expand Down Expand Up @@ -114,6 +116,19 @@ fun ASTNode.isCorrect() = this.findAllDescendantsWithSpecificType(TokenType.ERRO
fun ASTNode.getAllChildrenWithType(elementType: IElementType): List<ASTNode> =
this.getChildren(null).filter { it.elementType == elementType }

/**
* Generates a sequence of this ASTNode's children in reversed order
*
* @return a reevrsed sequence of children
*/
fun ASTNode.reversedChildren(): Sequence<ASTNode> = sequence {
var node = lastChildNode
while (node != null) {
yield(node)
node = node.treePrev
}
}

/**
* Replaces the [beforeNode] of type [WHITE_SPACE] with the node with specified [text]
*
Expand Down Expand Up @@ -489,7 +504,7 @@ fun ASTNode?.isAccessibleOutside(): Boolean =
*/
fun ASTNode.hasSuppress(warningName: String) = parent({ node ->
val annotationNode = if (node.elementType != FILE) {
node.findChildByType(MODIFIER_LIST)
node.findChildByType(MODIFIER_LIST) ?: node.findChildByType(ANNOTATED_EXPRESSION)
} else {
node.findChildByType(FILE_ANNOTATION_LIST)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package org.cqfn.diktat.ruleset.utils
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.KDOC_SECTION
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline
import com.pinterest.ktlint.core.ast.prevSibling
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
Expand All @@ -33,6 +34,23 @@ fun ASTNode.kDocTags(): List<KDocTag> {
fun Iterable<KDocTag>.hasKnownKdocTag(knownTag: KDocKnownTag): Boolean =
this.find { it.knownTag == knownTag } != null

/**
* Checks for trailing newlines in tag's body. Handles cases, when there is no leading asterisk on an empty line:
* ```
* * @param param
*
* * @return
* ```
* as well as usual simple cases.
*
* @return true if there is a trailing newline
*/
fun KDocTag.hasTrailingNewlineInTagBody() = node.lastChildNode.isWhiteSpaceWithNewline() ||
node.reversedChildren()
.takeWhile { it.elementType == WHITE_SPACE || it.elementType == ElementType.KDOC_LEADING_ASTERISK }
.firstOrNull { it.elementType == ElementType.KDOC_LEADING_ASTERISK }
?.takeIf { it.treeNext == null || it.treeNext.elementType == WHITE_SPACE } != null

/**
* This method inserts a new tag into KDoc before specified another tag, aligning it with the rest of this KDoc
*
Expand Down
4 changes: 4 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@
- name: MAGIC_NUMBER
enabled: true
configuration:
# Ignore numbers from test
ignoreTest: "true"
# Ignore numbers
ignoreNumbers: "-1, 1, 0, 2"
# Is ignore override hashCode function
Expand All @@ -312,6 +314,8 @@
ignorePropertyDeclaration: "false"
# Is ignore local variable
ignoreLocalVariableDeclaration: "false"
# Is ignore value parameter
ignoreValueParameter: "true"
# Is ignore constant
ignoreConstantDeclaration: "true"
# Is ignore property in companion object
Expand Down
2 changes: 2 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,8 @@
- name: MAGIC_NUMBER
enabled: true
configuration:
# Ignore numbers from test
ignoreTest: "true"
# Ignore numbers
ignoreNumbers: "-1, 1, 0, 2"
# Is ignore override hashCode function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) {
"ignoreHashCodeFunction" to "true",
"ignorePropertyDeclaration" to "true",
"ignoreLocalVariableDeclaration" to "true",
"ignoreValueParameter" to "false",
"ignoreConstantDeclaration" to "true",
"ignoreCompanionObjectPropertyDeclaration" to "true",
"ignoreEnums" to "true",
Expand Down Expand Up @@ -142,4 +143,68 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) {
""".trimMargin(), rulesConfigList = rulesConfigMagicNumber
)
}

@Test
@Tag(WarningNames.MAGIC_NUMBER)
fun `check value parameter`() {
lintMethod(
"""
class TomlDecoder(
var elementsCount: Int = 100
)
""".trimMargin(),
)
}

@Test
@Tag(WarningNames.MAGIC_NUMBER)
fun `check value parameter with config`() {
lintMethod(
"""
class TomlDecoder(
var elementsCount: Int = 100
)
""".trimMargin(),
LintError(2, 46, ruleId, "${MAGIC_NUMBER.warnText()} 100", false),
rulesConfigList = rulesConfigMagicNumber
)
}

@Test
@Tag(WarningNames.MAGIC_NUMBER)
fun `check value parameter in function with config`() {
lintMethod(
"""
fun TomlDecoder(elementsCount: Int = 100) {}
""".trimMargin(),
LintError(1, 54, ruleId, "${MAGIC_NUMBER.warnText()} 100", false),
rulesConfigList = rulesConfigMagicNumber
)
}

@Test
@Tag(WarningNames.MAGIC_NUMBER)
fun `check ignore numbers in test`() {
lintMethod(
"""
|fun f1oo() {
| val m = -1
| val a: Byte = 4
| val b = 0xff
|}
|
|enum class A(b:Int) {
| A(-240),
| B(50),
| C(3),
|}
|@Override
|fun hashCode(): Int {
| return 32
|}
""".trimMargin(),
fileName = "src/test/kotlin/org/cqfn/diktat/test/hehe/MagicNumberTest.kt",
rulesConfigList = rulesConfigIgnoreNumbersMagicNumber,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,12 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin",
)
}

@Test
@Tag("DiktatRuleSetProvider")
fun `regression - should correctly handle tags with empty lines`() {
fixAndCompare("KdocFormattingMultilineTagsExpected.kt", "KdocFormattingMultilineTagsTest.kt")
}

companion object {
private const val DEFAULT_CONFIG_PATH = "../diktat-analysis.yml"
private val unfixedLintErrors: MutableList<LintError> = mutableListOf()
Expand Down
Loading

0 comments on commit 1e14fda

Please sign in to comment.