Skip to content

Commit

Permalink
Rule 6.1.5: Explicit supertype qualification should not be used if th…
Browse files Browse the repository at this point in the history
…ere is not clash between called methods (#458)

* rule 6.1.5

### What's done:
   Implemented rule 6.1.5
  Added documentation
  • Loading branch information
kentr0w authored Nov 16, 2020
2 parents d296add + 80fd3f3 commit 1e019c0
Show file tree
Hide file tree
Showing 14 changed files with 398 additions and 0 deletions.
4 changes: 4 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -366,3 +366,7 @@
- name: COMPACT_OBJECT_INITIALIZATION
enabled: true
configuration: {}
# Checks explicit supertype qualification
- name: USELESS_SUPERTYPE
enabled: true
configuration: {}
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 @@ -213,4 +213,6 @@ public object WarningNames {
public const val CUSTOM_GETTERS_SETTERS: String = "CUSTOM_GETTERS_SETTERS"

public const val COMPACT_OBJECT_INITIALIZATION: String = "COMPACT_OBJECT_INITIALIZATION"

public const val USELESS_SUPERTYPE: String = "USELESS_SUPERTYPE"
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S
TRIVIAL_ACCESSORS_ARE_NOT_RECOMMENDED(true, "trivial property accessors are not recommended"),
CUSTOM_GETTERS_SETTERS(false, "Custom getters and setters are not recommended, use class methods instead"),
COMPACT_OBJECT_INITIALIZATION(true, "class instance can be initialized in `apply` block"),
USELESS_SUPERTYPE(true,"unnecessary supertype specification"),
;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
::PackageNaming,
::IdentifierNaming,
// code structure
::UselessSupertype,
::ClassLikeStructuresOrderRule,
::WhenMustHaveElseRule,
::BracesInConditionalsAndLoopsRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package org.cqfn.diktat.ruleset.rules

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
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_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.FILE
import com.pinterest.ktlint.core.ast.ElementType.FUN
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
import com.pinterest.ktlint.core.ast.ElementType.OPEN_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.SUPER_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_CALL_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_LIST
import com.pinterest.ktlint.core.ast.ElementType.TYPE_REFERENCE
import com.pinterest.ktlint.core.ast.parent
import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.constants.EmitType
import org.cqfn.diktat.ruleset.constants.Warnings.USELESS_SUPERTYPE
import org.cqfn.diktat.ruleset.utils.*
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.psi.psiUtil.siblings

/**
* rule 6.1.5
* Explicit supertype qualification should not be used if there is not clash between called methods
* fixme can't fix supertypes that are defined in other files.
*/
class UselessSupertype(private val configRules: List<RulesConfig>) : Rule("useless-override") {

companion object {
private val SUPER_TYPE = listOf(SUPER_TYPE_CALL_ENTRY, SUPER_TYPE_ENTRY)
}

private lateinit var emitWarn: EmitType
private var isFixMode: Boolean = false

override fun visit(node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) {
emitWarn = emit
isFixMode = autoCorrect

if (node.elementType == CLASS)
checkClass(node)
}

private fun checkClass(node: ASTNode) {
val superNodes = node.findChildByType(SUPER_TYPE_LIST)?.findAllNodesWithCondition({ it.elementType in SUPER_TYPE })?.takeIf { it.isNotEmpty() } ?: return
val qualifiedSuperCalls = node.findAllNodesWithSpecificType(DOT_QUALIFIED_EXPRESSION)
.mapNotNull { findFunWithSuper(it) }.ifEmpty { return }
if (superNodes.size == 1) {
qualifiedSuperCalls.map { removeSupertype(it.first) }
} else {
handleManyImpl(superNodes, qualifiedSuperCalls)
}
}

private fun handleManyImpl(superNodes: List<ASTNode>, overrideNodes: List<Pair<ASTNode, ASTNode>>) {
val uselessSuperType = findAllSupers(superNodes, overrideNodes.map { it.second.text })
?.filter { it.value == 1 } // filtering methods whose names occur only once
?.map { it.key } // take their names
?: return
overrideNodes
.filter {
it.second.text in uselessSuperType
}.map {
removeSupertype(it.first)
}
}

@Suppress("UnsafeCallOnNullableType")
private fun removeSupertype(node: ASTNode) {
USELESS_SUPERTYPE.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) {
val startNode = node.parent({ it.elementType == SUPER_EXPRESSION })!!.findChildByType(REFERENCE_EXPRESSION)!!
val lastNode = startNode.siblings(true).last()
startNode.treeParent.removeRange(startNode.treeNext, lastNode)
startNode.treeParent.removeChild(lastNode)
}
}

/**
* Method finds pair of identifier supertype and method name else return null
* example: super<A>.foo() -> return Pair(A, foo)
* super.foo() -> null
* @param node - node of type DOT_QUALIFIED_EXPRESSION
*
* @return pair of identifier
*/
@Suppress("UnsafeCallOnNullableType")
private fun findFunWithSuper(node: ASTNode): Pair<ASTNode, ASTNode>? {
return Pair(
node.findChildByType(SUPER_EXPRESSION)
?.findChildByType(TYPE_REFERENCE)
?.findAllNodesWithSpecificType(IDENTIFIER)
?.firstOrNull() ?: return null,
node.findChildByType(CALL_EXPRESSION)
?.findAllNodesWithSpecificType(IDENTIFIER)
?.firstOrNull() ?: return null)
}

/**
* The method looks in the same file for all super interfaces or a class, in each it looks for methods
* that can be overridden and creates a map with a key - the name of the method and value - the number of times it meets
*
* @param superTypeList - list of identifiers super classes
* @param methodsName - name of overrides methods
*
* @return map name of method and the number of times it meets
*/
@Suppress("UnsafeCallOnNullableType")
private fun findAllSupers(superTypeList: List<ASTNode>, methodsName: List<String>): Map<String, Int>? {
val fileNode = superTypeList.first().parent({ it.elementType == FILE })!!
val superNodesIdentifier = superTypeList.map { it.findAllNodesWithSpecificType(IDENTIFIER).first().text }
val superNodes = fileNode.findAllNodesWithCondition({ superClass ->
superClass.elementType == CLASS &&
superClass.getIdentifierName()!!.text in superNodesIdentifier
}).mapNotNull { it.findChildByType(CLASS_BODY) }
if (superNodes.size != superTypeList.size)
return null
val functionNameMap = hashMapOf<String, Int>()
superNodes.forEach { classBody ->
val overrideFunctions = classBody.findAllNodesWithSpecificType(FUN)
.filter {
(if (classBody.treeParent.hasChildOfType(CLASS_KEYWORD)) it.findChildByType(MODIFIER_LIST)!!.hasChildOfType(OPEN_KEYWORD) else true) &&
it.getIdentifierName()!!.text in methodsName
}
overrideFunctions.forEach {
functionNameMap.compute(it.getIdentifierName()!!.text) { _, oldValue -> (oldValue ?: 0) + 1}
}
}
return functionNameMap.toMap()
}
}
4 changes: 4 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -363,5 +363,9 @@
configuration: { }
# Checks if class instantiation can be wrapped in `apply` for better readability
- name: COMPACT_OBJECT_INITIALIZATION
enabled: true
configuration: {}
# Checks explicit supertype qualification
- name: USELESS_SUPERTYPE
enabled: true
configuration: {}
4 changes: 4 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -365,5 +365,9 @@
configuration: { }
# Checks if class instantiation can be wrapped in `apply` for better readability
- name: COMPACT_OBJECT_INITIALIZATION
enabled: true
configuration: {}
# Checks explicit supertype qualification
- name: USELESS_SUPERTYPE
enabled: true
configuration: {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.cqfn.diktat.ruleset.chapter6

import generated.WarningNames
import org.cqfn.diktat.util.FixTestBase
import org.cqfn.diktat.ruleset.rules.UselessSupertype
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

class UselessSupertypeFixTest : FixTestBase("test/paragraph6/useless-override", ::UselessSupertype) {

@Test
@Tag(WarningNames.USELESS_SUPERTYPE)
fun `fix example with one super`() {
fixAndCompare("UselessOverrideExpected.kt", "UselessOverrideTest.kt")
}

@Test
@Tag(WarningNames.USELESS_SUPERTYPE)
fun `fix several super`() {
fixAndCompare("SeveralSuperTypesExpected.kt", "SeveralSuperTypesTest.kt")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package org.cqfn.diktat.ruleset.chapter6

import com.pinterest.ktlint.core.LintError
import generated.WarningNames
import org.cqfn.diktat.ruleset.constants.Warnings.USELESS_SUPERTYPE
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.rules.UselessSupertype
import org.cqfn.diktat.util.LintTestBase
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

class UselessSupertypeWarnTest: LintTestBase(::UselessSupertype) {

private val ruleId = "$DIKTAT_RULE_SET_ID:useless-override"

@Test
@Tag(WarningNames.USELESS_SUPERTYPE)
fun `check simple wrong example`() {
lintMethod(
"""
open class Rectangle {
open fun draw() { /* ... */ }
}
class Square() : Rectangle() {
override fun draw() {
/**
*
* hehe
*/
super<Rectangle>.draw()
}
}
class Square2() : Rectangle() {
override fun draw() {
//hehe
/*
hehe
*/
super<Rectangle>.draw()
}
}
class Square2() : Rectangle() {
override fun draw() {
val q = super.draw()
}
}
class A: Runnable {
override fun run() {
}
}
""".trimMargin(),
LintError(11,35, ruleId, "${USELESS_SUPERTYPE.warnText()} Rectangle", true),
LintError(21,35, ruleId, "${USELESS_SUPERTYPE.warnText()} Rectangle", true)
)
}

@Test
@Tag(WarningNames.USELESS_SUPERTYPE)
fun `check example with two super`() {
lintMethod(
"""
open class Rectangle {
open fun draw() { /* ... */ }
}
interface KK {
fun draw() {}
fun kk() {}
}
class Square2() : Rectangle(), KK {
override fun draw() {
super<Rectangle>.draw()
super<KK>.draw()
}
private fun goo() {
super<KK>.kk()
}
}
""".trimMargin(),
LintError(17,35, ruleId, "${USELESS_SUPERTYPE.warnText()} KK", true)
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package test.paragraph6.`useless-override`

open class A {
open fun foo(){}
}

interface C {
fun foo(){}
fun goo(){}
}

class B: A(){
override fun foo() {
super.foo()
}
}

class D: A(), C {
override fun foo() {
super<C>.foo()
super<A>.foo()
super.goo()
}

private fun qwe(){
val q = super.goo()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package test.paragraph6.`useless-override`

open class A {
open fun foo(){}
}

interface C {
fun foo(){}
fun goo(){}
}

class B: A(){
override fun foo() {
super<A>.foo()
}
}

class D: A(), C {
override fun foo() {
super<C>.foo()
super<A>.foo()
super<C>.goo()
}

private fun qwe(){
val q = super<C>.goo()
}
}
Loading

0 comments on commit 1e019c0

Please sign in to comment.