Skip to content

Commit

Permalink
Fixed IdentifierNaming (#1746)
Browse files Browse the repository at this point in the history
- added toDeterministic() for repeatable functions
- fixed isFixMode and canBeAutoCorrected
  • Loading branch information
nulls authored Sep 25, 2023
1 parent feeeee9 commit 41265dc
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 30 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 @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -171,16 +171,16 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
""".trimIndent()

lintMethod(code,
DiktatError(1, 5, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} SOMEtest", true),
DiktatError(2, 11, ruleId, "${CONSTANT_UPPERCASE.warnText()} thisConstantShouldBeUpper", true),
DiktatError(1, 5, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} SOMEtest", false),
DiktatError(2, 11, ruleId, "${CONSTANT_UPPERCASE.warnText()} thisConstantShouldBeUpper", false),
DiktatError(3, 7, ruleId, "${CLASS_NAME_INCORRECT.warnText()} className", true),
DiktatError(4, 16, ruleId, "${CLASS_NAME_INCORRECT.warnText()} badClassName", true),
DiktatError(4, 33, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} FIRST", true),
DiktatError(4, 52, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} SECOND", true),
DiktatError(7, 19, ruleId, "${CONSTANT_UPPERCASE.warnText()} incorrect_case", true),
DiktatError(9, 13, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} INCORRECT", true),
DiktatError(12, 9, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} check_me", true),
DiktatError(13, 9, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} CHECK_ME", true)
DiktatError(4, 33, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} FIRST", false),
DiktatError(4, 52, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} SECOND", false),
DiktatError(7, 19, ruleId, "${CONSTANT_UPPERCASE.warnText()} incorrect_case", false),
DiktatError(9, 13, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} INCORRECT", false),
DiktatError(12, 9, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} check_me", false),
DiktatError(13, 9, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} CHECK_ME", false)
)
}

Expand Down Expand Up @@ -212,8 +212,8 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
data class ClassName(val FIRST: String, var SECOND: String)
""".trimIndent()
lintMethod(code,
DiktatError(1, 26, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} FIRST", true),
DiktatError(1, 45, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} SECOND", true)
DiktatError(1, 26, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} FIRST", false),
DiktatError(1, 45, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} SECOND", false)
)
}

Expand Down Expand Up @@ -346,7 +346,7 @@ class IdentifierNamingWarnTest : LintTestBase(::IdentifierNaming) {
const val I_AM_CONSTANT2 = ""
""".trimIndent()
lintMethod(code,
DiktatError(1, 5, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} I_AM_CONSTANT1", true),
DiktatError(1, 5, ruleId, "${VARIABLE_NAME_INCORRECT_FORMAT.warnText()} I_AM_CONSTANT1", false),
DiktatError(1, 5, ruleId, "${VARIABLE_HAS_PREFIX.warnText()} I_AM_CONSTANT1", true),
DiktatError(2, 11, ruleId, "${VARIABLE_HAS_PREFIX.warnText()} I_AM_CONSTANT2", true)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class PascalCase4 {}
class Pascalcase5 {}
class Pascalcase6 {}
class PascAlCase7 {}
class PascaLCase8 {}
class PascaLcase8 {}
class PascAlCase9 {}
class PascAlCase10 {}
class PascAlCase11 {}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ private val amConstant1 = ""
private val strangeName = ""
private val loWerValue = ""
private val lower = ""
private val valNX256 = ""
private val valNx256 = ""
private val voiceIpPort = ""

class A {
Expand All @@ -23,7 +23,7 @@ class A {

fun goo() {
val qwe = lowerSnake
val pre = valNX256
val pre = valNx256
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ object PascalCase4 {}
object Pascalcase5 {}
object Pascalcase6 {}
object PascAlCase7 {}
object PascaLCase8 {}
object PascaLcase8 {}
object PascAlCase9 {}

0 comments on commit 41265dc

Please sign in to comment.