Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule 6.1.6 abstract classes should have at least one abstract method #457

Merged
merged 6 commits into from
Oct 26, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -331,5 +331,9 @@
configuration: {}
# Checks that never use the name of a variable in the custom getter or setter
- name: WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR
enabled: true
configuration: {}
# Checks that there are abstract functions in abstract class
- name: CLASS_SHOULD_NOT_BE_ABSTRACT
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 @@ -196,4 +196,6 @@ public object WarningNames {

public const val WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR: String =
"WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR"

public const val CLASS_SHOULD_NOT_BE_ABSTRACT: String = "CLASS_SHOULD_NOT_BE_ABSTRACT"
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S
// ======== chapter 6 ========
USE_DATA_CLASS(false, "this class can be converted to a data class"),
WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR(false, "Use `field` keyword instead of property name inside property accessors"),
CLASS_SHOULD_NOT_BE_ABSTRACT(true, "class should not be abstract, because it has no abstract functions"),
;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.common.config.rules.RulesConfigReader
import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.rules.calculations.AccurateCalculationsRule
import org.cqfn.diktat.ruleset.rules.classes.AbstractClassesRule
import org.cqfn.diktat.ruleset.rules.classes.DataClassesRule
import org.cqfn.diktat.ruleset.rules.comments.CommentsRule
import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule
Expand Down Expand Up @@ -63,6 +64,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy
::SmartCastRule,
::PropertyAccessorFields,
::EnumsSeparated,
::AbstractClassesRule,
::VariableGenericTypeDeclarationRule,
::SingleLineStatementsRule,
::CommentsFormatting,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.cqfn.diktat.ruleset.rules.classes

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType.ABSTRACT_KEYWORD
import com.pinterest.ktlint.core.ast.ElementType.CLASS
import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
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.WHITE_SPACE
import com.pinterest.ktlint.core.ast.isWhiteSpace
import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.constants.Warnings.CLASS_SHOULD_NOT_BE_ABSTRACT
import org.cqfn.diktat.ruleset.utils.getAllChildrenWithType
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType
import org.cqfn.diktat.ruleset.utils.hasChildOfType
import org.jetbrains.kotlin.com.intellij.lang.ASTNode

/**
* Checks if abstract class has any abstract method. If not, warns that class should not be abstract
*/
class AbstractClassesRule(private val configRule: List<RulesConfig>) : Rule("abstract-classes") {
private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit)
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) {
val classBody = node.getFirstChildWithType(CLASS_BODY) ?: return

if (hasAbstractModifier(node)) {
handleAbstractClass(classBody, node)
}
}
}

private fun hasAbstractModifier(node: ASTNode): Boolean =
node.getFirstChildWithType(MODIFIER_LIST)?.hasChildOfType(ABSTRACT_KEYWORD) ?: false

@Suppress("UnsafeCallOnNullableType")
private fun handleAbstractClass(node: ASTNode, classNode: ASTNode) {
val functions = node.getAllChildrenWithType(FUN)

val identifier = classNode.getFirstChildWithType(IDENTIFIER)!!.text

if (functions.isNotEmpty() && functions.none { hasAbstractModifier(it) }) {
CLASS_SHOULD_NOT_BE_ABSTRACT.warnAndFix(configRule, emitWarn, isFixMode, identifier, node.startOffset, node) {
val modList = classNode.getFirstChildWithType(MODIFIER_LIST)!!
if (modList.getChildren(null).size > 1) {
val abstractKeyword = modList.getFirstChildWithType(ABSTRACT_KEYWORD)!!

// we are deleting one keyword, so we need to delete extra space
val spaceInModifiers = if (abstractKeyword == modList.lastChildNode) {
modList.treeNext
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved
} else {
abstractKeyword.treeNext
}
modList.removeChild(modList.getFirstChildWithType(ABSTRACT_KEYWORD)!!)
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved
if (spaceInModifiers != null && spaceInModifiers.isWhiteSpace()) {
modList.removeChild(spaceInModifiers)
}
} else {
classNode.removeChild(modList)
if (classNode.firstChildNode.isWhiteSpace()) {
aktsay6 marked this conversation as resolved.
Show resolved Hide resolved
classNode.removeChild(classNode.firstChildNode)
}
}
}
}
}
}
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 @@ -330,5 +330,9 @@
configuration: {}
# Checks that never use the name of a variable in the custom getter or setter
- name: WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR
enabled: true
configuration: {}
# Checks that there are abstract functions in abstract class
- name: CLASS_SHOULD_NOT_BE_ABSTRACT
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 @@ -332,5 +332,9 @@
configuration: {}
# Checks that never use the name of a variable in the custom getter or setter
- name: WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR
enabled: true
configuration: {}
# Checks that there are abstract functions in abstract class
- name: CLASS_SHOULD_NOT_BE_ABSTRACT
enabled: true
configuration: {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.cqfn.diktat.ruleset.chapter6

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

class AbstractClassesFixTest : FixTestBase("test/paragraph6/abstract_classes", ::AbstractClassesRule) {
@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `fix abstract class`() {
fixAndCompare("ShouldRemoveAbstractKeywordExpected.kt", "ShouldRemoveAbstractKeywordTest.kt")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package org.cqfn.diktat.ruleset.chapter6

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

class AbstractClassesWarnTest : LintTestBase(::AbstractClassesRule) {
private val ruleId = "$DIKTAT_RULE_SET_ID:abstract-classes"

@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `should not remove abstract`() {
lintMethod(
"""
|abstract class Some(val a: Int = 5) {
| abstract fun func() {}
|
| fun another() {}
|}
""".trimMargin()
)
}


@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `should remove abstract`() {
lintMethod(
"""
|abstract class Some(val a: Int = 5) {
| fun func() {}
|}
""".trimMargin(),
LintError(1, 37, ruleId, "${Warnings.CLASS_SHOULD_NOT_BE_ABSTRACT.warnText()} Some", true)
)
}

@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `should remove abstract with inner`() {
lintMethod(
"""
|class Some(val a: Int = 5) {
| fun func() {}
|
| inner abstract class Inner {
| fun another()
| }
|}
""".trimMargin(),
LintError(4, 32, ruleId, "${Warnings.CLASS_SHOULD_NOT_BE_ABSTRACT.warnText()} Inner", true)
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package test.paragraph6.abstract_classes

class Some() {
fun some(){}

fun another(){}

@SomeAnnotation @Another inner class Any {
fun func(){}
}

inner class Second {
fun someFunc(){}
}
}

abstract class Another {
abstract fun absFunc()

fun someFunc(){}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package test.paragraph6.abstract_classes

abstract class Some() {
fun some(){}

fun another(){}

@SomeAnnotation @Another abstract inner class Any {
fun func(){}
}

inner abstract class Second {
petertrr marked this conversation as resolved.
Show resolved Hide resolved
fun someFunc(){}
}
}

abstract class Another {
abstract fun absFunc()

fun someFunc(){}
}
3 changes: 2 additions & 1 deletion info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,5 @@
| 5 | 5.2.2 | TOO_MANY_PARAMETERS | Check: if function contains more parameters than allowed | no | maxParameterListSize |
| 5 | 5.2.3 | WRONG_OVERLOADING_FUNCTION_ARGUMENTS | Check: function has overloading instead use default arguments | no | -|
| 6 | 6.1.2 | USE_DATA_CLASS | Check: if class can be made as data class | no | - | yes |
| 6 | 6.1.9 | WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR | Check: used the name of a variable in the custom getter or setter | no | - |
| 6 | 6.1.9 | WRONG_NAME_OF_VARIABLE_INSIDE_ACCESSOR | Check: used the name of a variable in the custom getter or setter | no | - |
| 6 | 6.1.6 | CLASS_SHOULD_NOT_BE_ABSTRACT | Checks: if abstract class has any abstract method. If not, warns that class should not be abstract<br>Fix: deletes abstract modifier | yes | - | - |