Skip to content

Commit

Permalink
Merge branch 'master' into bugfix/no-braces-loops
Browse files Browse the repository at this point in the history
  • Loading branch information
DrAlexD authored Sep 25, 2023
2 parents 8f4686f + eb6ef05 commit d15ee36
Show file tree
Hide file tree
Showing 23 changed files with 131 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,13 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
val isNonPrivatePrimaryConstructorParameter = (node.psi as? KtParameter)?.run {
hasValOrVar() && getParentOfType<KtPrimaryConstructor>(true)?.valueParameters?.contains(this) == true && !isPrivate()
} ?: false
val isFix = isFixMode && !(isPublicOrNonLocalProperty || isNonPrivatePrimaryConstructorParameter)
val canBeAutoCorrected = !(isPublicOrNonLocalProperty || isNonPrivatePrimaryConstructorParameter)
namesOfVariables
.forEach { variableName ->
// variable should not contain only one letter in it's name. This is a bad example: b512
// but no need to raise a warning here if length of a variable. In this case we will raise IDENTIFIER_LENGTH
if (variableName.text.containsOneLetterOrZero() && variableName.text.length > 1) {
VARIABLE_NAME_INCORRECT.warn(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node)
VARIABLE_NAME_INCORRECT.warn(configRules, emitWarn, false, variableName.text, variableName.startOffset, node)
}
// check if identifier of a property has a confusing name
if (confusingIdentifierNames.contains(variableName.text) && !isValidCatchIdentifier(variableName) &&
Expand All @@ -176,15 +176,15 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
// it should be in UPPER_CASE, no need to raise this warning if it is one-letter variable
if (node.isConstant()) {
if (!variableName.text.isUpperSnakeCase() && variableName.text.length > 1) {
CONSTANT_UPPERCASE.warnAndFix(configRules, emitWarn, isFix, variableName.text, variableName.startOffset, node) {
(variableName as LeafPsiElement).rawReplaceWithText(variableName.text.toUpperSnakeCase())
CONSTANT_UPPERCASE.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node, canBeAutoCorrected) {
(variableName as LeafPsiElement).rawReplaceWithText(variableName.text.toDeterministic { toUpperSnakeCase() })
}
}
} else if (variableName.text != "_" && !variableName.text.isLowerCamelCase()) {
// variable name should be in camel case. The only exception is a list of industry standard variables like i, j, k.
VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(configRules, emitWarn, isFix, variableName.text, variableName.startOffset, node) {
VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node, canBeAutoCorrected) {
// FixMe: cover fixes with tests
val correctVariableName = variableName.text.toLowerCamelCase()
val correctVariableName = variableName.text.toDeterministic { toLowerCamelCase() }
variableName
.parent { it.elementType == KtFileElementType.INSTANCE }
?.findAllVariablesWithUsages { it.name == variableName.text }
Expand Down Expand Up @@ -282,7 +282,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
val className: ASTNode = node.getIdentifierName() ?: return emptyList()
if (!(className.text.isPascalCase())) {
CLASS_NAME_INCORRECT.warnAndFix(configRules, emitWarn, isFixMode, className.text, className.startOffset, className) {
(className as LeafPsiElement).rawReplaceWithText(className.text.toPascalCase())
(className as LeafPsiElement).rawReplaceWithText(className.text.toDeterministic { toPascalCase() })
}
}

Expand Down Expand Up @@ -321,7 +321,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
val objectName: ASTNode = node.getIdentifierName() ?: return emptyList()
if (!objectName.text.isPascalCase()) {
OBJECT_NAME_INCORRECT.warnAndFix(configRules, emitWarn, isFixMode, objectName.text, objectName.startOffset, objectName) {
(objectName as LeafPsiElement).rawReplaceWithText(objectName.text.toPascalCase())
(objectName as LeafPsiElement).rawReplaceWithText(objectName.text.toDeterministic { toPascalCase() })
}
}
return listOf(objectName)
Expand Down Expand Up @@ -383,7 +383,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
if (!functionName.text.isLowerCamelCase()) {
FUNCTION_NAME_INCORRECT_CASE.warnAndFix(configRules, emitWarn, isFixMode, functionName.text, functionName.startOffset, functionName) {
// FixMe: add tests for this
(functionName as LeafPsiElement).rawReplaceWithText(functionName.text.toLowerCamelCase())
(functionName as LeafPsiElement).rawReplaceWithText(functionName.text.toDeterministic { toLowerCamelCase() })
}
}

Expand Down Expand Up @@ -413,7 +413,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(

if (!aliasName.text.isPascalCase()) {
TYPEALIAS_NAME_INCORRECT_CASE.warnAndFix(configRules, emitWarn, isFixMode, aliasName.text, aliasName.startOffset, aliasName) {
(aliasName as LeafPsiElement).rawReplaceWithText(aliasName.text.toPascalCase())
(aliasName as LeafPsiElement).rawReplaceWithText(aliasName.text.toDeterministic { toPascalCase() })
}
}
return listOf(aliasName)
Expand Down Expand Up @@ -468,7 +468,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
* In which style enum members should be named
*/
val enumStyle = config["enumStyle"]?.let { styleString ->
val style = Style.values().firstOrNull {
val style = Style.entries.firstOrNull {
it.name == styleString.toUpperSnakeCase()
}
if (style == null || !style.isEnumStyle) {
Expand All @@ -486,6 +486,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
}

companion object {
private const val MAX_DETERMINISTIC_RUNS = 5
const val MAX_IDENTIFIER_LENGTH = 64
const val MIN_IDENTIFIER_LENGTH = 2
const val NAME_ID = "identifier-naming"
Expand All @@ -494,5 +495,16 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
val oneCharIdentifiers = setOf("i", "j", "k", "x", "y", "z")
val booleanMethodPrefixes = setOf("has", "is", "are", "have", "should", "can")
val confusingIdentifierNames = setOf("O", "D", "I", "l", "Z", "S", "e", "B", "h", "n", "m", "rn")

private fun String.toDeterministic(function: String.() -> String): String = generateSequence(function(this), function)
.runningFold(this to false) { (current, result), next ->
require(!result) {
"Should return a value already"
}
next to (current == next)
}
.take(MAX_DETERMINISTIC_RUNS)
.first { it.second }
.first
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import org.jetbrains.kotlin.KtNodeTypes.IF
import org.jetbrains.kotlin.KtNodeTypes.PROPERTY
import org.jetbrains.kotlin.KtNodeTypes.THEN
import org.jetbrains.kotlin.KtNodeTypes.VALUE_ARGUMENT_LIST
import org.jetbrains.kotlin.com.intellij.lang.ASTFactory
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
Expand Down Expand Up @@ -154,13 +155,11 @@ class CommentsFormatting(configRules: List<RulesConfig>) : DiktatRule(
val elseCodeBlock = elseBlock.getFirstChildWithType(BLOCK)!!
elseCodeBlock.addChild(comment,
elseCodeBlock.firstChildNode.treeNext)
elseCodeBlock.addChild(PsiWhiteSpaceImpl("\n"),
elseCodeBlock.addChild(ASTFactory.whitespace("\n"),
elseCodeBlock.firstChildNode.treeNext)
node.removeChild(comment)
} else {
elseKeyWord.treeParent.addChild(comment, elseKeyWord.treeNext)
elseKeyWord.treeParent.addChild(PsiWhiteSpaceImpl("\n"), elseKeyWord.treeNext)
node.removeChild(comment)
elseKeyWord.treeParent.addChild(ASTFactory.whitespace("\n"), elseKeyWord.treeNext)
}

val whiteSpace = elseKeyWord.prevNodeUntilNode(THEN, WHITE_SPACE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ import com.saveourtool.diktat.ruleset.utils.getIdentifierName
import org.jetbrains.kotlin.KtNodeTypes.ANNOTATION_ENTRY
import org.jetbrains.kotlin.KtNodeTypes.FUN
import org.jetbrains.kotlin.KtNodeTypes.MODIFIER_LIST
import org.jetbrains.kotlin.com.intellij.lang.ASTFactory
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.lexer.KtTokens.ABSTRACT_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.INTERNAL_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.OPEN_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.PRIVATE_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.PROTECTED_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.PUBLIC_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.WHITE_SPACE
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.psiUtil.isPrivate

Expand Down Expand Up @@ -58,7 +60,7 @@ class PreviewAnnotationRule(configRules: List<RulesConfig>) : DiktatRule(
val functionName = functionNameNode?.text ?: return

// warn if function is not private
if (!((functionNode.psi as KtNamedFunction).isPrivate())) {
if (!(functionNode.psi as KtNamedFunction).isPrivate()) {
PREVIEW_ANNOTATION.warnAndFix(
configRules,
emitWarn,
Expand Down Expand Up @@ -94,22 +96,23 @@ class PreviewAnnotationRule(configRules: List<RulesConfig>) : DiktatRule(
functionName.contains(PREVIEW_ANNOTATION_TEXT)

private fun addPrivateModifier(functionNode: ASTNode) {
val modifiersList = functionNode
.findChildByType(MODIFIER_LIST)
?.getChildren(KtTokens.MODIFIER_KEYWORDS)
?.toList()
// MODIFIER_LIST should be present since ANNOTATION_ENTRY is there
val modifierListNode = functionNode.findChildByType(MODIFIER_LIST) ?: return
val modifiersList = modifierListNode
.getChildren(KtTokens.MODIFIER_KEYWORDS)
.toList()

val isMethodAbstract = modifiersList?.any {
val isMethodAbstract = modifiersList.any {
it.elementType == ABSTRACT_KEYWORD
}

// private modifier is not applicable for abstract methods
if (isMethodAbstract == true) {
if (isMethodAbstract) {
return
}

// these modifiers could be safely replaced via `private`
val modifierForReplacement = modifiersList?.firstOrNull {
val modifierForReplacement = modifiersList.firstOrNull {
it.elementType in listOf(
PUBLIC_KEYWORD, PROTECTED_KEYWORD, INTERNAL_KEYWORD, OPEN_KEYWORD
)
Expand All @@ -118,19 +121,21 @@ class PreviewAnnotationRule(configRules: List<RulesConfig>) : DiktatRule(
modifierForReplacement?.let {
// replace current modifier with `private`
val parent = it.treeParent
parent.replaceChild(it, createPrivateModifierNode()
)
parent.replaceChild(it, createPrivateModifierNode())
} ?: run {
// the case, when there is no explicit modifier, i.e. `fun foo`
// just add `private` before function identifier `fun`
// just add `private` in MODIFIER_LIST at the end
// and move WHITE_SPACE before function identifier `fun` to MODIFIER_LIST
val funNode = functionNode.findAllNodesWithCondition { it.text == "fun" }.single()
// add `private ` nodes before `fun`
funNode.treeParent?.addChild(PsiWhiteSpaceImpl(" "), funNode)
funNode.treeParent?.addChild(createPrivateModifierNode(), funNode.treePrev)
val whiteSpaceAfterAnnotation = modifierListNode.treeNext
modifierListNode.addChild(whiteSpaceAfterAnnotation, null)
modifierListNode.addChild(createPrivateModifierNode(), null)
// add ` ` node before `fun`
functionNode.addChild(ASTFactory.leaf(WHITE_SPACE, " "), funNode)
}
}

private fun createPrivateModifierNode() = KotlinParser().createNode("private")
private fun createPrivateModifierNode() = ASTFactory.leaf(PRIVATE_KEYWORD, "private")

companion object {
const val ANNOTATION_SYMBOL = "@"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class StringTemplateFormatRule(configRules: List<RulesConfig>) : DiktatRule(

if (identifierName != null && node.treeParent.text.trim('"', '$') == identifierName) {
STRING_TEMPLATE_QUOTES.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) {
val identifier = node.findChildByType(REFERENCE_EXPRESSION)!!.copyElement()
val identifier = node.findChildByType(REFERENCE_EXPRESSION)!!
// node.treeParent is String template that we need to delete
node.treeParent.treeParent.addChild(identifier, node.treeParent)
node.treeParent.treeParent.removeChild(node.treeParent)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import org.jetbrains.kotlin.KtNodeTypes.WHEN_CONDITION_EXPRESSION
import org.jetbrains.kotlin.KtNodeTypes.WHEN_CONDITION_IN_RANGE
import org.jetbrains.kotlin.KtNodeTypes.WHEN_CONDITION_IS_PATTERN
import org.jetbrains.kotlin.KtNodeTypes.WHEN_ENTRY
import org.jetbrains.kotlin.com.intellij.lang.ASTFactory
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.kdoc.lexer.KDocTokens.KDOC
import org.jetbrains.kotlin.lexer.KtTokens.BLOCK_COMMENT
import org.jetbrains.kotlin.lexer.KtTokens.COMMA
Expand Down Expand Up @@ -101,9 +101,13 @@ class TrailingCommaRule(configRules: List<RulesConfig>) : DiktatRule(
}

private fun ASTNode.checkTrailingComma(config: Boolean) {
val shouldFix = this.siblings(true).toList().run {
!this.map { it.elementType }.contains(COMMA) && this.any { it.isWhiteSpaceWithNewline() || it.isPartOfComment() }
}
val noCommaInSiblings = siblings(true).toSet()
.let { siblings ->
siblings.none { it.elementType == COMMA } && siblings.any { it.isWhiteSpaceWithNewline() || it.isPartOfComment() }
}
val noCommaInChildren = children().none { it.elementType == COMMA }
val shouldFix = noCommaInSiblings && noCommaInChildren

if (shouldFix && config) {
// we should write type of node in warning, to make it easier for user to find the parameter
TRAILING_COMMA.warnAndFix(configRules, emitWarn, isFixMode, "after ${this.elementType}: ${this.text}", this.startOffset, this) {
Expand All @@ -116,9 +120,9 @@ class TrailingCommaRule(configRules: List<RulesConfig>) : DiktatRule(
val comments = listOf(EOL_COMMENT, BLOCK_COMMENT, KDOC)
val firstCommentNodeOrNull = if (this.elementType == VALUE_PARAMETER) this.children().firstOrNull { it.elementType in comments } else null
firstCommentNodeOrNull?.let {
this.addChild(LeafPsiElement(COMMA, ","), firstCommentNodeOrNull)
this.addChild(ASTFactory.leaf(COMMA, ","), it)
}
?: parent.addChild(LeafPsiElement(COMMA, ","), this.treeNext)
?: parent.addChild(ASTFactory.leaf(COMMA, ","), this.treeNext)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ import org.jetbrains.kotlin.KtNodeTypes.FUN
import org.jetbrains.kotlin.KtNodeTypes.OBJECT_DECLARATION
import org.jetbrains.kotlin.KtNodeTypes.SUPER_TYPE_ENTRY
import org.jetbrains.kotlin.KtNodeTypes.SUPER_TYPE_LIST
import org.jetbrains.kotlin.com.intellij.lang.ASTFactory
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
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.lexer.KtTokens.CLASS_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.IDENTIFIER
import org.jetbrains.kotlin.lexer.KtTokens.INTERFACE_KEYWORD
Expand Down Expand Up @@ -50,14 +49,16 @@ class StatelessClassesRule(configRules: List<RulesConfig>) : DiktatRule(
if (isClassExtendsValidInterface(node, interfaces) && isStatelessClass(node)) {
OBJECT_IS_PREFERRED.warnAndFix(configRules, emitWarn, isFixMode,
"class ${(node.psi as KtClass).name!!}", node.startOffset, node) {
val newObjectNode = CompositeElement(OBJECT_DECLARATION)
val newObjectNode = ASTFactory.composite(OBJECT_DECLARATION)
val children = node.children().toList()
node.treeParent.addChild(newObjectNode, node)
node.children().forEach {
newObjectNode.addChild(it.copyElement(), null)
children.forEach {
if (it.elementType == CLASS_KEYWORD) {
newObjectNode.addChild(ASTFactory.leaf(OBJECT_KEYWORD, "object"))
} else {
newObjectNode.addChild(it)
}
}
newObjectNode.addChild(LeafPsiElement(OBJECT_KEYWORD, "object"),
newObjectNode.getFirstChildWithType(CLASS_KEYWORD))
newObjectNode.removeChild(newObjectNode.getFirstChildWithType(CLASS_KEYWORD)!!)
node.treeParent.removeChild(node)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.saveourtool.diktat.ruleset.rules.chapter1.IdentifierNaming
import com.saveourtool.diktat.util.FixTestBase

import generated.WarningNames
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

Expand All @@ -23,14 +22,12 @@ class IdentifierNamingFixTest : FixTestBase(
fixAndCompare("identifiers/IdentifierNameRegressionExpected.kt", "identifiers/IdentifierNameRegressionTest.kt")
}

@Disabled("https://github.com/saveourtool/diktat/issues/1737")
@Test
@Tag(WarningNames.CLASS_NAME_INCORRECT)
fun `incorrect class name fix`() {
fixAndCompare("class_/IncorrectClassNameExpected.kt", "class_/IncorrectClassNameTest.kt")
}

@Disabled("https://github.com/saveourtool/diktat/issues/1737")
@Test
@Tag(WarningNames.OBJECT_NAME_INCORRECT)
fun `incorrect object name fix`() {
Expand All @@ -49,7 +46,6 @@ class IdentifierNamingFixTest : FixTestBase(
fixAndCompare("identifiers/ConstantValNameExpected.kt", "identifiers/ConstantValNameTest.kt")
}

@Disabled("https://github.com/saveourtool/diktat/issues/1737")
@Test
@Tag(WarningNames.VARIABLE_NAME_INCORRECT_FORMAT)
fun `incorrect variable name case fix`() {
Expand Down
Loading

0 comments on commit d15ee36

Please sign in to comment.