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

Fixed IdentifierNaming #1746

Merged
merged 6 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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 @@ -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)
nulls marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -216,7 +216,7 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
.forEach { variableName ->
// generally, variables with prefixes are not allowed (like mVariable, xCode, iValue)
if (variableName.text.hasPrefix()) {
VARIABLE_HAS_PREFIX.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node) {
VARIABLE_HAS_PREFIX.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset, node, canBeAutoCorrected) {
(variableName as LeafPsiElement).rawReplaceWithText(variableName.text.removePrefix())
}
}
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 @@ -489,10 +489,22 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
const val MAX_IDENTIFIER_LENGTH = 64
const val MIN_IDENTIFIER_LENGTH = 2
const val NAME_ID = "identifier-naming"
private const val MAX_DETERMINISTIC_RUNS = 5

// FixMe: this should be moved to properties
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 @@ -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 {}
Loading