Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/new lines rule dot qualified expression and safe access expression #1337

Merged
merged 32 commits into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
a8c8ec0
### Whats added:
May 25, 2022
edfac82
### Whats added:
May 25, 2022
e24e06b
### Whats added:
May 25, 2022
678a95b
### Whats added:
May 25, 2022
56bef60
### Whats added:
May 25, 2022
e70e1e8
### Whats added:
May 26, 2022
89ac89e
### Whats added:
May 26, 2022
7fb1d93
### Whats added:
May 27, 2022
8f3ed88
### Whats added:
May 31, 2022
80d5e21
### Whats added:
May 31, 2022
51c7c47
### Whats added:
May 31, 2022
9e03d0a
### Whats added:
May 31, 2022
57201a6
### Whats added:
May 31, 2022
f8a817a
Merge branch 'master' into bugfix/NewLinesRuleDotQualifiedExpression
Arrgentum May 31, 2022
dfcf81a
### Whats added:
Jun 1, 2022
0e20d17
Merge branch 'master' into bugfix/NewLinesRuleDotQualifiedExpression
Jun 1, 2022
5507399
### Whats added:
Jun 1, 2022
b4898a8
### Whats added:
Jun 1, 2022
d8fb515
Correct warning message
Jun 1, 2022
163fb83
Corrected diktats code according to this rule
Jun 1, 2022
9192489
Corrected diktats code according to this rule
Jun 1, 2022
686f2d6
Corrected diktats code according to this rule
Jun 1, 2022
120d2e4
Corrected diktats code according to this rule
Jun 1, 2022
279d697
Corrected diktats code according to this rule
Jun 1, 2022
55c6e2c
Corrected the code according to the review
Jun 3, 2022
05c5a44
Corrected the code using check
Jun 3, 2022
cfbdc7d
commented logics code
Jun 9, 2022
593a6ed
Merge branch 'master' into bugfix/NewLinesRuleDotQualifiedExpression
Arrgentum Jun 9, 2022
c2c49c5
corrected code with diktat:fix
Jun 9, 2022
e418532
Merge remote-tracking branch 'origin/bugfix/NewLinesRuleDotQualifiedE…
Jun 9, 2022
0a15780
corrected code with diktat:fix
Jun 9, 2022
07487f1
Merge branch 'master' into bugfix/NewLinesRuleDotQualifiedExpression
Arrgentum Jun 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ class DiktatGradlePluginTest {
@Test
fun `check default extension properties`() {
val diktatExtension = project.extensions.getByName("diktat") as DiktatExtension
val actualInputs = project.tasks.named("diktatCheck", DiktatJavaExecTaskBase::class.java).get().actualInputs
val actualInputs = project.tasks
.named("diktatCheck", DiktatJavaExecTaskBase::class.java)
.get()
.actualInputs
Assertions.assertFalse(diktatExtension.debug)
Assertions.assertIterableEquals(project.fileTree("src").files, actualInputs.files)
Assertions.assertTrue(actualInputs.files.isNotEmpty())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,10 @@ class HeaderCommentRule(configRules: List<RulesConfig>) : DiktatRule(

// Check if provided copyright node differs only in the first date from pattern
private fun isCopyRightTextMatchesPattern(copyrightNode: ASTNode?, copyrightPattern: String): Boolean {
val copyrightText = copyrightNode?.text?.replace("/*", "")?.replace("*/", "")?.replace("*", "")
val copyrightText = copyrightNode?.text
?.replace("/*", "")
?.replace("*/", "")
?.replace("*", "")

val datesInPattern = hyphenRegex.find(copyrightPattern)?.value
val datesInCode = copyrightText?.let { hyphenRegex.find(it)?.value }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,10 @@ class CommentsFormatting(configRules: List<RulesConfig>) : DiktatRule(
} else if (node.treeParent.elementType != FILE && node.treeParent.treePrev != null &&
node.treeParent.treePrev.treePrev != null) {
// When comment inside of a PROPERTY
node.treeParent.treePrev.treePrev.elementType == LBRACE
node.treeParent
.treePrev
.treePrev
.elementType == LBRACE
} else {
node.treeParent.getAllChildrenWithType(node.elementType).first() == node
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,15 @@ class KdocComments(configRules: List<RulesConfig>) : DiktatRule(
propertyInLocalKdoc: ASTNode?
) {
val kdocText = if (prevComment.elementType == KDOC) {
prevComment.text.removePrefix("/**").removeSuffix("*/").trim('\n', ' ')
prevComment.text
.removePrefix("/**")
.removeSuffix("*/")
.trim('\n', ' ')
} else {
prevComment.text.removePrefix("/*").removeSuffix("*/").trim('\n', ' ')
prevComment.text
.removePrefix("/*")
.removeSuffix("*/")
.trim('\n', ' ')
}
// if property is documented with KDoc, which has a property tag inside, then it can contain some additional more complicated
// structure, that will be hard to move automatically
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,11 @@ class CollapseIfStatementsRule(configRules: List<RulesConfig>) : DiktatRule(

private fun takeCommentsBeforeNestedIf(node: ASTNode): List<ASTNode> {
val thenNode = (node.psi as KtIfExpression).then?.node
return thenNode?.children()?.takeWhile { it.elementType != IF }?.filter {
it.elementType == EOL_COMMENT ||
it.elementType == BLOCK_COMMENT
}
return thenNode?.children()
?.takeWhile { it.elementType != IF }
?.filter {
it.elementType == EOL_COMMENT || it.elementType == BLOCK_COMMENT
}
?.toList() ?: emptyList()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
}
// check, that space to split is a part of text - not code
// If the space split is part of the code, then there is a chance of breaking the code when fixing, that why we should ignore it
val isSpaceIsWhiteSpace = node.psi.findElementAt(delimiterIndex)!!.node.isWhiteSpace()
val isSpaceIsWhiteSpace = node.psi
.findElementAt(delimiterIndex)!!
.node
.isWhiteSpace()
if (isSpaceIsWhiteSpace) {
return None()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ class StringTemplateFormatRule(configRules: List<RulesConfig>) : DiktatRule(
?.elementType == ARRAY_ACCESS_EXPRESSION

return if (onlyOneRefExpr && !isArrayAccessExpression) {
(!(node.treeNext.text.first().isLetterOrDigit() // checking if first letter is valid
(!(node.treeNext
.text
.first()
.isLetterOrDigit() // checking if first letter is valid
|| node.treeNext.text.startsWith("_")) || node.treeNext.elementType == CLOSING_QUOTE)
} else if (!isArrayAccessExpression) {
node.hasAnyChildOfTypes(FLOAT_CONSTANT, INTEGER_CONSTANT) // it also fixes "${1.0}asd" cases
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ class FileStructureRule(configRules: List<RulesConfig>) : DiktatRule(
) {
findAllReferences(node)
packageName = (node.findChildByType(PACKAGE_DIRECTIVE)?.psi as KtPackageDirective).qualifiedName
node.findChildByType(IMPORT_LIST)?.getChildren(TokenSet.create(IMPORT_DIRECTIVE))?.toList()
node.findChildByType(IMPORT_LIST)
?.getChildren(TokenSet.create(IMPORT_DIRECTIVE))
?.toList()
?.forEach { import ->
val ktImportDirective = import.psi as KtImportDirective
val importName = ktImportDirective.importPath?.importedName?.asString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,17 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
/**
* If it is triple-quoted string template we need to indent all its parts
*/
@Suppress("LOCAL_VARIABLE_EARLY_DECLARATION")
private fun fixStringLiteral(
whiteSpace: PsiWhiteSpace,
expectedIndent: Int,
actualIndent: Int
) {
val textIndent = " ".repeat(expectedIndent + INDENT_SIZE)
val templateEntries = whiteSpace.node.treeNext.firstChildNode.getAllChildrenWithType(LITERAL_STRING_TEMPLATE_ENTRY)
val templateEntries = whiteSpace.node
.treeNext
.firstChildNode
.getAllChildrenWithType(LITERAL_STRING_TEMPLATE_ENTRY)
templateEntries.forEach { node ->
if (!node.text.contains("\n")) {
fixFirstTemplateEntries(node, textIndent, actualIndent)
Expand All @@ -263,8 +267,14 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
) {
val correctedText = StringBuilder()
// shift of the node depending on its initial string template indent
val nodeStartIndent = if (node.firstChildNode.text.takeWhile { it == ' ' }.count() - actualIndent - INDENT_SIZE > 0) {
node.firstChildNode.text.takeWhile { it == ' ' }.count() - actualIndent - INDENT_SIZE
val nodeStartIndent = if (node.firstChildNode
.text
.takeWhile { it == ' ' }
.count() - actualIndent - INDENT_SIZE > 0) {
node.firstChildNode
.text
.takeWhile { it == ' ' }
.count() - actualIndent - INDENT_SIZE
} else {
0
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,7 @@ import org.cqfn.diktat.ruleset.constants.Warnings.COMPLEX_EXPRESSION
import org.cqfn.diktat.ruleset.constants.Warnings.REDUNDANT_SEMICOLON
import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_NEWLINES
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.appendNewlineMergingWhiteSpace
import org.cqfn.diktat.ruleset.utils.emptyBlockList
import org.cqfn.diktat.ruleset.utils.extractLineOfText
import org.cqfn.diktat.ruleset.utils.findAllDescendantsWithSpecificType
import org.cqfn.diktat.ruleset.utils.findParentNodeWithSpecificType
import org.cqfn.diktat.ruleset.utils.getFilePath
import org.cqfn.diktat.ruleset.utils.getIdentifierName
import org.cqfn.diktat.ruleset.utils.getRootNode
import org.cqfn.diktat.ruleset.utils.hasParent
import org.cqfn.diktat.ruleset.utils.isBeginByNewline
import org.cqfn.diktat.ruleset.utils.isEol
import org.cqfn.diktat.ruleset.utils.isFollowedByNewline
import org.cqfn.diktat.ruleset.utils.isGradleScript
import org.cqfn.diktat.ruleset.utils.isSingleLineIfElse
import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine
import org.cqfn.diktat.ruleset.utils.*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, when we have an editor config - we should prohibit wildcard imports

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please enable it in diktat-analysis

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's in the excludes, sorry, no need to change


import com.pinterest.ktlint.core.ast.ElementType.ANDAND
import com.pinterest.ktlint.core.ast.ElementType.ARROW
Expand Down Expand Up @@ -124,6 +110,7 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
private val configuration by lazy {
NewlinesRuleConfiguration(configRules.getRuleConfig(WRONG_NEWLINES)?.configuration ?: emptyMap())
}

override fun logic(node: ASTNode) {
when (node.elementType) {
SEMICOLON -> handleSemicolon(node)
Expand All @@ -135,11 +122,69 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
BLOCK -> handleLambdaBody(node)
RETURN -> handleReturnStatement(node)
SUPER_TYPE_LIST, VALUE_PARAMETER_LIST, VALUE_ARGUMENT_LIST -> handleList(node)
DOT_QUALIFIED_EXPRESSION, SAFE_ACCESS_EXPRESSION, POSTFIX_EXPRESSION -> handDotQualifiedAndSafeAccessExpression(node)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should have fixed previous logic and not make a new one...

image

else -> {
}
}
}

@Suppress("GENERIC_VARIABLE_WRONG_DECLARATION", "MagicNumber")
private fun handDotQualifiedAndSafeAccessExpression(node: ASTNode) {
val listParentTypesNoFix = listOf<IElementType>(PACKAGE_DIRECTIVE, IMPORT_DIRECTIVE, VALUE_PARAMETER_LIST,
VALUE_ARGUMENT_LIST, DOT_QUALIFIED_EXPRESSION, SAFE_ACCESS_EXPRESSION, POSTFIX_EXPRESSION)
if (isNotFindParentNodeWithSpecificManyType(node, listParentTypesNoFix)) {
val listDot = node.findAllNodesWithCondition(
withSelf = true,
excludeChildrenCondition = { !isDotQuaOrSafeAccessOrPostfixExpression(it) }
) {
isDotQuaOrSafeAccessOrPostfixExpression(it) && it.elementType != POSTFIX_EXPRESSION
}.reversed()
if (listDot.size > 3) {
val without = listDot.filterNot {
val whiteSpaceBeforeDotOrSafeAccess = it.findChildByType(DOT)?.treePrev ?: it.findChildByType(SAFE_ACCESS)?.treePrev
whiteSpaceBeforeDotOrSafeAccess?.elementType == WHITE_SPACE && whiteSpaceBeforeDotOrSafeAccess.text.lines().size > 1
}
if (without.size > 1 || (without.size == 1 && without[0] != listDot[0])) {
WRONG_NEWLINES.warnAndFix(configRules, emitWarn, isFixMode, "should be split before second and other dot/safe access", node.startOffset, node) {
fixDotQualifiedExpression(listDot)
}
}
}
}
}

/**
* Return false, if you find parent with types in list else return true
*/
private fun isNotFindParentNodeWithSpecificManyType(node: ASTNode, list: List<IElementType>): Boolean {
list.forEach { elem ->
node.findParentNodeWithSpecificType(elem)?.let {
return false
}
}
return true
}

/**
* Fix Dot Qualified Expression and Safe Access Expression -
* 1) Append new White Space node before second and subsequent node Dot or Safe Access
* in Dot Qualified Expression? Safe Access Expression and Postfix Expression
* 2) If before first Dot or Safe Access node stay White Space node with \n - remove this node
*/
private fun fixDotQualifiedExpression(list: List<ASTNode>) {
list.forEachIndexed { index, astNode ->
val dotNode = astNode.getFirstChildWithType(DOT) ?: astNode.getFirstChildWithType(SAFE_ACCESS)
val nodeBeforeDot = dotNode?.treePrev
if (index > 0) {
astNode.appendNewlineMergingWhiteSpace(nodeBeforeDot, dotNode)
}
}
}

private fun isDotQuaOrSafeAccessOrPostfixExpression(node: ASTNode) =
node.elementType == DOT_QUALIFIED_EXPRESSION || node.elementType == SAFE_ACCESS_EXPRESSION || node.elementType == POSTFIX_EXPRESSION
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed


/**
* Check that EOL semicolon is used only in enums
*/
Expand Down Expand Up @@ -294,9 +339,13 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
}
}

@Suppress("TOO_LONG_FUNCTION")
private fun handleLambdaBody(node: ASTNode) {
if (node.treeParent.elementType == FUNCTION_LITERAL) {
val isSingleLineLambda = node.treeParent.text.lines().size == 1
val isSingleLineLambda = node.treeParent
.text
.lines()
.size == 1
val arrowNode = node.siblings(false).find { it.elementType == ARROW }
if (!isSingleLineLambda && arrowNode != null) {
// lambda with explicit arguments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,18 @@ class WhiteSpaceRule(configRules: List<RulesConfig>) : DiktatRule(
else -> {
}
}
val isDeclaration = node.treeParent.elementType == VALUE_PARAMETER_LIST && node.treeParent.treeParent.elementType.let {
it == PRIMARY_CONSTRUCTOR || it == FUN || it == CALL_EXPRESSION
}
val isCall = node.treeParent.elementType == VALUE_ARGUMENT_LIST && node.treeParent.treeParent.elementType.let {
it == CONSTRUCTOR_DELEGATION_CALL || it == CALL_EXPRESSION
}
val isDeclaration = node.treeParent.elementType == VALUE_PARAMETER_LIST && node.treeParent
.treeParent
.elementType
.let {
it == PRIMARY_CONSTRUCTOR || it == FUN || it == CALL_EXPRESSION
}
val isCall = node.treeParent.elementType == VALUE_ARGUMENT_LIST && node.treeParent
.treeParent
.elementType
.let {
it == CONSTRUCTOR_DELEGATION_CALL || it == CALL_EXPRESSION
}
if (isDeclaration || isCall) {
handleToken(node, 0, 0)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ class NullChecksRule(configRules: List<RulesConfig>) : DiktatRule(
private fun isComplexIfStatement(parentIf: ASTNode): Boolean {
val parentIfPsi = parentIf.psi
require(parentIfPsi is KtIfExpression)
return (parentIfPsi.`else`?.node?.firstChildNode?.elementType == IF_KEYWORD)
return (parentIfPsi.`else`
?.node
?.firstChildNode
?.elementType == IF_KEYWORD)
}

private fun conditionInIfStatement(node: ASTNode) {
Expand Down Expand Up @@ -218,7 +221,8 @@ class NullChecksRule(configRules: List<RulesConfig>) : DiktatRule(
.treeParent
.findChildByType(type)
?.let { it.findChildByType(BLOCK) ?: it }
?.findAllNodesWithCondition { it.elementType == BREAK }?.isNotEmpty()
?.findAllNodesWithCondition { it.elementType == BREAK }
?.isNotEmpty()
?: false

private fun ASTNode.extractLinesFromBlock(type: IElementType): List<String>? =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ class RunInScript(private val configRules: List<RulesConfig>) : Rule(NAME_ID) {

private fun checkScript(node: ASTNode) {
val isLambdaArgument = node.firstChildNode.hasChildOfType(LAMBDA_ARGUMENT)
val isLambdaInsideValueArgument = node.firstChildNode.findChildByType(VALUE_ARGUMENT_LIST)?.findChildByType(VALUE_ARGUMENT)?.findChildByType(LAMBDA_EXPRESSION) != null
val isLambdaInsideValueArgument = node.firstChildNode
.findChildByType(VALUE_ARGUMENT_LIST)
?.findChildByType(VALUE_ARGUMENT)
?.findChildByType(LAMBDA_EXPRESSION) != null
if (!isLambdaArgument && !isLambdaInsideValueArgument) {
warnRunInScript(node)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ class InlineClassesRule(configRules: List<RulesConfig>) : DiktatRule(
// Fixme: In Kotlin 1.4.30 inline classes may be used with internal constructors. When it will be released need to check it
if (hasValidProperties(classPsi) &&
!isExtendingClass(classPsi.node) &&
classPsi.node.getFirstChildWithType(MODIFIER_LIST)?.getChildren(null)?.all { it.elementType in goodModifiers } != false) {
classPsi.node
.getFirstChildWithType(MODIFIER_LIST)
?.getChildren(null)
?.all { it.elementType in goodModifiers } != false) {
// Fixme: since it's an experimental feature we shouldn't do fixer
INLINE_CLASS_CAN_BE_USED.warn(configRules, emitWarn, isFixMode, "class ${classPsi.name}", classPsi.node.startOffset, classPsi.node)
}
Expand All @@ -56,8 +59,14 @@ class InlineClassesRule(configRules: List<RulesConfig>) : DiktatRule(
return !classPsi.getProperties().single().isVar
} else if (classPsi.getProperties().isEmpty() && classPsi.hasExplicitPrimaryConstructor()) {
return classPsi.primaryConstructorParameters.size == 1 &&
!classPsi.primaryConstructorParameters.first().node.hasChildOfType(VAR_KEYWORD) &&
classPsi.primaryConstructor?.visibilityModifierType()?.value?.let { it == "public" } ?: true
!classPsi.primaryConstructorParameters
.first()
.node
.hasChildOfType(VAR_KEYWORD) &&
classPsi.primaryConstructor
?.visibilityModifierType()
?.value
?.let { it == "public" } ?: true
}
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -539,15 +539,17 @@ fun ASTNode.leaveExactlyNumNewLines(num: Int) {
}

/**
* If [whiteSpaceNode] is not null and has type [WHITE_SPACE], prepend a line break to it's text.
* Otherwise, insert a new node with a line break before [beforeNode]
* If [whiteSpaceNode] is not null and has type [WHITE_SPACE] and this [WHITE_SPACE] contains 1 line, prepend a line break to it's text.
* If [whiteSpaceNode] is null or has`t type [WHITE_SPACE], insert a new node with a line break before [beforeNode]
*
* @param whiteSpaceNode a node that can possibly be modified
* @param beforeNode a node before which a new WHITE_SPACE node will be inserted
*/
fun ASTNode.appendNewlineMergingWhiteSpace(whiteSpaceNode: ASTNode?, beforeNode: ASTNode?) {
if (whiteSpaceNode != null && whiteSpaceNode.elementType == WHITE_SPACE) {
(whiteSpaceNode as LeafPsiElement).rawReplaceWithText("\n${whiteSpaceNode.text}")
if (whiteSpaceNode.text.lines().size == 1) {
(whiteSpaceNode as LeafPsiElement).rawReplaceWithText("\n${whiteSpaceNode.text}")
}
} else {
addChild(PsiWhiteSpaceImpl("\n"), beforeNode)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ class KotlinParser {
.createBlockCodeFragment(text, null)
.node
.findChildByType(BLOCK)!!
.findChildByType(ERROR_ELEMENT)!!.findChildByType(IMPORT_LIST)!!
.findChildByType(ERROR_ELEMENT)!!
.findChildByType(IMPORT_LIST)!!
}
} else {
ktPsiFactory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ val JAVA = arrayOf("abstract", "assert", "boolean",
"try", "void", "volatile", "while")

@Suppress("VARIABLE_NAME_INCORRECT_FORMAT")
val KOTLIN = KtTokens.KEYWORDS.types.map { line -> line.toString() }
val KOTLIN = KtTokens.KEYWORDS
.types
.map { line -> line.toString() }
.plus(KtTokens.SOFT_KEYWORDS.types.map { line -> line.toString() })

val loggerPropertyRegex = "(log|LOG|logger)".toRegex()
Expand Down
Loading