Skip to content

Commit

Permalink
Merge branch 'master' into feature/inline-classes(#698)
Browse files Browse the repository at this point in the history
# Conflicts:
#	diktat-rules/src/main/kotlin/generated/WarningNames.kt
  • Loading branch information
aktsay6 committed Jan 21, 2021
2 parents 79d7a2c + 66fc684 commit edc0fa9
Show file tree
Hide file tree
Showing 45 changed files with 476 additions and 83 deletions.
3 changes: 3 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,9 @@
enabled: true
configuration:
maxLambdaLength: 10 # max length of lambda without parameters
# Checks that using unnecessary, custom label
- name: CUSTOM_LABEL
enabled: true
# Checks that property in constructor doesn't contains comment
- name: KDOC_NO_CONSTRUCTOR_PROPERTY
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ data class CommonConfiguration(private val configuration: Map<String, String>?)
/**
* Start of package name, which shoould be common, e.g. org.example.myproject
*/
val domainName: String by lazy {
(configuration ?: emptyMap()).getOrDefault("domainName", "")
val domainName: String? by lazy {
configuration?.get("domainName")
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ open class DiktatExtension {

/**
* Path to diktat yml config file. Can be either absolute or relative to project's root directory.
* Default value: `diktat-analysis.yml` in rootDir.
*/
var diktatConfigFile: File = File("diktat-analysis.yml")
lateinit var diktatConfigFile: File

/**
* Paths that will be excluded from diktat run
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class DiktatGradlePlugin : Plugin<Project> {
inputs = project.fileTree("src").apply {
include("**/*.kt")
}
reporter = PlainReporter(System.out)
diktatConfigFile = project.rootProject.file("diktat-analysis.yml")
excludes = project.files()
reporter = PlainReporter(System.out)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,14 @@ open class DiktatJavaExecTaskBase @Inject constructor(
add((if (negate) "!" else "") + path)
}

/**
* todo: share logic with maven plugin, it's basically copy-paste
*/
@Suppress("ForbiddenComment")
private fun resolveConfigFile(file: File): String {
if (file.isAbsolute) {
if (file.toPath().startsWith(project.rootDir.toPath())) {
// In gradle, project.files() returns File relative to project.projectDir.
// There is no need to resolve file further if it has been passed via gradle files API.
return file.absolutePath
}

// otherwise, e.g. if file is passed as java.io.File with relative path, we try to find it
return generateSequence(project.projectDir) { it.parentFile }
.map { it.resolve(file) }
.run {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
package org.cqfn.diktat.plugin.gradle

import org.cqfn.diktat.ruleset.rules.DIKTAT_CONF_PROPERTY
import org.gradle.api.Project
import org.gradle.testfixtures.ProjectBuilder
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import java.io.File
import java.nio.file.Files

class DiktatJavaExecTaskTest {
private val projectBuilder = ProjectBuilder.builder()
private lateinit var project: Project

@BeforeEach
fun setUp() {
project = projectBuilder.build()
project = projectBuilder
.withName("testProject")
.build()
}

@Test
Expand Down Expand Up @@ -49,22 +53,102 @@ class DiktatJavaExecTaskTest {

@Test
fun `check command line with non-existent inputs`() {
val task = registerDiktatTask {
val task = project.registerDiktatTask {
inputs = project.files()
}
Assertions.assertFalse(task.shouldRun)
}

private fun registerDiktatTask(extensionConfiguration: DiktatExtension.() -> Unit): DiktatJavaExecTaskBase {
DiktatGradlePlugin().apply(project)
project.extensions.configure("diktat", extensionConfiguration)
return project.tasks.getByName("diktatCheck") as DiktatJavaExecTaskBase
@Test
fun `check system property with default config`() {
val task = project.registerDiktatTask {
inputs = project.files()
}
Assertions.assertEquals(File(project.projectDir, "diktat-analysis.yml").absolutePath, task.systemProperties[DIKTAT_CONF_PROPERTY])
}

@Test
fun `check system property with custom config`() {
val task = project.registerDiktatTask {
inputs = project.files()
diktatConfigFile = project.file("../diktat-analysis.yml")
}
Assertions.assertEquals(File(project.projectDir.parentFile, "diktat-analysis.yml").absolutePath, task.systemProperties[DIKTAT_CONF_PROPERTY])
}

@Test
fun `check system property with multiproject build with default config`() {
setupMultiProject()
val subproject = project.subprojects.first()
project.allprojects {
it.registerDiktatTask {
inputs = it.files()
}
}
val task = project.tasks.getByName(DIKTAT_CHECK_TASK) as DiktatJavaExecTaskBase
val subprojectTask = subproject.tasks.getByName(DIKTAT_CHECK_TASK) as DiktatJavaExecTaskBase
Assertions.assertEquals(File(project.projectDir, "diktat-analysis.yml").absolutePath, task.systemProperties[DIKTAT_CONF_PROPERTY])
Assertions.assertEquals(File(project.projectDir, "diktat-analysis.yml").absolutePath, subprojectTask.systemProperties[DIKTAT_CONF_PROPERTY])
}

@Test
fun `check system property with multiproject build with custom config`() {
setupMultiProject()
val subproject = project.subprojects.first()
project.allprojects {
it.registerDiktatTask {
inputs = it.files()
diktatConfigFile = it.rootProject.file("diktat-analysis.yml")
}
}
val task = project.tasks.getByName(DIKTAT_CHECK_TASK) as DiktatJavaExecTaskBase
val subprojectTask = subproject.tasks.getByName(DIKTAT_CHECK_TASK) as DiktatJavaExecTaskBase
Assertions.assertEquals(File(project.projectDir, "diktat-analysis.yml").absolutePath, task.systemProperties[DIKTAT_CONF_PROPERTY])
Assertions.assertEquals(File(project.projectDir, "diktat-analysis.yml").absolutePath, subprojectTask.systemProperties[DIKTAT_CONF_PROPERTY])
}

@Test
fun `check system property with multiproject build with custom config and two config files`() {
setupMultiProject()
val subproject = project.subprojects.single()
subproject.file("diktat-analysis.yml").createNewFile()
project.allprojects {
it.registerDiktatTask {
inputs = it.files()
diktatConfigFile = it.file("diktat-analysis.yml")
}
}
val task = project.tasks.getByName(DIKTAT_CHECK_TASK) as DiktatJavaExecTaskBase
val subprojectTask = subproject.tasks.getByName(DIKTAT_CHECK_TASK) as DiktatJavaExecTaskBase
Assertions.assertEquals(File(project.projectDir, "diktat-analysis.yml").absolutePath, task.systemProperties[DIKTAT_CONF_PROPERTY])
Assertions.assertEquals(File(subproject.projectDir, "diktat-analysis.yml").absolutePath, subprojectTask.systemProperties[DIKTAT_CONF_PROPERTY])
}

private fun Project.registerDiktatTask(extensionConfiguration: DiktatExtension.() -> Unit): DiktatJavaExecTaskBase {
DiktatGradlePlugin().apply(this)
extensions.configure("diktat", extensionConfiguration)
return tasks.getByName(DIKTAT_CHECK_TASK) as DiktatJavaExecTaskBase
}

private fun assertCommandLineEquals(expected: List<String?>, extensionConfiguration: DiktatExtension.() -> Unit) {
val task = registerDiktatTask(extensionConfiguration)
val task = project.registerDiktatTask(extensionConfiguration)
Assertions.assertIterableEquals(expected, task.commandLine)
}

private fun setupMultiProject() {
ProjectBuilder.builder()
.withParent(project)
.withName("testSubproject")
.withProjectDir(File(project.projectDir, "testSubproject").also {
Files.createDirectory(it.toPath())
})
.build()
project.file("diktat-analysis.yml").createNewFile()
}

private fun combinePathParts(vararg parts: String) = parts.joinToString(File.separator)

companion object {
private const val DIKTAT_CHECK_TASK = "diktatCheck"
}
}
2 changes: 2 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ public object WarningNames {

public const val TOO_MANY_LINES_IN_LAMBDA: String = "TOO_MANY_LINES_IN_LAMBDA"

public const val CUSTOM_LABEL: String = "CUSTOM_LABEL"

public const val SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY: String =
"SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ enum class Warnings(
WRONG_OVERLOADING_FUNCTION_ARGUMENTS(false, "5.2.3", "use default argument instead of function overloading"),
RUN_BLOCKING_INSIDE_ASYNC(false, "5.2.4", "avoid using runBlocking in asynchronous code"),
TOO_MANY_LINES_IN_LAMBDA(false, "5.2.5", "long lambdas should have a parameter name instead of it"),
CUSTOM_LABEL(false, "5.2.6", "avoid using expression with custom label"),

// ======== chapter 6 ========
SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY(true, "6.1.1", "if a class has single constructor, it should be converted to a primary constructor"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class AsyncAndSyncRule(private val configRules: List<RulesConfig>) : Rule("sync-
}

private fun checkRunBlocking(node: ASTNode) {
node.parent({it.isAsync() || it.isSuspend()})?.let {
node.parent({ it.isAsync() || it.isSuspend() })?.let {
RUN_BLOCKING_INSIDE_ASYNC.warn(configRules, emitWarn, isFixMode, node.text, node.startOffset, node)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class BlockStructureBraces(private val configRules: List<RulesConfig>) : Rule("b
FUN, CLASS_INITIALIZER, SECONDARY_CONSTRUCTOR -> checkFun(node, configuration)
IF -> checkIf(node, configuration)
WHEN -> checkWhen(node, configuration)
FOR, WHILE, DO_WHILE -> checkLoop(node, configuration)
in loopType -> checkLoop(node, configuration)
TRY -> checkTry(node, configuration)
else -> return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,18 @@ import org.cqfn.diktat.ruleset.constants.EmitType
import org.cqfn.diktat.ruleset.constants.Warnings.NO_BRACES_IN_CONDITIONALS_AND_LOOPS
import org.cqfn.diktat.ruleset.utils.findChildrenMatching
import org.cqfn.diktat.ruleset.utils.isSingleLineIfElse
import org.cqfn.diktat.ruleset.utils.loopType

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.BLOCK
import com.pinterest.ktlint.core.ast.ElementType.DO_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.DO_WHILE
import com.pinterest.ktlint.core.ast.ElementType.ELSE_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.FOR
import com.pinterest.ktlint.core.ast.ElementType.IF
import com.pinterest.ktlint.core.ast.ElementType.IF_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.LBRACE
import com.pinterest.ktlint.core.ast.ElementType.RBRACE
import com.pinterest.ktlint.core.ast.ElementType.WHEN
import com.pinterest.ktlint.core.ast.ElementType.WHILE
import com.pinterest.ktlint.core.ast.ElementType.WHILE_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.isPartOfComment
Expand Down Expand Up @@ -50,7 +48,7 @@ class BracesInConditionalsAndLoopsRule(private val configRules: List<RulesConfig
when (node.elementType) {
IF -> checkIfNode(node)
WHEN -> checkWhenBranches(node)
FOR, WHILE, DO_WHILE -> checkLoop(node)
in loopType -> checkLoop(node)
else -> return
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class CheckInverseMethodRule(private val configRules: List<RulesConfig>) : Rule(
.treeParent
.siblings(forward = false)
.takeWhile { it.elementType in intermediateTokens }
.firstOrNull { it.elementType == OPERATION_REFERENCE}
.firstOrNull { it.elementType == OPERATION_REFERENCE }
if (operationRef?.text == "!") {
INVERSE_FUNCTION_PREFERRED.warnAndFix(configRules, emitWarn, isFixMode, "${methodMap[node.text]} instead of !${node.text}", node.startOffset, node) {
val callExpression = CompositeElement(CALL_EXPRESSION)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class ClassLikeStructuresOrderRule(private val configRules: List<RulesConfig>) :
unusedClasses)
.allBlockFlattened()
.map { astNode ->
Pair(astNode, astNode.siblings(false).takeWhile { it.elementType == WHITE_SPACE || it.isPartOfComment()}.toList())
Pair(astNode, astNode.siblings(false).takeWhile { it.elementType == WHITE_SPACE || it.isPartOfComment() }.toList())
}

val classChildren = node.children().filter { it.elementType in childrenTypes }.toList()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.cqfn.diktat.ruleset.rules

import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.constants.EmitType
import org.cqfn.diktat.ruleset.constants.Warnings.CUSTOM_LABEL
import org.cqfn.diktat.ruleset.utils.loopType

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType.BREAK
import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.CONTINUE
import com.pinterest.ktlint.core.ast.ElementType.LABEL_QUALIFIER
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.RETURN
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.psi.psiUtil.parents

/**
* Rule that checks using custom label
*/
class CustomLabel(private val configRules: List<RulesConfig>) : Rule("custom-label") {
private var isFixMode: Boolean = false
private val forEachReference = listOf("forEach", "forEachIndexed")
private val labels = listOf("@loop", "@forEach", "@forEachIndexed")
private val stopWords = listOf(RETURN, BREAK, CONTINUE)
private lateinit var emitWarn: EmitType

override fun visit(node: ASTNode,
autoCorrect: Boolean,
emit: EmitType) {
emitWarn = emit
isFixMode = autoCorrect

if (node.elementType == LABEL_QUALIFIER && node.text !in labels && node.treeParent.elementType in stopWords) {
val nestedCount = node.parents().count {
it.elementType in loopType ||
(it.elementType == CALL_EXPRESSION && it.findChildByType(REFERENCE_EXPRESSION)?.text in forEachReference)
}
if (nestedCount == 1) {
CUSTOM_LABEL.warn(configRules, emitWarn, isFixMode, node.text, node.startOffset, node)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
::PropertyAccessorFields,
::AbstractClassesRule,
::SingleInitRule,
::CustomLabel,
::VariableGenericTypeDeclarationRule,
::LongNumericalValuesSeparatedRule,
::NestedFunctionBlock,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,10 @@ class IdentifierNaming(private val configRules: List<RulesConfig>) : Rule("ident
// FixMe: cover fixes with tests
val correctVariableName = variableName.text.toLowerCamelCase()
variableName
.parent({it.elementType == FILE})
.parent({ it.elementType == FILE })
?.findAllVariablesWithUsages { it.name == variableName.text }
?.flatMap { it.value.toList() }
?.forEach { (it.node.firstChildNode as LeafPsiElement).replaceWithText(correctVariableName)}
?.forEach { (it.node.firstChildNode as LeafPsiElement).replaceWithText(correctVariableName) }
(variableName as LeafPsiElement).replaceWithText(correctVariableName)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class LambdaLengthRule(private val configRules: List<RulesConfig>) : Rule("lambd
node.treeParent.removeChild(node)
}
}
val isIt = copyNode.findAllNodesWithSpecificType(ElementType.REFERENCE_EXPRESSION).map {re -> re.text}.contains("it")
val isIt = copyNode.findAllNodesWithSpecificType(ElementType.REFERENCE_EXPRESSION).map { re -> re.text }.contains("it")
val parameters = node.findChildByType(ElementType.FUNCTION_LITERAL)?.findChildByType(ElementType.VALUE_PARAMETER_LIST)
if (parameters == null && isIt) {
Warnings.TOO_MANY_LINES_IN_LAMBDA.warn(configRules, emitWarn, isFixMode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class LambdaParameterOrder(private val configRules: List<RulesConfig>) : Rule("l
private fun checkArguments(node: ASTNode) {
val funArguments = (node.psi as KtFunction).valueParameters
val sortArguments = funArguments.sortedBy { it.typeReference?.node?.hasChildOfType(FUNCTION_TYPE) }
funArguments.filterIndexed {index, ktParameter -> ktParameter != sortArguments[index] }.ifNotEmpty {
funArguments.filterIndexed { index, ktParameter -> ktParameter != sortArguments[index] }.ifNotEmpty {
LAMBDA_IS_NOT_LAST_PARAMETER.warn(configRules, emitWarn, isFixMode, node.findChildByType(IDENTIFIER)!!.text,
first().node.startOffset, node)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class OverloadingArgumentsFunction(private val configRules: List<RulesConfig>) :
.map { it.psi as KtFunction }
.filter { it.nameIdentifier!!.text == funPsi.nameIdentifier!!.text && it.valueParameters.containsAll(funPsi.valueParameters) }
.filter { funPsi.node.findChildBefore(IDENTIFIER, TYPE_REFERENCE)?.text == it.node.findChildBefore(IDENTIFIER, TYPE_REFERENCE)?.text }
.filter {funPsi.node.findChildAfter(IDENTIFIER, TYPE_REFERENCE)?.text == it.node.findChildAfter(IDENTIFIER, TYPE_REFERENCE)?.text }
.filter { funPsi.node.findChildAfter(IDENTIFIER, TYPE_REFERENCE)?.text == it.node.findChildAfter(IDENTIFIER, TYPE_REFERENCE)?.text }
.toList()
if (allOverloadFunction.isNotEmpty()) {
WRONG_OVERLOADING_FUNCTION_ARGUMENTS.warn(configRules, emitWarn, isFixMode, funPsi.node.findChildByType(IDENTIFIER)!!.text, funPsi.startOffset, funPsi.node)
Expand Down
Loading

0 comments on commit edc0fa9

Please sign in to comment.