Skip to content

Commit

Permalink
Merge branch 'master' into feature/ktlint-0.41.0
Browse files Browse the repository at this point in the history
  • Loading branch information
petertrr authored Aug 23, 2021
2 parents eba28f7 + 9fb026f commit ce196cf
Show file tree
Hide file tree
Showing 11 changed files with 111 additions and 7 deletions.
2 changes: 1 addition & 1 deletion diktat-common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<dependency>
<groupId>com.charleskorn.kaml</groupId>
<artifactId>kaml-jvm</artifactId>
<version>0.35.0</version>
<version>0.35.2</version>
</dependency>
<dependency>
<groupId>commons-cli</groupId>
Expand Down
14 changes: 13 additions & 1 deletion diktat-maven-plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,18 @@
<packaging>maven-plugin</packaging>

<properties>
<maven.api.version>3.8.1</maven.api.version>
<maven.api.version>3.8.2</maven.api.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.itf.version>0.11.0</maven.itf.version>
</properties>

<dependencies>
<!-- FixMe: Use this, when it's released -->
<!-- <dependency> -->
<!-- <groupId>org.apache.maven</groupId> -->
<!-- <artifactId>maven-bom</artifactId> -->
<!-- <version>${maven.api.version}</version> -->
<!-- </dependency> -->
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-core</artifactId>
Expand Down Expand Up @@ -78,6 +84,12 @@
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-cipher</artifactId>
<version>1.8</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.cqfn.diktat.ruleset.constants.Warnings.VARIABLE_NAME_INCORRECT_FORMAT
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.Style
import org.cqfn.diktat.ruleset.utils.containsOneLetterOrZero
import org.cqfn.diktat.ruleset.utils.findAllDescendantsWithSpecificType
import org.cqfn.diktat.ruleset.utils.findChildAfter
import org.cqfn.diktat.ruleset.utils.findLeafWithSpecificType
import org.cqfn.diktat.ruleset.utils.findParentNodeWithSpecificType
Expand All @@ -37,6 +38,7 @@ import org.cqfn.diktat.ruleset.utils.isOverridden
import org.cqfn.diktat.ruleset.utils.isPascalCase
import org.cqfn.diktat.ruleset.utils.isTextLengthInRange
import org.cqfn.diktat.ruleset.utils.isUpperSnakeCase
import org.cqfn.diktat.ruleset.utils.kDocTags
import org.cqfn.diktat.ruleset.utils.removePrefix
import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages
import org.cqfn.diktat.ruleset.utils.toLowerCamelCase
Expand All @@ -46,10 +48,13 @@ import org.cqfn.diktat.ruleset.utils.toUpperSnakeCase
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.CATCH
import com.pinterest.ktlint.core.ast.ElementType.CATCH_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.CLASS
import com.pinterest.ktlint.core.ast.ElementType.DESTRUCTURING_DECLARATION
import com.pinterest.ktlint.core.ast.ElementType.DESTRUCTURING_DECLARATION_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.FILE
import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_TYPE
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
import com.pinterest.ktlint.core.ast.ElementType.KDOC
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.TYPE_PARAMETER
import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE
Expand All @@ -60,9 +65,14 @@ import org.jetbrains.kotlin.builtins.PrimitiveType
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.tree.TokenSet
import org.jetbrains.kotlin.kdoc.parser.KDocKnownTag
import org.jetbrains.kotlin.psi.KtParameter
import org.jetbrains.kotlin.psi.KtPrimaryConstructor
import org.jetbrains.kotlin.psi.KtProperty
import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
import org.jetbrains.kotlin.psi.psiUtil.isPrivate
import org.jetbrains.kotlin.psi.psiUtil.parents

import java.util.Locale

/**
Expand Down Expand Up @@ -156,14 +166,19 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
@Suppress(
"SAY_NO_TO_VAR",
"TOO_LONG_FUNCTION",
"ComplexMethod"
"ComplexMethod",
"UnsafeCallOnNullableType",
)
private fun checkVariableName(node: ASTNode): List<ASTNode> {
// special case for Destructuring declarations that can be treated as parameters in lambda:
var namesOfVariables = extractVariableIdentifiers(node)
// Only local private properties will be autofix in order not to break code if there are usages in other files.
// Destructuring declarations are only allowed for local variables/values, so we don't need to calculate `isFix` for every node in `namesOfVariables`
val isFix = isFixMode && if (node.elementType == ElementType.PROPERTY) (node.psi as KtProperty).run { isLocal || isPrivate() } else true
val isPublicOrNonLocalProperty = if (node.elementType == ElementType.PROPERTY) (node.psi as KtProperty).run { !isLocal && !isPrivate() } else false
val isNonPrivatePrimaryConstructorParameter = (node.psi as? KtParameter)?.run {
hasValOrVar() && getParentOfType<KtPrimaryConstructor>(true)?.valueParameters?.contains(this) == true && !isPrivate()
} ?: false
val isFix = isFixMode && !(isPublicOrNonLocalProperty || isNonPrivatePrimaryConstructorParameter)
namesOfVariables
.forEach { variableName ->
// variable should not contain only one letter in it's name. This is a bad example: b512
Expand Down Expand Up @@ -195,6 +210,20 @@ class IdentifierNaming(configRules: List<RulesConfig>) : DiktatRule(
?.findAllVariablesWithUsages { it.name == variableName.text }
?.flatMap { it.value.toList() }
?.forEach { (it.node.firstChildNode as LeafPsiElement).rawReplaceWithText(correctVariableName) }
if (variableName.treeParent.psi.run {
(this is KtProperty && isMember) ||
(this is KtParameter && getParentOfType<KtPrimaryConstructor>(true)?.valueParameters?.contains(this) == true)
}) {
// For class members also check `@property` KDoc tag.
// If we are here, then `variableName` is definitely a node from a class.
variableName.parent(CLASS)!!.findChildByType(KDOC)?.kDocTags()
?.firstOrNull {
it.knownTag == KDocKnownTag.PROPERTY && it.getSubjectName() == variableName.text
}
?.run {
(getSubjectLink()!!.node.findAllDescendantsWithSpecificType(IDENTIFIER).single() as LeafPsiElement).rawReplaceWithText(correctVariableName)
}
}
(variableName as LeafPsiElement).rawReplaceWithText(correctVariableName)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import org.jetbrains.kotlin.kdoc.psi.impl.KDocTag
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCatchClause
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.kotlin.psi.KtThrowExpression
import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
import org.jetbrains.kotlin.psi.psiUtil.referenceExpression
Expand Down Expand Up @@ -118,7 +119,7 @@ class KdocMethods(configRules: List<RulesConfig>) : DiktatRule(
// if no tag failed, we have too little information to suggest KDoc - it would just be empty
if (kdoc == null && anyTagFailed) {
addKdocTemplate(node, name, missingParameters, explicitlyThrownExceptions, returnCheckFailed)
} else if (kdoc == null) {
} else if (kdoc == null && !isReferenceExpressionWithSameName(node, kdocTags)) {
MISSING_KDOC_ON_FUNCTION.warn(configRules, emitWarn, false, name, node.startOffset, node)
} else {
if (paramCheckFailed) {
Expand Down Expand Up @@ -146,6 +147,14 @@ class KdocMethods(configRules: List<RulesConfig>) : DiktatRule(
}
}

private fun isReferenceExpressionWithSameName(node: ASTNode, kdocTags: Collection<KDocTag>?): Boolean {
val lastDotQualifiedExpression = node.findChildByType(DOT_QUALIFIED_EXPRESSION)?.psi
?.let { (it as KtDotQualifiedExpression).selectorExpression?.text?.substringBefore('(') }
val funName = (node.psi as KtFunction).name
return funName == lastDotQualifiedExpression
}

@Suppress("WRONG_NEWLINES")
private fun hasReturnCheckFailed(node: ASTNode, kdocTags: Collection<KDocTag>?): Boolean {
if (node.isSingleLineGetterOrSetter()) {
return false
Expand All @@ -156,8 +165,10 @@ class KdocMethods(configRules: List<RulesConfig>) : DiktatRule(
val hasExplicitNotUnitReturnType = explicitReturnType != null && explicitReturnType.text != "Unit"
val hasExplicitUnitReturnType = explicitReturnType != null && explicitReturnType.text == "Unit"
val isFunWithExpressionBody = node.hasChildOfType(EQ)
val isReferenceExpressionWithSameName = node.findAllDescendantsWithSpecificType(REFERENCE_EXPRESSION).map { it.text }.contains((node.psi as KtFunction).name)
val hasReturnKdoc = kdocTags != null && kdocTags.hasKnownKdocTag(KDocKnownTag.RETURN)
return (hasExplicitNotUnitReturnType || isFunWithExpressionBody && !hasExplicitUnitReturnType && hasNotExpressionBodyTypes) && !hasReturnKdoc
return (hasExplicitNotUnitReturnType || isFunWithExpressionBody && !hasExplicitUnitReturnType && hasNotExpressionBodyTypes)
&& !hasReturnKdoc && !isReferenceExpressionWithSameName
}

private fun getExplicitlyThrownExceptions(node: ASTNode): Set<String> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import com.pinterest.ktlint.core.ast.ElementType.BLOCK
import com.pinterest.ktlint.core.ast.ElementType.CLASS
import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
import com.pinterest.ktlint.core.ast.ElementType.CLASS_INITIALIZER
import com.pinterest.ktlint.core.ast.ElementType.DATA_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.ENUM_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.FUN
import com.pinterest.ktlint.core.ast.ElementType.INNER_KEYWORD
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,10 @@ class IdentifierNamingFixTest : FixTestBase(
fun `typeAlias name incorrect`() {
fixAndCompare("identifiers/TypeAliasNameExpected.kt", "identifiers/TypeAliasNameTest.kt")
}

@Test
@Tag(WarningNames.TYPEALIAS_NAME_INCORRECT_CASE)
fun `should update property name in KDoc after fixing`() {
fixAndCompare("identifiers/PropertyInKdocExpected.kt", "identifiers/PropertyInKdocTest.kt")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,17 @@ class KdocMethodsTest : LintTestBase(::KdocMethods) {
)
}

@Test
@Tag(WarningNames.MISSING_KDOC_ON_FUNCTION)
fun `KDoc shouldn't be for function with name as method`() {
lintMethod(
"""
|@GetMapping("/projects")
|fun getProjects() = projectService.getProjects(x.prop())
""".trimMargin(),
)
}

@Test
@Tag(WarningNames.MISSING_KDOC_ON_FUNCTION)
fun `KDoc shouldn't trigger on actual methods`() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package test.paragraph1.naming.identifiers

/**
* @property anAbcMember
* @property another_abc_member
* @property anDefMember
* @property anotherDefMember
*/
data class Abc(
private val anAbcMember: String,
val another_abc_member: String,
) {
private val anDefMember: String = ""
private val anotherDefMember: String = ""
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package test.paragraph1.naming.identifiers

/**
* @property an_abc_member
* @property another_abc_member
* @property an_def_member
* @property another_def_member
*/
data class abc(
private val an_abc_member: String,
val another_abc_member: String,
) {
private val an_def_member: String = ""
private val another_def_member: String = ""
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,7 @@ fun hasNoChildren() = children.size == 0
* @return
*/
fun getFirstChild() = children.elementAtOrNull(0)

@GetMapping("/projects")
fun getProjects() = projectService.getProjects()
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ class Example {
fun hasNoChildren() = children.size == 0

fun getFirstChild() = children.elementAtOrNull(0)

@GetMapping("/projects")
fun getProjects() = projectService.getProjects()
}

0 comments on commit ce196cf

Please sign in to comment.