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.

Sign up for GitHub

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

Fixed IdentifierNaming #1746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4bdcf27
Fixed IdentifierNaming
nulls Sep 22, 2023
f8590fe
diktatFix
nulls Sep 22, 2023
cc6f799
diktatFix 2
nulls Sep 22, 2023
113da78
diktatFix 3
nulls Sep 22, 2023
e0a1124
fixed warn tests and restore the old logic
nulls Sep 22, 2023
1e41d01
diktatFix
nulls Sep 22, 2023
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
34 changes: 23 additions & 11 deletions ...t-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/IdentifierNaming.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,13 @@
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 @@
// 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 @@
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 @@
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 @@
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() })

Check warning on line 386 in diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/IdentifierNaming.kt

View check run for this annotation

Codecov / codecov/patch

diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/IdentifierNaming.kt#L386 

Added line #L386 was not covered by tests
}
}

Expand Down Expand Up @@ -413,7 +413,7 @@

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 @@
* 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 @@
}

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 @@
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"

Check warning on line 502 in diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/IdentifierNaming.kt

View check run for this annotation

Codecov / codecov/patch

diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter1/IdentifierNaming.kt#L502 

Added line #L502 was not covered by tests
}
next to (current == next)
}
.take(MAX_DETERMINISTIC_RUNS)
.first { it.second }
.first
}
}
4 changes: 0 additions & 4 deletions ...-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter1/IdentifierNamingFixTest.kt
View file
Open in desktop
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
22 changes: 11 additions & 11 deletions ...rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter1/IdentifierNamingWarnTest.kt
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion diktat-rules/src/test/resources/test/paragraph1/naming/class_/IncorrectClassNameExpected.kt
View file
Open in desktop
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 {}
4 changes: 2 additions & 2 deletions diktat-rules/src/test/resources/test/paragraph1/naming/identifiers/VariableNamingExpected.kt
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...at-rules/src/test/resources/test/paragraph1/naming/object_/IncorrectObjectNameExpected.kt
View file
Open in desktop
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