DebugPrintRule (#1314)

* added DebugPrintRule * supported detection of kotlin.io.print()\kotlin.io.println() * added detection of kotlin.js.console.*
saveourtool · May 31, 2022 · 9423868 · 9423868
1 parent 8aae73e
commit 9423868
Show file tree

Hide file tree

Showing 14 changed files with 261 additions and 3 deletions.
diff --git a/diktat-analysis.yml b/diktat-analysis.yml
@@ -516,6 +516,9 @@
   enabled: true
   configuration:
     isRangeToIgnore: false
+# Check if there is a call of print()\println() or console.log(). Assumption that it's a debug print
+- name: DEBUG_PRINT
+  enabled: true
 # Check that typealias name is in PascalCase
 - name: TYPEALIAS_NAME_INCORRECT_CASE
   enabled: true

diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt
@@ -194,6 +194,8 @@ public object WarningNames {
 
     public const val CONVENTIONAL_RANGE: String = "CONVENTIONAL_RANGE"
 
+    public const val DEBUG_PRINT: String = "DEBUG_PRINT"
+
     public const val NULLABLE_PROPERTY_TYPE: String = "NULLABLE_PROPERTY_TYPE"
 
     public const val TYPE_ALIAS: String = "TYPE_ALIAS"

diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
@@ -136,6 +136,7 @@ enum class Warnings(
     FILE_NAME_MATCH_CLASS(true, "3.1.2", "file name is incorrect - it should match with the class described in it if there is the only one class declared"),
     COLLAPSE_IF_STATEMENTS(true, "3.16.1", "avoid using redundant nested if-statements, which could be collapsed into a single one"),
     CONVENTIONAL_RANGE(true, "3.17.1", "use conventional rule for range case"),
+    DEBUG_PRINT(false, "3.18.1", "use a dedicate logging library"),
 
     // ======== chapter 4 ========
     NULLABLE_PROPERTY_TYPE(true, "4.3.1", "try to avoid use of nullable types"),

diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt
@@ -22,6 +22,7 @@ import org.cqfn.diktat.ruleset.rules.chapter3.BracesInConditionalsAndLoopsRule
 import org.cqfn.diktat.ruleset.rules.chapter3.ClassLikeStructuresOrderRule
 import org.cqfn.diktat.ruleset.rules.chapter3.CollapseIfStatementsRule
 import org.cqfn.diktat.ruleset.rules.chapter3.ConsecutiveSpacesRule
+import org.cqfn.diktat.ruleset.rules.chapter3.DebugPrintRule
 import org.cqfn.diktat.ruleset.rules.chapter3.EmptyBlock
 import org.cqfn.diktat.ruleset.rules.chapter3.EnumsSeparated
 import org.cqfn.diktat.ruleset.rules.chapter3.LineLength
@@ -186,6 +187,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
             ::TrailingCommaRule,
             ::SingleInitRule,
             ::RangeConventionalRule,
+            ::DebugPrintRule,
             ::CustomLabel,
             ::VariableGenericTypeDeclarationRule,
             ::LongNumericalValuesSeparatedRule,

diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/DebugPrintRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/DebugPrintRule.kt
@@ -0,0 +1,70 @@
+package org.cqfn.diktat.ruleset.rules.chapter3
+
+import org.cqfn.diktat.common.config.rules.RulesConfig
+import org.cqfn.diktat.ruleset.constants.Warnings
+import org.cqfn.diktat.ruleset.rules.DiktatRule
+
+import com.pinterest.ktlint.core.ast.ElementType
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet
+
+/**
+ * This rule detects `print()` or `println()`.
+ * Assumption that it's a debug logging
+ *
+ */
+class DebugPrintRule(configRules: List<RulesConfig>) : DiktatRule(
+    NAME_ID,
+    configRules,
+    listOf(Warnings.DEBUG_PRINT)
+) {
+    override fun logic(node: ASTNode) {
+        checkPrintln(node)
+        checkJsConsole(node)
+    }
+
+    // check kotlin.io.print()/kotlin.io.println()
+    private fun checkPrintln(node: ASTNode) {
+        if (node.elementType == ElementType.CALL_EXPRESSION) {
+            val referenceExpression = node.findChildByType(ElementType.REFERENCE_EXPRESSION)?.text
+            val valueArgumentList = node.findChildByType(ElementType.VALUE_ARGUMENT_LIST)
+            if (referenceExpression in setOf("print", "println") &&
+                    node.treePrev?.elementType != ElementType.DOT &&
+                    valueArgumentList?.getChildren(TokenSet.create(ElementType.VALUE_ARGUMENT))?.size?.let { it <= 1 } == true &&
+                    node.findChildByType(ElementType.LAMBDA_ARGUMENT) == null) {
+                Warnings.DEBUG_PRINT.warn(
+                    configRules, emitWarn, isFixMode,
+                    "found $referenceExpression()", node.startOffset, node,
+                )
+            }
+        }
+    }
+
+    // check kotlin.js.console.*()
+    private fun checkJsConsole(node: ASTNode) {
+        if (node.elementType == ElementType.DOT_QUALIFIED_EXPRESSION) {
+            val isConsole = node.firstChildNode.let { referenceExpression ->
+                referenceExpression.elementType == ElementType.REFERENCE_EXPRESSION &&
+                        referenceExpression.firstChildNode.let { it.elementType == ElementType.IDENTIFIER && it.text == "console" }
+            }
+            if (isConsole) {
+                val logMethod = node.lastChildNode
+                    .takeIf { it.elementType == ElementType.CALL_EXPRESSION }
+                    ?.takeIf { it.findChildByType(ElementType.LAMBDA_ARGUMENT) == null }
+                    ?.firstChildNode
+                    ?.takeIf { it.elementType == ElementType.REFERENCE_EXPRESSION }
+                    ?.text
+                if (logMethod in setOf("error", "info", "log", "warn")) {
+                    Warnings.DEBUG_PRINT.warn(
+                        configRules, emitWarn, isFixMode,
+                        "found console.$logMethod()", node.startOffset, node,
+                    )
+                }
+            }
+        }
+    }
+
+    internal companion object {
+        const val NAME_ID = "debug-print"
+    }
+}
diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml
@@ -518,6 +518,9 @@
   enabled: true
   configuration:
     isRangeToIgnore: false
+# Check if there is a call of print()\println() or console.log(). Assumption that it's a debug print
+- name: DEBUG_PRINT
+  enabled: true
 # Check that typealias name is in PascalCase
 - name: TYPEALIAS_NAME_INCORRECT_CASE
   enabled: true

diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml
@@ -514,6 +514,9 @@
   enabled: true
   configuration:
     isRangeToIgnore: false
+# Check if there is a call of print()\println() or console.log(). Assumption that it's a debug print
+- name: DEBUG_PRINT
+  enabled: true
 # Check that typealias name is in PascalCase
 - name: TYPEALIAS_NAME_INCORRECT_CASE
   enabled: true

diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/DebugPrintRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/DebugPrintRuleWarnTest.kt
@@ -0,0 +1,146 @@
+package org.cqfn.diktat.ruleset.chapter3
+
+import org.cqfn.diktat.ruleset.constants.Warnings
+import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
+import org.cqfn.diktat.ruleset.rules.chapter3.DebugPrintRule
+import org.cqfn.diktat.util.LintTestBase
+
+import com.pinterest.ktlint.core.LintError
+import generated.WarningNames
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class DebugPrintRuleWarnTest : LintTestBase(::DebugPrintRule) {
+    private val ruleId = "$DIKTAT_RULE_SET_ID:${DebugPrintRule.NAME_ID}"
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `call of print`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    print("test print")
+                |}
+            """.trimMargin(),
+            LintError(2, 5, ruleId, "${Warnings.DEBUG_PRINT.warnText()} found print()", false)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `call of println`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    println("test println")
+                |}
+            """.trimMargin(),
+            LintError(2, 5, ruleId, "${Warnings.DEBUG_PRINT.warnText()} found println()", false)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `call of println without arguments`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    println()
+                |}
+            """.trimMargin(),
+            LintError(2, 5, ruleId, "${Warnings.DEBUG_PRINT.warnText()} found println()", false)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `custom method print by argument list`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    print("test custom args", 123)
+                |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `custom method print with lambda as last parameter`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    print("test custom method") {
+                |       foo("")
+                |    }
+                |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `custom method print in another object`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    foo.print("test custom method")
+                |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `call of console`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    console.error("1")
+                |    console.info("1", "2")
+                |    console.log("1", "2", "3")
+                |    console.warn("1", "2", "3", "4")
+                |}
+            """.trimMargin(),
+            LintError(2, 5, ruleId, "${Warnings.DEBUG_PRINT.warnText()} found console.error()", false),
+            LintError(3, 5, ruleId, "${Warnings.DEBUG_PRINT.warnText()} found console.info()", false),
+            LintError(4, 5, ruleId, "${Warnings.DEBUG_PRINT.warnText()} found console.log()", false),
+            LintError(5, 5, ruleId, "${Warnings.DEBUG_PRINT.warnText()} found console.warn()", false)
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `custom method console with lambda as last parameter`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    console.error("1") {
+                |       foo("")
+                |    }
+                |    console.info("1", "2") {
+                |       foo("")
+                |    }
+                |    console.log("1", "2", "3") {
+                |       foo("")
+                |    }
+                |    console.warn("1", "2", "3", "4") {
+                |       foo("")
+                |    }
+                |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.DEBUG_PRINT)
+    fun `call parameter from console`() {
+        lintMethod(
+            """
+                |fun test() {
+                |    val foo = console.size
+                |}
+            """.trimMargin()
+        )
+    }
+}
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt
@@ -243,14 +243,14 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin",
     @Tag("DiktatRuleSetProvider")
     fun `smoke test with kts files #2`() {
         fixAndCompareSmokeTest("script/SimpleRunInScriptExpected.kts", "script/SimpleRunInScriptTest.kts")
-        Assertions.assertEquals(3, unfixedLintErrors.size)
+        Assertions.assertEquals(7, unfixedLintErrors.size)
     }
 
     @Test
     @Tag("DiktatRuleSetProvider")
     fun `smoke test with kts files with package name`() {
         fixAndCompareSmokeTest("script/PackageInScriptExpected.kts", "script/PackageInScriptTest.kts")
-        Assertions.assertEquals(3, unfixedLintErrors.size)
+        Assertions.assertEquals(7, unfixedLintErrors.size)
     }
 
     @Test

diff --git a/examples/gradle-groovy-dsl/diktat-analysis.yml b/examples/gradle-groovy-dsl/diktat-analysis.yml
@@ -514,6 +514,9 @@
   enabled: true
   configuration:
     isRangeToIgnore: false
+# Check if there is a call of print()\println() or console.log(). Assumption that it's a debug print
+- name: DEBUG_PRINT
+  enabled: true
 # Check that typealias name is in PascalCase
 - name: TYPEALIAS_NAME_INCORRECT_CASE
   enabled: true

diff --git a/examples/gradle-kotlin-dsl/diktat-analysis.yml b/examples/gradle-kotlin-dsl/diktat-analysis.yml
@@ -514,6 +514,9 @@
   enabled: true
   configuration:
     isRangeToIgnore: false
+# Check if there is a call of print()\println() or console.log(). Assumption that it's a debug print
+- name: DEBUG_PRINT
+  enabled: true
 # Check that typealias name is in PascalCase
 - name: TYPEALIAS_NAME_INCORRECT_CASE
   enabled: true

diff --git a/examples/maven/diktat-analysis.yml b/examples/maven/diktat-analysis.yml
@@ -514,6 +514,9 @@
   enabled: true
   configuration:
     isRangeToIgnore: false
+# Check if there is a call of print()\println() or console.log(). Assumption that it's a debug print
+- name: DEBUG_PRINT
+  enabled: true
 # Check that typealias name is in PascalCase
 - name: TYPEALIAS_NAME_INCORRECT_CASE
   enabled: true

diff --git a/info/available-rules.md b/info/available-rules.md
@@ -89,7 +89,8 @@
 |    3  | 3.15.2   |  STRING_TEMPLATE_QUOTES                       | Check: warns if there are redundant quotes in string template<br> Fix: deletes quotes and $ symbol                                                                                                                                  |  yes  |  no  |  + |
 |    3  | 3.16.1   |  COLLAPSE_IF_STATEMENTS                   | Check: warns if there are redundant nested if-statements, which could be collapsed into a single one by concatenating their conditions                                                                                              |  yes  |  no    |   -  |
 |    3  | 3.16.2   |  COMPLEX_BOOLEAN_EXPRESSION                   | Check: warns if boolean expression is complex and can be simplified.<br>Fix: replaces boolean expression with the simpler one                                                                                                       |  yes  |  no    |   +  |
-|    3  | 3.17     |  CONVENTIONAL_RANGE                           | Check: warns if possible to replace range with until or replace `rangeTo` function with range.<br>Fix: replace range with until or replace `rangeTo` function with range                                                            |  yes  |  no    |   -  |
+|    3  | 3.17.1   |  CONVENTIONAL_RANGE                           | Check: warns if possible to replace range with until or replace `rangeTo` function with range.<br>Fix: replace range with until or replace `rangeTo` function with range                                                            |  yes  |  no    |   -  |
+| 3 | 3.18.1 | DEBUG_PRINT | Check: warns if there is a call of print()\println() or console.log(). Assumption that it's a debug | no | | - |
 |    4  | 4.2.1    |  SMART_CAST_NEEDED                      | Check: warns if casting can be omitted<br>Fix: Deletes casting                                                                                                                                                                      |  yes  |  no  |  -  | |
 |    4  | 4.3.1    |  NULLABLE_PROPERTY_TYPE                   | Check: warns if immutable property is initialized with null or if immutable property can have non-nullable type instead of nullable<br>Fix: suggests initial value instead of null or changes immutable property type               |  yes  |  no  |  - |
 |    4  | 4.2.2    |  TYPE_ALIAS                               | Check: if type reference of property is longer than expected                                                                                                                                                                        |  yes  |  typeReferenceLength        |                                                                                      -|

diff --git a/info/guide/guide-chapter-3.md b/info/guide/guide-chapter-3.md
@@ -1043,4 +1043,22 @@ if (condition1 && condition2) {
 if (condition1 && condition2 && condition1) {
     foo()
 }
+```
+
+<!-- =============================================================================== -->
+### <a name="c3.18"></a> 3.18 Logging
+This section describes the general rules of logging.
+
+#### <a name="r3.18.1"></a> 3.18.1 Debug logging
+The dedicated logging library should be used for logging purposes.
+Need to try to avoid the following statements (assumption that it's a debug logging):
+```kotlin
+print("I'M HERE")
+println("test")
+```
+
+Additionaly, need to avoid the following statements on Kotlin JS:
+```kotlin
+console.info("info test")
+console.log("test")
 ```