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

expect/actual removing abstract modifier #1011

Merged
merged 4 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ 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.isWhiteSpace
import com.pinterest.ktlint.core.ast.ElementType.OPEN_KEYWORD
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement

/**
* Checks if abstract class has any abstract method. If not, warns that class should not be abstract
Expand Down Expand Up @@ -45,25 +46,9 @@ class AbstractClassesRule(configRules: List<RulesConfig>) : DiktatRule(
if (functions.isNotEmpty() && functions.none { hasAbstractModifier(it) }) {
CLASS_SHOULD_NOT_BE_ABSTRACT.warnAndFix(configRules, 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.firstChildNode) {
abstractKeyword.treeNext
} else {
abstractKeyword.treePrev
}
modList.removeChild(abstractKeyword)
if (spaceInModifiers != null && spaceInModifiers.isWhiteSpace()) {
modList.removeChild(spaceInModifiers)
}
} else {
if (modList.treeNext.isWhiteSpace()) {
classNode.removeChild(modList.treeNext)
}
classNode.removeChild(modList)
}
val abstractKeyword = modList.getFirstChildWithType(ABSTRACT_KEYWORD)!!
val newOpenKeyword = LeafPsiElement(OPEN_KEYWORD, "open")
modList.replaceChild(abstractKeyword, newOpenKeyword)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ class AbstractClassesFixTest : FixTestBase("test/chapter6/abstract_classes", ::A
@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `fix abstract class`() {
fixAndCompare("ShouldRemoveAbstractKeywordExpected.kt", "ShouldRemoveAbstractKeywordTest.kt")
fixAndCompare("ShouldReplaceAbstractKeywordExpected.kt", "ShouldReplaceAbstractKeywordTest.kt")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class AbstractClassesWarnTest : LintTestBase(::AbstractClassesRule) {

@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `should not remove abstract`() {
fun `should not replace abstract with open`() {
lintMethod(
"""
|abstract class Some(val a: Int = 5) {
Expand All @@ -29,7 +29,7 @@ class AbstractClassesWarnTest : LintTestBase(::AbstractClassesRule) {

@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `should remove abstract`() {
fun `should replace abstract on open`() {
lintMethod(
"""
|abstract class Some(val a: Int = 5) {
Expand All @@ -42,7 +42,7 @@ class AbstractClassesWarnTest : LintTestBase(::AbstractClassesRule) {

@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `should remove abstract with inner`() {
fun `should replace abstract on open with inner`() {
lintMethod(
"""
|class Some(val a: Int = 5) {
Expand All @@ -56,4 +56,34 @@ class AbstractClassesWarnTest : LintTestBase(::AbstractClassesRule) {
LintError(4, 32, ruleId, "${Warnings.CLASS_SHOULD_NOT_BE_ABSTRACT.warnText()} Inner", true)
)
}

@Test
@Tag(CLASS_SHOULD_NOT_BE_ABSTRACT)
fun `should replace abstract on open in actual or expect classes`() {
lintMethod(
"""
|actual abstract class CoroutineTest actual constructor() {
| /**
| * Test rule
| */
| @get:Rule
| var coroutineTestRule = CoroutineTestRule()
|
| /**
| * Run test
| *
| * @param T
| * @param block
| * @receiver a Coroutine Scope
| */
| actual fun <T> runTest(block: suspend CoroutineScope.() -> T) {
| runBlocking {
| block()
| }
| }
|}
""".trimMargin(),
LintError(1, 58, ruleId, "${Warnings.CLASS_SHOULD_NOT_BE_ABSTRACT.warnText()} CoroutineTest", true)
)
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package test.paragraph6.abstract_classes

actual open class CoroutineTest actual constructor() {
actual fun <T> runTest(block: suspend CoroutineScope.() -> T) {
runBlocking {
block()
}
}
}

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

fun another(){}

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

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

abstract class Another {
abstract fun absFunc()

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

actual abstract class CoroutineTest actual constructor() {
actual fun <T> runTest(block: suspend CoroutineScope.() -> T) {
runBlocking {
block()
}
}
}

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

Expand Down
11 changes: 9 additions & 2 deletions info/guide/guide-chapter-6.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ class Square() : Rectangle() {

#### <a name="r6.1.6"></a> 6.1.6 Abstract class should have at least one abstract method
Abstract classes are used to force a developer to implement some of its parts in their inheritors.
When the abstract class has no abstract methods, it was set `abstract` incorrectly and can be converted to a regular class.
When the abstract class has no abstract methods, it was set `abstract` incorrectly and can be converted to open class.

**Invalid example**:
```kotlin
Expand All @@ -204,11 +204,18 @@ abstract class NotAbstract {
}

// OR
class NotAbstract {
open class NotAbstract {
fun foo() {}

fun test() {}
}

// OR
class NotAbstract {
fun foo() {}

fun test() {}
}
```


Expand Down