Skip to content

Commit

Permalink
feature/no-extension-func-with-class(#731)
Browse files Browse the repository at this point in the history
### What's done:
  * Logic done
  * Added warn tests
  • Loading branch information
aktsay6 committed Jan 27, 2021
1 parent 00a00ae commit 718005c
Show file tree
Hide file tree
Showing 12 changed files with 209 additions and 5 deletions.
3 changes: 3 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -430,4 +430,7 @@
enabled: true
# If there exists negated version of function you should prefer it instead of !functionCall
- name: INVERSE_FUNCTION_PREFERRED
enabled: true
# If file contains class, then it can't contain extension functions
- name: EXTENSION_FUNCTION_WITH_CLASS
enabled: true
4 changes: 3 additions & 1 deletion diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ public object WarningNames {

public const val CUSTOM_LABEL: String = "CUSTOM_LABEL"

public const val INVERSE_FUNCTION_PREFERRED: String = "INVERSE_FUNCTION_PREFERRED"

public const val SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY: String =
"SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY"

Expand Down Expand Up @@ -238,5 +240,5 @@ public object WarningNames {

public const val OBJECT_IS_PREFERRED: String = "OBJECT_IS_PREFERRED"

public const val INVERSE_FUNCTION_PREFERRED: String = "INVERSE_FUNCTION_PREFERRED"
public const val EXTENSION_FUNCTION_WITH_CLASS: String = "EXTENSION_FUNCTION_WITH_CLASS"
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ enum class Warnings(
RUN_BLOCKING_INSIDE_ASYNC(false, "5.2.4", "avoid using runBlocking in asynchronous code"),
TOO_MANY_LINES_IN_LAMBDA(false, "5.2.5", "long lambdas should have a parameter name instead of it"),
CUSTOM_LABEL(false, "5.2.6", "avoid using expression with custom label"),
INVERSE_FUNCTION_PREFERRED(true, "5.1.4", "it is better to use inverse function"),

// ======== chapter 6 ========
SINGLE_CONSTRUCTOR_SHOULD_BE_PRIMARY(true, "6.1.1", "if a class has single constructor, it should be converted to a primary constructor"),
Expand All @@ -151,7 +152,7 @@ enum class Warnings(
NO_CORRESPONDING_PROPERTY(false, "6.1.7", "backing property should have the same name, but there is no corresponding property"),
AVOID_USING_UTILITY_CLASS(false, "6.4.1", "avoid using utility classes/objects, use extensions functions"),
OBJECT_IS_PREFERRED(true, "6.4.2", "it is better to use object for stateless classes"),
INVERSE_FUNCTION_PREFERRED(true, "5.1.4", "it is better to use inverse function"),
EXTENSION_FUNCTION_WITH_CLASS(false, "6.2.3", "do not use extension functions with class defined in the same file"),
;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import org.cqfn.diktat.ruleset.rules.chapter6.classes.StatelessClassesRule

import com.pinterest.ktlint.core.RuleSet
import com.pinterest.ktlint.core.RuleSetProvider
import org.cqfn.diktat.ruleset.rules.chapter6.ExtensionFunctionsInFileRule
import org.jetbrains.kotlin.org.jline.utils.Levenshtein
import org.slf4j.LoggerFactory

Expand Down Expand Up @@ -153,6 +154,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
::CustomGetterSetterRule,
::CompactInitialization,
// other rules
::ExtensionFunctionsInFileRule,
::CheckInverseMethodRule,
::StatelessClassesRule,
::ImplicitBackingPropertyRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.cqfn.diktat.ruleset.rules.chapter6

import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.constants.EmitType
import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.utils.findAllNodesWithSpecificType
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType
import org.cqfn.diktat.ruleset.utils.hasChildOfType

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.CLASS
import com.pinterest.ktlint.core.ast.ElementType.DOT
import com.pinterest.ktlint.core.ast.ElementType.FUN
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.psi.KtFunction

/**
* This rule checks if there are any extension functions in the file, where class is already defined.
*/
class ExtensionFunctionsInFileRule(private val configRules: List<RulesConfig>) : Rule("extension-functions-class-file") {
private var isFixMode: Boolean = false
private lateinit var emitWarn: EmitType

override fun visit(node: ASTNode,
autoCorrect: Boolean,
emit: EmitType) {
emitWarn = emit
isFixMode = autoCorrect

if (node.elementType == ElementType.FILE && node.hasChildOfType(CLASS)) {
collectAllExtensionFunctions(node).forEach {
fireWarning(it)
}
}
}

private fun fireWarning(node: ASTNode) {
Warnings.EXTENSION_FUNCTION_WITH_CLASS.warn(configRules, emitWarn, isFixMode, "fun ${(node.psi as KtFunction).name}", node.startOffset, node)
}

private fun collectAllExtensionFunctions(node: ASTNode): List<ASTNode> {
return node.findAllNodesWithSpecificType(FUN).filter { isExtensionFunction(it) }
}

@Suppress("UnsafeCallOnNullableType")
private fun isExtensionFunction(node: ASTNode): Boolean =
node
.getFirstChildWithType(IDENTIFIER)!!
.treePrev
.elementType == DOT

}
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -430,4 +430,7 @@
enabled: true
# If there exists negated version of function you should prefer it instead of !functionCall
- name: INVERSE_FUNCTION_PREFERRED
enabled: true
# If file contains class, then it can't contain extension functions
- name: EXTENSION_FUNCTION_WITH_CLASS
enabled: true
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -427,4 +427,7 @@
enabled: true
# If there exists negated version of function you should prefer it instead of !functionCall
- name: INVERSE_FUNCTION_PREFERRED
enabled: true
# If file contains class, then it can't contain extension functions
- name: EXTENSION_FUNCTION_WITH_CLASS
enabled: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package org.cqfn.diktat.ruleset.chapter6

import org.cqfn.diktat.ruleset.constants.Warnings
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.rules.chapter6.ExtensionFunctionsInFileRule
import org.cqfn.diktat.util.LintTestBase

import com.pinterest.ktlint.core.LintError
import generated.WarningNames.EXTENSION_FUNCTION_WITH_CLASS
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

class ExtensionFunctionsInFileWarnTest : LintTestBase(::ExtensionFunctionsInFileRule) {
private val ruleId = "$DIKTAT_RULE_SET_ID:extension-functions-class-file"

@Test
@Tag(EXTENSION_FUNCTION_WITH_CLASS)
fun `should warn on function`() {
lintMethod(
"""
|class Some1 private constructor () {
|
|}
|
|private fun String.coolStr() {
|
|}
""".trimMargin(),
LintError(5, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_WITH_CLASS.warnText()} fun coolStr")
)
}

@Test
@Tag(EXTENSION_FUNCTION_WITH_CLASS)
fun `should warn on several functions`() {
lintMethod(
"""
|class Some1 private constructor () {
|
|}
|
|private fun /* Random comment */ String.coolStr() {
|
|}
|
|private fun Another.extMethod() {
|
|}
""".trimMargin(),
LintError(5, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_WITH_CLASS.warnText()} fun coolStr"),
LintError(9, 1, ruleId, "${Warnings.EXTENSION_FUNCTION_WITH_CLASS.warnText()} fun extMethod"),
)
}

@Test
@Tag(EXTENSION_FUNCTION_WITH_CLASS)
fun `should not raise a warning when there is no class`() {
lintMethod(
"""
|private fun String.coolStr() {
|
|}
|
|private fun /* Random comment */ Another.extMethod() {
|
|}
""".trimMargin()
)
}

@Test
@Tag(EXTENSION_FUNCTION_WITH_CLASS)
fun `should not raise a warning when there is no extension functions`() {
lintMethod(
"""
|class Some {
|
|}
""".trimMargin()
)
}

@Test
@Tag(EXTENSION_FUNCTION_WITH_CLASS)
fun `should raise a warning when extension function is in the class`() {
lintMethod(
"""
|class Some {
|
| fun String.str() {
|
| }
|}
""".trimMargin(),
LintError(3, 4, ruleId, "${Warnings.EXTENSION_FUNCTION_WITH_CLASS.warnText()} fun str")
)
}

@Test
@Tag(EXTENSION_FUNCTION_WITH_CLASS)
fun `should not trigger on regular functions in the same file with class`() {
lintMethod(
"""
|class Some {
| fun foo() {
|
| }
|}
|
|fun bar() {
|
|}
""".trimMargin()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test
import java.time.LocalDate

import kotlinx.serialization.encodeToString
import org.cqfn.diktat.ruleset.constants.Warnings.EXTENSION_FUNCTION_WITH_CLASS

typealias ruleToConfig = Map<Warnings, Map<String, String>>

Expand Down Expand Up @@ -166,12 +167,15 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin",
LintError(3, 6, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_TOP_LEVEL.warnText()} Example", false),
LintError(3, 26, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_CLASS_ELEMENTS.warnText()} isValid", false),
LintError(6, 9, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_CLASS_ELEMENTS.warnText()} foo", false),
LintError(8, 8, "$DIKTAT_RULE_SET_ID:extension-functions-class-file", "${EXTENSION_FUNCTION_WITH_CLASS.warnText()} fun foo", false),
LintError(8, 8, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_CLASS_ELEMENTS.warnText()} foo", false),
LintError(8, 8, "$DIKTAT_RULE_SET_ID:kdoc-methods", "${MISSING_KDOC_ON_FUNCTION.warnText()} foo", false),
LintError(9, 3, "$DIKTAT_RULE_SET_ID:empty-block-structure", EMPTY_BLOCK_STRUCTURE_ERROR.warnText() +
" empty blocks are forbidden unless it is function with override keyword", false),
LintError(12, 10, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false),
LintError(13, 6, "$DIKTAT_RULE_SET_ID:extension-functions-class-file", "${EXTENSION_FUNCTION_WITH_CLASS.warnText()} fun countSubStringOccurrences", false),
LintError(14, 3, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false),
LintError(19, 4, "$DIKTAT_RULE_SET_ID:extension-functions-class-file", "${EXTENSION_FUNCTION_WITH_CLASS.warnText()} fun splitPathToDirs", false),
LintError(19, 15, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false)
)
}
Expand Down
3 changes: 2 additions & 1 deletion info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,5 @@
| 6 | 6.1.11 | COMPACT_OBJECT_INITIALIZATION | Checks if class instantiation can be wrapped in `apply` for better readability | | no | |
| 6 | 6.2.2 | EXTENSION_FUNCTION_SAME_SIGNATURE | Checks if extension function has the same signature as another extension function and their classes are related | no | no | + |
| 6 | 6.4.1 | AVOID_USING_UTILITY_CLASS | Checks if there is class/object that can be replace with extension function | no | no | - |
| 6 | 6.4.2 | OBJECT_IS_PREFERRED | Checks: if class is stateless it is preferred to use `object` | yes | no | + |
| 6 | 6.4.2 | OBJECT_IS_PREFERRED | Checks: if class is stateless it is preferred to use `object` | yes | no | + |
| 6 | 6.2.3 | EXTENSION_FUNCTION_WITH_CLASS | Check: if file contains class, then it can not have extension functions | no | no | - |
5 changes: 3 additions & 2 deletions info/guide/guide-TOC.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ I [Preface](#c0)
* [6.1.9 Never use the name of a variable in the custom getter or setter (possible_bug)](#r6.1.9)
* [6.1.10 No trivial getters and setters are allowed in the code](#r6.1.10)
* [6.1.11 Use 'apply' for grouping object initialization](#r6.1.11)
* [6.2 Classes](#c6.2)
* [6.2 Extension functions](#c6.2)
* [6.2.1 Use extension functions for making logic of classes less coupled](#r6.2.1)
* [6.2.2 No extension functions with the same name and signature if they extend base and inheritor classes (possible_bug)](#r6.2.2)
* [6.2.2 No extension functions with the same name and signature if they extend base and inheritor classes (possible_bug)](#r6.2.2)
* [6.2.3 Don't use extension functions in the same file, where class is defined](#r6.2.3)
* [6.3 Interfaces](#c6.3)
* [6.4 Objects](#c6.4)
* [6.4.1 Instead of using utility classes/objects, use extensions](#r6.4.1)
Expand Down
14 changes: 14 additions & 0 deletions info/guide/guide-chapter-6.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,20 @@ fun printClassName(s: A) { println(s.foo()) }
fun main() { printClassName(B()) }
```

#### <a name="r6.2.3"></a> 6.2.3 Don't use extension functions in the same file, where class is defined
You should not use extension functions in the file, where class is already defined. Better to create an util file.

**Invalid example**:
```kotlin
class SomeClass {

}

fun String.deleteAllSpaces() {

}
```

<!-- =============================================================================== -->
### <a name="c6.3"></a> 6.3 Interfaces
An `Interface` in Kotlin can contain declarations of abstract methods, as well as method implementations. What makes them different from abstract classes is that interfaces cannot store state.
Expand Down

0 comments on commit 718005c

Please sign in to comment.