From 0664e2a57eaeb201899c6caa87928eb31ceaec21 Mon Sep 17 00:00:00 2001 From: akuleshov7 Date: Mon, 5 Oct 2020 03:31:30 +0300 Subject: [PATCH 1/4] Improving variable search mechanism ### What's done: 1) Adding mechanism for searching assignments of VARs 2) Moving all usages to new utilities of searching variables --- .../ruleset/rules/ImmutableValNoVarRule.kt | 48 +++++--- .../diktat/ruleset/rules/files/FileSize.kt | 1 + .../ruleset/rules/files/FileStructureRule.kt | 1 + .../rules/identifiers/LocalVariablesRule.kt | 22 ++-- .../cqfn/diktat/ruleset/utils/AstNodeUtils.kt | 2 - .../ruleset/utils/VariableSearchASTUtils.kt | 108 ------------------ .../ruleset/utils/search/VariablesSearch.kt | 95 +++++++++++++++ .../search/VariablesWithAssignmentSearch.kt | 39 +++++++ .../utils/search/VariablesWithUsagesSearch.kt | 32 ++++++ .../ruleset/chapter4/NoVarRuleWarnTest.kt | 1 + .../VariablesWithAssignmentsSearchTest.kt | 79 +++++++++++++ ...st.kt => VariablesWithUsagesSearchTest.kt} | 35 +++--- 12 files changed, 303 insertions(+), 160 deletions(-) delete mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/VariableSearchASTUtils.kt create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithAssignmentsSearchTest.kt rename diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/{VariableSearchASTUtilsTest.kt => VariablesWithUsagesSearchTest.kt} (88%) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImmutableValNoVarRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImmutableValNoVarRule.kt index e169d74e7c..71d7fc4506 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImmutableValNoVarRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/ImmutableValNoVarRule.kt @@ -4,13 +4,12 @@ import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.ruleset.constants.Warnings.SAY_NO_TO_VAR -import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType -import org.cqfn.diktat.ruleset.utils.getAllUsages +import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithAssignments +import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.psi.KtBlockExpression import org.jetbrains.kotlin.psi.KtLambdaExpression import org.jetbrains.kotlin.psi.KtLoopExpression -import org.jetbrains.kotlin.psi.KtProperty import org.jetbrains.kotlin.psi.psiUtil.getParentOfType /** @@ -29,22 +28,33 @@ class ImmutableValNoVarRule(private val configRules: List) : Rule(" emitWarn = emit isFixMode = autoCorrect - if (node.elementType == ElementType.FUN) { - node.findAllNodesWithSpecificType(ElementType.PROPERTY) - .map { it.psi as KtProperty } - // we can force to be immutable only variables that are from local context (not from class and not from file-level) - .filter { it.isLocal && it.name != null && it.parent is KtBlockExpression } - .filter { it.isVar } - .forEach { property -> - val usedInAccumulators = property.getAllUsages().any { - it.getParentOfType(true) != null || - it.getParentOfType(true) != null - } - - if (!usedInAccumulators) { - SAY_NO_TO_VAR.warn(configRules, emitWarn, isFixMode, property.text, property.node.startOffset, property.node) - } - } + if (node.elementType == ElementType.FILE) { + // we will raise warning for cases when var property has no assignments + val varNoAssignments = node + .findAllVariablesWithAssignments { it.name != null && it.isVar } + .filter { it.value.isEmpty() } + + varNoAssignments.forEach { (property, usages) -> + // FixMe: raise another warning and fix the code (change to val) for variables without assignment + } + + // we can force to be immutable only variables that are from local context (not from class and not from file-level) + val usages = node + .findAllVariablesWithUsages { it.isLocal && it.name != null && it.parent is KtBlockExpression && it.isVar } + .filter { !varNoAssignments.containsKey(it.key) } + + usages.forEach { (property, usages) -> + val usedInAccumulators = usages.any { + it.getParentOfType(true) != null || + it.getParentOfType(true) != null + } + + if (!usedInAccumulators) { + SAY_NO_TO_VAR.warn(configRules, emitWarn, isFixMode, property.text, property.node.startOffset, property.node) + } + } + + return } } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileSize.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileSize.kt index 3575e3e82a..939ba7c1cc 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileSize.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileSize.kt @@ -45,6 +45,7 @@ class FileSize(private val configRules: List) : Rule("file-size") { checkFileSize(node, configuration.maxSize) } } + return } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt index 1d507d2c69..86f0661542 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt @@ -59,6 +59,7 @@ class FileStructureRule(private val configRules: List) : Rule("file if (checkFileHasCode(node)) { checkCodeBlocksOrderAndEmptyLines(node) } + return } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/identifiers/LocalVariablesRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/identifiers/LocalVariablesRule.kt index 1ad3c50690..6a105e2058 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/identifiers/LocalVariablesRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/identifiers/LocalVariablesRule.kt @@ -2,16 +2,14 @@ package org.cqfn.diktat.ruleset.rules.identifiers import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType.FILE -import com.pinterest.ktlint.core.ast.ElementType.PROPERTY import com.pinterest.ktlint.core.ast.isPartOfComment import com.pinterest.ktlint.core.ast.lineNumber import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.ruleset.constants.Warnings.LOCAL_VARIABLE_EARLY_DECLARATION import org.cqfn.diktat.ruleset.utils.containsOnlyConstants -import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType -import org.cqfn.diktat.ruleset.utils.getAllUsages import org.cqfn.diktat.ruleset.utils.getDeclarationScope import org.cqfn.diktat.ruleset.utils.lastLineNumber +import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace @@ -57,7 +55,7 @@ class LocalVariablesRule(private val configRules: List) : Rule("loc if (node.elementType == FILE) { // collect all local properties and associate with corresponding references - val propertiesToUsages = collectPropertiesWithUsages(node) + val propertiesToUsages = collectLocalPropertiesWithUsages(node) // find all usages which include more than one property val multiPropertyUsages = groupPropertiesByUsages(propertiesToUsages) @@ -73,16 +71,14 @@ class LocalVariablesRule(private val configRules: List) : Rule("loc } } - private fun collectPropertiesWithUsages(node: ASTNode) = node - .findAllNodesWithSpecificType(PROPERTY) - .map { it.psi as KtProperty } - .filter { it.isLocal && it.name != null && it.parent is KtBlockExpression } - .filter { - it.isVar && it.initializer == null || - (it.initializer?.containsOnlyConstants() ?: false) || - (it.initializer as? KtCallExpression).isWhitelistedMethod() + private fun collectLocalPropertiesWithUsages(node: ASTNode) = node + .findAllVariablesWithUsages { propertyNode -> + propertyNode.isLocal && propertyNode.name != null && propertyNode.parent is KtBlockExpression && + (propertyNode.isVar && propertyNode.initializer == null || + (propertyNode.initializer?.containsOnlyConstants() ?: false) || + (propertyNode.initializer as? KtCallExpression).isWhitelistedMethod()) } - .associateWith { it.getAllUsages() } + .filterNot { it.value.isEmpty() } private fun groupPropertiesByUsages(propertiesToUsages: Map>) = propertiesToUsages diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt index 256f4a676b..123ee50334 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt @@ -138,7 +138,6 @@ fun ASTNode.findChildBefore(beforeThisNodeType: IElementType, childNodeType: IEl .find { it.elementType == childNodeType } ?.let { return it } - log.warn("Not able to find a node with type $childNodeType before $beforeThisNodeType") return null } @@ -160,7 +159,6 @@ fun ASTNode.findChildAfter(afterThisNodeType: IElementType, childNodeType: IElem } } - log.warn("Not able to find a node with type $childNodeType after $afterThisNodeType") return null } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/VariableSearchASTUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/VariableSearchASTUtils.kt deleted file mode 100644 index 4dc7be6e7c..0000000000 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/VariableSearchASTUtils.kt +++ /dev/null @@ -1,108 +0,0 @@ -package org.cqfn.diktat.ruleset.utils - -import com.pinterest.ktlint.core.ast.ElementType -import org.jetbrains.kotlin.com.intellij.lang.ASTNode -import org.jetbrains.kotlin.psi.KtClassBody -import org.jetbrains.kotlin.psi.KtDotQualifiedExpression -import org.jetbrains.kotlin.psi.KtNameReferenceExpression -import org.jetbrains.kotlin.psi.KtProperty -import org.jetbrains.kotlin.psi.KtFile -import org.jetbrains.kotlin.psi.KtBlockExpression -import org.jetbrains.kotlin.psi.KtElement -import org.jetbrains.kotlin.psi.KtFunctionLiteral -import org.jetbrains.kotlin.psi.psiUtil.getChildrenOfType -import org.jetbrains.kotlin.psi.psiUtil.getParentOfType -import org.jetbrains.kotlin.psi.psiUtil.parents -import org.jetbrains.kotlin.psi.psiUtil.referenceExpression - -/** - * [this] - root node of a type File that is used to search all declared properties (variables) - * and it's usages (rvalues). - * (!) ONLY for nodes of File elementType - * - * @return a map of a property to it's usages - */ -fun ASTNode.collectAllDeclaredVariablesWithUsages(): Map> { - require(this.elementType == ElementType.FILE) { - "To collect all variables in a file you need to provide file root node" - } - - return this - .findAllNodesWithSpecificType(ElementType.PROPERTY) - .map { it.psi as KtProperty } - .associateWith { it.getAllUsages() } -} - -/** - * Finds all references to [this] in the same code block. - * [this] - usages of this property will be searched - * @return list of references as [KtNameReferenceExpression] - */ -@Suppress("UnsafeCallOnNullableType") -fun KtProperty.getAllUsages(): List { - return this - .getDeclarationScope() - // if declaration scope is not null - then we have found out the block where this variable is stored - // else - it is a global variable on a file level or a property on the class level - .let { declarationScope -> - // searching in the scope with declaration (in the context) - declarationScope?.getAllUsagesOfProperty(this) - // searching on the class level in class body - ?: (this.getParentOfType(true)?.getAllUsagesOfProperty(this)) - // searching on the file level - ?: (this.getParentOfType(true)!!.getAllUsagesOfProperty(this)) - } -} - -/** - * getting all usages of a variable inside the same (or nested) block (where variable was declared) - */ -private fun KtElement.getAllUsagesOfProperty(property: KtProperty) = - this.node.findAllNodesWithSpecificType(ElementType.REFERENCE_EXPRESSION) - // filtering out all usages that are declared in the same context but are going before the variable declaration - .filter { it.isGoingAfter(property.node) } - .map { it.psi as KtNameReferenceExpression } - .filter { it.getReferencedNameAsName() == property.nameAsName } - .filterNot { expression -> - // to avoid false triggering on objects' fields with same name as property - isReferenceToFieldOfObject(expression) || - // to exclude usages of local properties from other context (shadowed) and lambda arguments with same name - isReferenceToOtherVariableWithSameName(expression, this, property) - } - -/** - * filtering object's fields (expressions) that have same name as variable - */ -private fun isReferenceToFieldOfObject(expression: KtNameReferenceExpression) = - (expression.parent as? KtDotQualifiedExpression)?.run { - receiverExpression != expression && selectorExpression?.referenceExpression() == expression - } ?: false - - -/** - * filtering local properties from other context (shadowed) and lambda and function arguments with same name - * going through all parent scopes from bottom to top until we will find the scope where the initial variable was declared - * all these scopes are on lower level of inheritance that's why if in one of these scopes we will find any - * variable declaration with the same name - we will understand that it is usage of another variable -*/ -private fun isReferenceToOtherVariableWithSameName(expression: KtNameReferenceExpression, codeBlock: KtElement, property: KtProperty): Boolean { - return expression.parents - // getting all block expressions/class bodies/file node from bottom to the top - // FixMe: Object companion is not resolved properly yet - .filter { it is KtBlockExpression || it is KtClassBody || it is KtFile } - // until we reached the block that contains the initial declaration - .takeWhile { codeBlock != it } - .any { block -> - // this is not the expression that we needed if: - // 1) there is a new shadowed declaration for this expression (but the declaration should stay on the previous line!) - // 2) or there one of top blocks is a function/lambda that has arguments with the same name - // FixMe: in class or a file the declaration can easily go after the usage (by lines of code) - block.getChildrenOfType().any { it.nameAsName == property.nameAsName && expression.node.isGoingAfter(it.node) } || - block.parent - .let { it as? KtFunctionLiteral } - ?.valueParameters - ?.any { it.nameAsName == property.nameAsName } - ?: false - // FixMe: also see very strange behavior of Kotlin in tests (disabled) - } -} diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt new file mode 100644 index 0000000000..ba58fa6772 --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt @@ -0,0 +1,95 @@ +package org.cqfn.diktat.ruleset.utils.search + +import com.pinterest.ktlint.core.ast.ElementType +import org.cqfn.diktat.ruleset.utils.* +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getChildrenOfType +import org.jetbrains.kotlin.psi.psiUtil.getParentOfType +import org.jetbrains.kotlin.psi.psiUtil.parents +import org.jetbrains.kotlin.psi.psiUtil.referenceExpression + +/** + * @param node - root node of a type File that is used to search all declared properties (variables) + * it should be ONLY node of File elementType + * @param filterForVariables - condition to filter + */ +abstract class VariableSearch(val node: ASTNode, private val filterForVariables: (KtProperty) -> Boolean) { + + /** + * to complete implementation of a search mechanism you need to specify what and how you will search in current scope + * [this] - scope where to search the usages/assignments/e.t.c of the variable (can be of types KtBlockExpression/KtFile/KtClassBody) + */ + protected abstract fun KtElement.getAllSearchResults(property: KtProperty): List + + /** + * method collects all declared variables and it's usages + * + * @return a map of a property to it's usages + */ + fun collectVariables(): Map> { + require(node.elementType == ElementType.FILE) { + "To collect all variables in a file you need to provide file root node" + } + return node + .findAllNodesWithSpecificType(ElementType.PROPERTY) + .map { it.psi as KtProperty } + .filter(filterForVariables) + .associateWith { it.getSearchResults() } + } + + @Suppress("UnsafeCallOnNullableType") + fun KtProperty.getSearchResults(): List { + return this + .getDeclarationScope() + // if declaration scope is not null - then we have found out the block where this variable is stored + // else - it is a global variable on a file level or a property on the class level + .let { declarationScope -> + // searching in the scope with declaration (in the context) + declarationScope?.getAllSearchResults(this) + // searching on the class level in class body + ?: (this.getParentOfType(true)?.getAllSearchResults(this)) + // searching on the file level + ?: (this.getParentOfType(true)!!.getAllSearchResults(this)) + } + } + + /** + * filtering object's fields (expressions) that have same name as variable + */ + protected fun isReferenceToFieldOfObject(expression: KtNameReferenceExpression) = + (expression.parent as? KtDotQualifiedExpression)?.run { + receiverExpression != expression && selectorExpression?.referenceExpression() == expression + } ?: false + + /** + * filtering local properties from other context (shadowed) and lambda and function arguments with same name + * going through all parent scopes from bottom to top until we will find the scope where the initial variable was declared + * all these scopes are on lower level of inheritance that's why if in one of these scopes we will find any + * variable declaration with the same name - we will understand that it is usage of another variable + */ + protected fun isReferenceToOtherVariableWithSameName(expression: KtNameReferenceExpression, + codeBlock: KtElement, property: KtProperty): Boolean { + return expression.parents + // getting all block expressions/class bodies/file node from bottom to the top + // FixMe: Object companion is not resolved properly yet + .filter { it is KtBlockExpression || it is KtClassBody || it is KtFile } + // until we reached the block that contains the initial declaration + .takeWhile { codeBlock != it } + .any { block -> + // this is not the expression that we needed if: + // 1) there is a new shadowed declaration for this expression (but the declaration should stay on the previous line!) + // 2) or there one of top blocks is a function/lambda that has arguments with the same name + // FixMe: in class or a file the declaration can easily go after the usage (by lines of code) + block.getChildrenOfType().any { it.nameAsName == property.nameAsName && expression.node.isGoingAfter(it.node) } || + block.parent + .let { it as? KtFunctionLiteral } + ?.valueParameters + ?.any { it.nameAsName == property.nameAsName } + ?: false + // FixMe: also see very strange behavior of Kotlin in tests (disabled) + } + } +} + +fun default(node: KtProperty) = true \ No newline at end of file diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt new file mode 100644 index 0000000000..7eac2bcbd4 --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt @@ -0,0 +1,39 @@ +package org.cqfn.diktat.ruleset.utils.search + +import com.pinterest.ktlint.core.ast.ElementType +import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType +import org.cqfn.diktat.ruleset.utils.isGoingAfter +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.psi.KtBinaryExpression +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtProperty + +class VariablesWithAssignmentSearch(fileNode: ASTNode, + filterForVariables: (KtProperty) -> Boolean) : VariableSearch(fileNode, filterForVariables) { + + override fun KtElement.getAllSearchResults(property: KtProperty): List { + return this.node.findAllNodesWithSpecificType(ElementType.BINARY_EXPRESSION) + // filtering out all usages that are declared in the same context but are going before the variable declaration + .filter { + // FixMe: bug is here with a search of postfix/prefix variables assignment + // FixMe: also there can be some tricky cases with setters, but I am not able to imagine them now + it.isGoingAfter(property.node) && + (it.psi as KtBinaryExpression).operationToken == ElementType.EQ && + (it.psi as KtBinaryExpression).left!!.node.elementType == ElementType.REFERENCE_EXPRESSION + } + .map { (it.psi as KtBinaryExpression).left as KtNameReferenceExpression } + .filter { it.getReferencedNameAsName() == property.nameAsName } + .filterNot { expression -> + // to avoid false triggering on objects' fields with same name as property + isReferenceToFieldOfObject(expression) || + // to exclude usages of local properties from other context (shadowed) and lambda arguments with same name + isReferenceToOtherVariableWithSameName(expression, this, property) + } + .toList() + } +} + +// the default value for filtering condition is always true +fun ASTNode.findAllVariablesWithAssignments(filterForVariables: (KtProperty) -> Boolean = ::default) = + VariablesWithAssignmentSearch(this, filterForVariables).collectVariables() diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt new file mode 100644 index 0000000000..c85ef98f3a --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt @@ -0,0 +1,32 @@ +package org.cqfn.diktat.ruleset.utils.search + +import com.pinterest.ktlint.core.ast.ElementType +import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType +import org.cqfn.diktat.ruleset.utils.isGoingAfter +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtProperty + +class VariablesWithUsagesSearch(fileNode: ASTNode, + filterForVariables: (KtProperty) -> Boolean) : VariableSearch(fileNode, filterForVariables) { + + override fun KtElement.getAllSearchResults(property: KtProperty): List { + return this.node.findAllNodesWithSpecificType(ElementType.REFERENCE_EXPRESSION) + // filtering out all usages that are declared in the same context but are going before the variable declaration + .filter { it.isGoingAfter(property.node) } + .map { it.psi as KtNameReferenceExpression } + .filter { it.getReferencedNameAsName() == property.nameAsName } + .filterNot { expression -> + // to avoid false triggering on objects' fields with same name as property + isReferenceToFieldOfObject(expression) || + // to exclude usages of local properties from other context (shadowed) and lambda arguments with same name + isReferenceToOtherVariableWithSameName(expression, this, property) + } + } +} + +// the default value for filtering condition is always true +fun ASTNode.findAllVariablesWithUsages(filterForVariables: (KtProperty) -> Boolean = ::default) = + VariablesWithUsagesSearch(this, filterForVariables).collectVariables() + diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NoVarRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NoVarRuleWarnTest.kt index cf607b585b..69f16d4c43 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NoVarRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NoVarRuleWarnTest.kt @@ -35,6 +35,7 @@ class NoVarRuleWarnTest : LintTestBase(::ImmutableValNoVarRule) { """ | fun foo() { | var a = emptyList() + | a = 15 | var y = 0 | a.forEach { x -> | y = x + 1 diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithAssignmentsSearchTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithAssignmentsSearchTest.kt new file mode 100644 index 0000000000..dca992f238 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithAssignmentsSearchTest.kt @@ -0,0 +1,79 @@ +package org.cqfn.diktat.ruleset.utils + +import com.pinterest.ktlint.core.ast.ElementType.FILE +import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithAssignments +import org.cqfn.diktat.util.applyToCode +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Disabled +import org.junit.jupiter.api.Test + +@Suppress("UnsafeCallOnNullableType") +class VariablesWithAssignmentsSearchTest { + @Test + fun `testing proper variables search in function`() { + applyToCode(""" + fun foo(a: Int) { + fun foo1() { + var o = 1 + b = o + c = o + o = 15 + o = 17 + } + } + """.trimIndent(), 0) { node, counter -> + if (node.elementType == FILE) { + val vars = node.findAllVariablesWithAssignments().mapKeys { it.key.text } + val keys = vars.keys + val var1 = keys.elementAt(0) + Assertions.assertEquals("var o = 1", var1) + Assertions.assertEquals(2, vars[var1]?.size) + } + } + } + + @Test + fun `testing proper variables search in class`() { + applyToCode(""" + class A { + var o = 1 + fun foo(a: Int) { + fun foo1() { + b = o + c = o + d = o + o = 15 + o = 17 + } + } + } + """.trimIndent(), 0) { node, counter -> + if (node.elementType == FILE) { + val vars = node.findAllVariablesWithAssignments().mapKeys { it.key.text } + val keys = vars.keys + val var1 = keys.elementAt(0) + Assertions.assertEquals("var o = 1", var1) + Assertions.assertEquals(2, vars[var1]?.size) + } + } + } + + @Test + @Disabled + fun `testing proper variables search with lambda`() { + applyToCode(""" + fun foo(a: Int) { + var a = 1 + a++ + } + """.trimIndent(), 0) { node, counter -> + if (node.elementType == FILE) { + val vars = node.findAllVariablesWithAssignments().mapKeys { it.key.text } + val keys = vars.keys + val var1 = keys.elementAt(0) + Assertions.assertEquals("var a = 1", var1) + Assertions.assertEquals(1, vars[var1]?.size) + } + } + } +} diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariableSearchASTUtilsTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithUsagesSearchTest.kt similarity index 88% rename from diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariableSearchASTUtilsTest.kt rename to diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithUsagesSearchTest.kt index e9247ea480..ee45294e9f 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariableSearchASTUtilsTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithUsagesSearchTest.kt @@ -1,13 +1,14 @@ package org.cqfn.diktat.ruleset.utils import com.pinterest.ktlint.core.ast.ElementType.FILE +import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithUsages import org.cqfn.diktat.util.applyToCode import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test @Suppress("UnsafeCallOnNullableType") -class VariableSearchASTUtilsTest { +class VariablesWithUsagesSearchTest { @Test fun `testing proper variables search in function`() { applyToCode(""" @@ -20,7 +21,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -44,7 +45,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -70,7 +71,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -94,7 +95,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) assertEquals("val v = 0", var1) @@ -119,7 +120,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -142,7 +143,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) assertEquals("val someVal = 0", var1) @@ -163,7 +164,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -188,7 +189,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) assertEquals("var a = 5", var1) @@ -208,7 +209,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) assertEquals("var a = 5", var1) @@ -230,7 +231,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -260,7 +261,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -292,7 +293,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -326,7 +327,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) val var2 = keys.elementAt(1) @@ -358,7 +359,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) assertEquals("var v = 1", var1) @@ -380,7 +381,7 @@ class VariableSearchASTUtilsTest { } """.trimIndent(), 0) { node, counter -> if (node.elementType == FILE) { - val vars = node.collectAllDeclaredVariablesWithUsages().mapKeys { it.key.text } + val vars = node.findAllVariablesWithUsages().mapKeys { it.key.text } val keys = vars.keys val var1 = keys.elementAt(0) @@ -389,6 +390,4 @@ class VariableSearchASTUtilsTest { } } } - } - From 4da0958eb088dacadd983a267daf257c4b7e07e7 Mon Sep 17 00:00:00 2001 From: akuleshov7 Date: Mon, 12 Oct 2020 01:26:12 +0300 Subject: [PATCH 2/4] Improving variable search mechanism ### What's done: 1) Review notes --- .../ruleset/utils/search/VariablesSearch.kt | 10 ++-- .../search/VariablesWithAssignmentSearch.kt | 10 +++- .../utils/search/VariablesWithUsagesSearch.kt | 2 +- .../ruleset/utils/VariablesSearchTest.kt | 51 +++++++++++++++++++ 4 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt index ba58fa6772..b9f83cd035 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt @@ -57,10 +57,12 @@ abstract class VariableSearch(val node: ASTNode, private val filterForVariables: /** * filtering object's fields (expressions) that have same name as variable */ - protected fun isReferenceToFieldOfObject(expression: KtNameReferenceExpression) = - (expression.parent as? KtDotQualifiedExpression)?.run { - receiverExpression != expression && selectorExpression?.referenceExpression() == expression - } ?: false + protected fun KtNameReferenceExpression.isReferenceToFieldOfObject(): Boolean { + val expression = this + return (expression.parent as? KtDotQualifiedExpression)?.run { + receiverExpression != expression && selectorExpression?.referenceExpression() == expression + } ?: false + } /** * filtering local properties from other context (shadowed) and lambda and function arguments with same name diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt index 7eac2bcbd4..2c904d56f7 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt @@ -12,21 +12,27 @@ import org.jetbrains.kotlin.psi.KtProperty class VariablesWithAssignmentSearch(fileNode: ASTNode, filterForVariables: (KtProperty) -> Boolean) : VariableSearch(fileNode, filterForVariables) { + /** + * searching for all assignments of variables in current context [this] + */ override fun KtElement.getAllSearchResults(property: KtProperty): List { return this.node.findAllNodesWithSpecificType(ElementType.BINARY_EXPRESSION) // filtering out all usages that are declared in the same context but are going before the variable declaration + // AND checking that there is an assignment .filter { - // FixMe: bug is here with a search of postfix/prefix variables assignment + // FixMe: bug is here with a search of postfix/prefix variables assignment (like ++). + // FixMe: Currently we check only val a = 5, ++a is not checked here // FixMe: also there can be some tricky cases with setters, but I am not able to imagine them now it.isGoingAfter(property.node) && (it.psi as KtBinaryExpression).operationToken == ElementType.EQ && (it.psi as KtBinaryExpression).left!!.node.elementType == ElementType.REFERENCE_EXPRESSION } .map { (it.psi as KtBinaryExpression).left as KtNameReferenceExpression } + // checking that name of the property in usage matches with the name in the declaration .filter { it.getReferencedNameAsName() == property.nameAsName } .filterNot { expression -> // to avoid false triggering on objects' fields with same name as property - isReferenceToFieldOfObject(expression) || + expression.isReferenceToFieldOfObject() || // to exclude usages of local properties from other context (shadowed) and lambda arguments with same name isReferenceToOtherVariableWithSameName(expression, this, property) } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt index c85ef98f3a..51e9a13bdf 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt @@ -19,7 +19,7 @@ class VariablesWithUsagesSearch(fileNode: ASTNode, .filter { it.getReferencedNameAsName() == property.nameAsName } .filterNot { expression -> // to avoid false triggering on objects' fields with same name as property - isReferenceToFieldOfObject(expression) || + expression.isReferenceToFieldOfObject() || // to exclude usages of local properties from other context (shadowed) and lambda arguments with same name isReferenceToOtherVariableWithSameName(expression, this, property) } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt new file mode 100644 index 0000000000..883f3f1022 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt @@ -0,0 +1,51 @@ +package org.cqfn.diktat.ruleset.utils + +import com.pinterest.ktlint.core.ast.ElementType +import org.cqfn.diktat.ruleset.utils.search.VariableSearch +import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithAssignments +import org.cqfn.diktat.util.applyToCode +import org.jetbrains.kotlin.psi.KtProperty +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.mockito.Mockito + +@Suppress("UnsafeCallOnNullableType") +class VariablesSearchTest { + @Test + fun `testing requirement for collecting variables`() { + applyToCode(""" + fun foo(a: Int) { + fun foo1() { + var o = 1 + b = o + c = o + o = 15 + o = 17 + } + } + """.trimIndent(), 0) { node, counter -> + if (node.elementType != ElementType.FILE) { + val thrown = Assertions.assertThrows(IllegalArgumentException::class.java) { + val variablesSearchAbstract: VariableSearch = Mockito.mock(VariableSearch::class.java, Mockito.CALLS_REAL_METHODS) + val nodeField = VariableSearch::class.java.getDeclaredField("node") + val filter = VariableSearch::class.java.getDeclaredField("filterForVariables") + nodeField.isAccessible = true + filter.isAccessible = true + + nodeField.set(variablesSearchAbstract, node) + filter.set(variablesSearchAbstract, ::filter) + + variablesSearchAbstract.collectVariables() + } + assertTrue(thrown.message!!.contains("To collect all variables in a file you need to provide file root node")); + + } + } + } + + private fun filter(prop: KtProperty) = true + +} + From 488c54c30e11cd764049b0f7db4070d5e97bb6a3 Mon Sep 17 00:00:00 2001 From: akuleshov7 Date: Mon, 12 Oct 2020 13:04:59 +0300 Subject: [PATCH 3/4] Improving variable search mechanism ### What's done: 1) Review notes --- diktat-rules/pom.xml | 5 +++++ pom.xml | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/diktat-rules/pom.xml b/diktat-rules/pom.xml index d4bd8db61d..8835182240 100644 --- a/diktat-rules/pom.xml +++ b/diktat-rules/pom.xml @@ -65,6 +65,11 @@ assertj-core test + + org.mockito + mockito-all + test + diff --git a/pom.xml b/pom.xml index 8d9b81efa1..f6a529ad14 100644 --- a/pom.xml +++ b/pom.xml @@ -163,7 +163,14 @@ assertj-core 3.17.0 + + org.mockito + mockito-all + 1.10.19 + test + + From a462d642950f7d6f380040ff98f0d44e914b03c2 Mon Sep 17 00:00:00 2001 From: akuleshov7 Date: Mon, 12 Oct 2020 13:37:34 +0300 Subject: [PATCH 4/4] Improving variable search mechanism ### What's done: 1) Detekt fixes --- .../ruleset/utils/search/VariablesSearch.kt | 21 +++++++++++++++---- .../search/VariablesWithAssignmentSearch.kt | 4 ++-- .../utils/search/VariablesWithUsagesSearch.kt | 2 +- .../ruleset/utils/VariablesSearchTest.kt | 17 ++++++--------- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt index b9f83cd035..d98a41ead9 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesSearch.kt @@ -1,9 +1,18 @@ package org.cqfn.diktat.ruleset.utils.search import com.pinterest.ktlint.core.ast.ElementType -import org.cqfn.diktat.ruleset.utils.* +import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType +import org.cqfn.diktat.ruleset.utils.getDeclarationScope +import org.cqfn.diktat.ruleset.utils.isGoingAfter import org.jetbrains.kotlin.com.intellij.lang.ASTNode -import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.KtClassBody +import org.jetbrains.kotlin.psi.KtElement +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.KtBlockExpression +import org.jetbrains.kotlin.psi.KtFunctionLiteral +import org.jetbrains.kotlin.psi.KtDotQualifiedExpression +import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.psiUtil.getChildrenOfType import org.jetbrains.kotlin.psi.psiUtil.getParentOfType import org.jetbrains.kotlin.psi.psiUtil.parents @@ -14,7 +23,7 @@ import org.jetbrains.kotlin.psi.psiUtil.referenceExpression * it should be ONLY node of File elementType * @param filterForVariables - condition to filter */ -abstract class VariableSearch(val node: ASTNode, private val filterForVariables: (KtProperty) -> Boolean) { +abstract class VariablesSearch(val node: ASTNode, private val filterForVariables: (KtProperty) -> Boolean) { /** * to complete implementation of a search mechanism you need to specify what and how you will search in current scope @@ -94,4 +103,8 @@ abstract class VariableSearch(val node: ASTNode, private val filterForVariables: } } -fun default(node: KtProperty) = true \ No newline at end of file +/** + * this is a small workaround in case we don't want to make any custom filter while searching variables + */ +@SuppressWarnings("FunctionOnlyReturningConstant") +fun default(node: KtProperty) = true diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt index 2c904d56f7..29a24947a3 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithAssignmentSearch.kt @@ -10,7 +10,7 @@ import org.jetbrains.kotlin.psi.KtNameReferenceExpression import org.jetbrains.kotlin.psi.KtProperty class VariablesWithAssignmentSearch(fileNode: ASTNode, - filterForVariables: (KtProperty) -> Boolean) : VariableSearch(fileNode, filterForVariables) { + filterForVariables: (KtProperty) -> Boolean) : VariablesSearch(fileNode, filterForVariables) { /** * searching for all assignments of variables in current context [this] @@ -25,7 +25,7 @@ class VariablesWithAssignmentSearch(fileNode: ASTNode, // FixMe: also there can be some tricky cases with setters, but I am not able to imagine them now it.isGoingAfter(property.node) && (it.psi as KtBinaryExpression).operationToken == ElementType.EQ && - (it.psi as KtBinaryExpression).left!!.node.elementType == ElementType.REFERENCE_EXPRESSION + (it.psi as KtBinaryExpression).left?.node?.elementType == ElementType.REFERENCE_EXPRESSION } .map { (it.psi as KtBinaryExpression).left as KtNameReferenceExpression } // checking that name of the property in usage matches with the name in the declaration diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt index 51e9a13bdf..2422a88bc8 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/search/VariablesWithUsagesSearch.kt @@ -9,7 +9,7 @@ import org.jetbrains.kotlin.psi.KtNameReferenceExpression import org.jetbrains.kotlin.psi.KtProperty class VariablesWithUsagesSearch(fileNode: ASTNode, - filterForVariables: (KtProperty) -> Boolean) : VariableSearch(fileNode, filterForVariables) { + filterForVariables: (KtProperty) -> Boolean) : VariablesSearch(fileNode, filterForVariables) { override fun KtElement.getAllSearchResults(property: KtProperty): List { return this.node.findAllNodesWithSpecificType(ElementType.REFERENCE_EXPRESSION) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt index 883f3f1022..8b1430039e 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt @@ -1,14 +1,12 @@ package org.cqfn.diktat.ruleset.utils import com.pinterest.ktlint.core.ast.ElementType -import org.cqfn.diktat.ruleset.utils.search.VariableSearch -import org.cqfn.diktat.ruleset.utils.search.findAllVariablesWithAssignments +import org.cqfn.diktat.ruleset.utils.search.VariablesSearch +import org.cqfn.diktat.ruleset.utils.search.default import org.cqfn.diktat.util.applyToCode -import org.jetbrains.kotlin.psi.KtProperty import org.junit.jupiter.api.Assertions import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test -import org.junit.jupiter.api.assertThrows import org.mockito.Mockito @Suppress("UnsafeCallOnNullableType") @@ -28,14 +26,14 @@ class VariablesSearchTest { """.trimIndent(), 0) { node, counter -> if (node.elementType != ElementType.FILE) { val thrown = Assertions.assertThrows(IllegalArgumentException::class.java) { - val variablesSearchAbstract: VariableSearch = Mockito.mock(VariableSearch::class.java, Mockito.CALLS_REAL_METHODS) - val nodeField = VariableSearch::class.java.getDeclaredField("node") - val filter = VariableSearch::class.java.getDeclaredField("filterForVariables") + val variablesSearchAbstract: VariablesSearch = Mockito.mock(VariablesSearch::class.java, Mockito.CALLS_REAL_METHODS) + val nodeField = VariablesSearch::class.java.getDeclaredField("node") + val filter = VariablesSearch::class.java.getDeclaredField("filterForVariables") nodeField.isAccessible = true filter.isAccessible = true nodeField.set(variablesSearchAbstract, node) - filter.set(variablesSearchAbstract, ::filter) + filter.set(variablesSearchAbstract, ::default) variablesSearchAbstract.collectVariables() } @@ -44,8 +42,5 @@ class VariablesSearchTest { } } } - - private fun filter(prop: KtProperty) = true - }