Fix: Don't remove abstract for subclasses (#1116)

### What's done: * Fix in logic * Tests Closes #1114
saveourtool · Nov 10, 2021 · 80d64ed · 80d64ed
1 parent 0f1f8af
commit 80d64ed
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 4 deletions.
diff --git a/...les/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/AbstractClassesRule.kt b/...les/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/AbstractClassesRule.kt
@@ -15,8 +15,11 @@ 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.PROPERTY
+import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_CALL_ENTRY
+import com.pinterest.ktlint.core.ast.ElementType.SUPER_TYPE_LIST
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
+import org.jetbrains.kotlin.psi.psiUtil.children
 
 /**
  * Checks if abstract class has any abstract method. If not, warns that class should not be abstract
@@ -29,14 +32,20 @@ class AbstractClassesRule(configRules: List<RulesConfig>) : DiktatRule(
         if (node.elementType == CLASS) {
             val classBody = node.getFirstChildWithType(CLASS_BODY) ?: return
 
-            if (hasAbstractModifier(node)) {
+            // If an abstract class extends another class, than that base class can be abstract too.
+            // Then this class must have `abstract` modifier even if it doesn't have any abstract members.
+            if (node.hasAbstractModifier() && node.isNotSubclass()) {
                 handleAbstractClass(classBody, node)
             }
         }
     }
 
-    private fun hasAbstractModifier(node: ASTNode): Boolean =
-            node.getFirstChildWithType(MODIFIER_LIST)?.hasChildOfType(ABSTRACT_KEYWORD) ?: false
+    private fun ASTNode.hasAbstractModifier(): Boolean =
+            getFirstChildWithType(MODIFIER_LIST)?.hasChildOfType(ABSTRACT_KEYWORD) ?: false
+
+    private fun ASTNode.isNotSubclass(): Boolean = findChildByType(SUPER_TYPE_LIST)?.children()?.filter {
+        it.elementType == SUPER_TYPE_CALL_ENTRY
+    }?.none() ?: true
 
     @Suppress("UnsafeCallOnNullableType")
     private fun handleAbstractClass(node: ASTNode, classNode: ASTNode) {
@@ -46,7 +55,7 @@ class AbstractClassesRule(configRules: List<RulesConfig>) : DiktatRule(
 
         val identifier = classNode.getFirstChildWithType(IDENTIFIER)!!.text
 
-        if (members.isNotEmpty() && members.none { hasAbstractModifier(it) }) {
+        if (members.isNotEmpty() && members.none { it.hasAbstractModifier() }) {
             CLASS_SHOULD_NOT_BE_ABSTRACT.warnAndFix(configRules, emitWarn, isFixMode, identifier, node.startOffset, node) {
                 val modList = classNode.getFirstChildWithType(MODIFIER_LIST)!!
                 val abstractKeyword = modList.getFirstChildWithType(ABSTRACT_KEYWORD)!!

diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/AbstractClassesWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter6/AbstractClassesWarnTest.kt
@@ -101,4 +101,29 @@ class AbstractClassesWarnTest : LintTestBase(::AbstractClassesRule) {
             """.trimMargin()
         )
     }
+
+    @Test
+    @Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
+    fun `should not trigger on classes that extend other classes`() {
+        lintMethod(
+            """
+                |abstract class Example: Base() {
+                |    fun foo() {}
+                |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
+    fun `should trigger on classes that implement interfaces`() {
+        lintMethod(
+            """
+                |abstract class Example: Base {
+                |    fun foo() {}
+                |}
+            """.trimMargin(),
+            LintError(1, 30, ruleId, "[CLASS_SHOULD_NOT_BE_ABSTRACT] class should not be abstract, because it has no abstract functions: Example", true)
+        )
+    }
 }