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

DebugPrintRule #1314

Merged
merged 14 commits into from
May 31, 2022
3 changes: 3 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
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 @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -186,6 +187,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
::TrailingCommaRule,
::SingleInitRule,
::RangeConventionalRule,
::DebugPrintRule,
::CustomLabel,
::VariableGenericTypeDeclarationRule,
::LongNumericalValuesSeparatedRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Member

@orchestr7 orchestr7 May 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be we will have when by elementType?
And move logic in these branches of when to separate methods - method for console and method for print?

Sorry for this note, depends on you

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"
}
}
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 @@ -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
Expand Down
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 @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions examples/gradle-groovy-dsl/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions examples/gradle-kotlin-dsl/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions examples/maven/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | -|
Expand Down
18 changes: 18 additions & 0 deletions info/guide/guide-chapter-3.md
Original file line number Diff line number Diff line change
Expand Up @@ -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")
```