From d71af9ead88d488f9ac3518ce36bf49b66b271a1 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Wed, 16 Sep 2020 17:27:06 +0300 Subject: [PATCH 1/9] Rule 4.1.1 - operations with float/double ### What's done: * Initial implementation * Tests --- .../src/main/kotlin/generated/WarningNames.kt | 2 + .../cqfn/diktat/ruleset/constants/Warnings.kt | 2 + .../calculations/AccurateCalculationsRule.kt | 56 ++++++++ .../org/cqfn/diktat/ruleset/utils/PsiUtils.kt | 19 +++ .../chapter4/AccurateCalculationsWarnTest.kt | 124 ++++++++++++++++++ 5 files changed, 203 insertions(+) create mode 100644 diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt index 74d2c5e415..1faf4ecf9f 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -147,4 +147,6 @@ object WarningNames { const val WRONG_MULTIPLE_MODIFIERS_ORDER: String = "WRONG_MULTIPLE_MODIFIERS_ORDER" const val LOCAL_VARIABLE_EARLY_DECLARATION: String = "LOCAL_VARIABLE_EARLY_DECLARATION" + + const val FLOAT_IN_ACCURATE_CALCULATIONS: String = "FLOAT_IN_ACCURATE_CALCULATIONS" } 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 index 95c7b90de6..17f4c26ae5 100644 --- 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 @@ -91,6 +91,8 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S WRONG_DECLARATIONS_ORDER(true, "declarations of constants and enum members should be sorted alphabetically"), WRONG_MULTIPLE_MODIFIERS_ORDER(true, "sequence of modifiers is incorrect"), LOCAL_VARIABLE_EARLY_DECLARATION(false, "local variables should be declared close to the line where they are first used"), + // FixMe: change float literal to BigDecimal? Or kotlin equivalent? + FLOAT_IN_ACCURATE_CALCULATIONS(false, "floating-point values shouldn't be used in accurate calculations"), ; /** diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt new file mode 100644 index 0000000000..72ad7722c1 --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt @@ -0,0 +1,56 @@ +package org.cqfn.diktat.ruleset.rules.calculations + +import com.pinterest.ktlint.core.Rule +import com.pinterest.ktlint.core.ast.ElementType +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.Warnings.FLOAT_IN_ACCURATE_CALCULATIONS +import org.cqfn.diktat.ruleset.utils.findLocalDeclaration +import org.jetbrains.kotlin.backend.common.onlyIf +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.com.intellij.psi.PsiElement +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.KtBinaryExpression +import org.jetbrains.kotlin.psi.KtBlockExpression +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.psiUtil.parents +import org.jetbrains.kotlin.psi.psiUtil.startOffset + +/** + * Rule that checks that floating-point numbers are not used for accurate calculations + * 1. Checks that floating-point numbers are not used in arithmetic binary expressions + * Fixme: detect variables by type, not only floating-point literals + */ +class AccurateCalculationsRule(private val configRules: List) : Rule("accurate-calculations") { + companion object { + private val arithmeticOperationTokens = listOf(KtTokens.PLUS, KtTokens.PLUSEQ, KtTokens.PLUSPLUS, + KtTokens.MINUS, KtTokens.MINUSEQ, KtTokens.MINUSMINUS, + KtTokens.MUL, KtTokens.MULTEQ, KtTokens.DIV, KtTokens.DIVEQ, KtTokens.EQEQ + ) + } + + @Suppress("UnsafeCallOnNullableType") + override fun visit(node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { + (node.psi as? KtBinaryExpression)?.onlyIf({ operationToken in arithmeticOperationTokens }) { expr -> + // !! is safe because `KtBinaryExpression#left` is annotated `Nullable IfNotParsed` + val floatValue = expr.left!!.takeIf { it.isFloatingPoint() } ?: expr.right!!.takeIf { it.isFloatingPoint() } + if (floatValue != null) { + // float value is used in comparison + FLOAT_IN_ACCURATE_CALCULATIONS.warn(configRules, emit, autoCorrect, + "float value of <${floatValue.text}> used in arithmetic expression in ${expr.text}", expr.startOffset, node) + } + } + } +} + +private fun PsiElement.isFloatingPoint() = + node.elementType == ElementType.FLOAT_LITERAL || + node.elementType == ElementType.FLOAT_CONSTANT || + ((this as? KtNameReferenceExpression) + ?.findLocalDeclaration() + ?.initializer + ?.node + ?.run { elementType == ElementType.FLOAT_LITERAL || elementType == ElementType.FLOAT_CONSTANT } + ?: false) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt index f74a46228f..3f83062f0e 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt @@ -95,3 +95,22 @@ fun PsiElement.isContainingScope(block: KtBlockExpression): Boolean { } return isAncestor(block, false) } + +/** + * Method that tries to find a local property declaration with the same name as current [KtNameReferenceExpression] element + */ +fun KtNameReferenceExpression.findLocalDeclaration(): KtProperty? = parents + .mapNotNull { it as? KtBlockExpression } + .mapNotNull { blockExpression -> + blockExpression + .statements + .takeWhile { !it.isAncestor(this, true) } + .mapNotNull { it as? KtProperty } + .find { + it.isLocal && + it.hasInitializer() && + it.name?.equals(getReferencedName()) + ?: false + } + } + .firstOrNull() diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt new file mode 100644 index 0000000000..9c67105b59 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt @@ -0,0 +1,124 @@ +package org.cqfn.diktat.ruleset.chapter4 + +import com.pinterest.ktlint.core.LintError +import generated.WarningNames +import org.cqfn.diktat.ruleset.constants.Warnings.FLOAT_IN_ACCURATE_CALCULATIONS +import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID +import org.cqfn.diktat.ruleset.rules.calculations.AccurateCalculationsRule +import org.cqfn.diktat.util.LintTestBase +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class AccurateCalculationsWarnTest : LintTestBase(::AccurateCalculationsRule) { + private val ruleId = "$DIKTAT_RULE_SET_ID:accurate-calculations" + + private fun warnText(ref: String, expr: String) = "${FLOAT_IN_ACCURATE_CALCULATIONS.warnText()} float value of <$ref> used in arithmetic expression in $expr" + + @Test + @Tag(WarningNames.FLOAT_IN_ACCURATE_CALCULATIONS) + fun `should detect comparison with float literal`() { + lintMethod( + """ + |class Example { + | fun foo() { + | x == 1.0 + | 1.0 == x + | } + |} + """.trimMargin(), + LintError(3, 9, ruleId, warnText("1.0", "x == 1.0"), false), + LintError(4, 9, ruleId, warnText("1.0", "1.0 == x"), false) + ) + } + + @Test + @Tag(WarningNames.FLOAT_IN_ACCURATE_CALCULATIONS) + fun `should detect comparisons with local floating-point variables - 1`() { + lintMethod( + """ + |class Example { + | fun foo() { + | val x = 1.0 + | x == 1 + | } + |} + """.trimMargin(), + LintError(4, 9, ruleId, warnText("x", "x == 1"), false) + ) + } + + @Test + @Tag(WarningNames.FLOAT_IN_ACCURATE_CALCULATIONS) + fun `should detect comparisons with local floating-point variables - 2`() { + lintMethod( + """ + |class Example { + | fun foo() { + | val x = 1L + | list.forEach { + | val x = 1.0 + | x == 1 + | } + | } + |} + """.trimMargin(), + LintError(6, 13, ruleId, warnText("x", "x == 1"), false) + ) + } + + @Test + @Tag(WarningNames.FLOAT_IN_ACCURATE_CALCULATIONS) + fun `should detect comparisons with local floating-point variables - 3`() { + lintMethod( + """ + |class Example { + | fun foo() { + | val x = 1L + | list.forEach { + | obj.let { + | x == 1 + | } + | val x = 1.0 + | } + | } + |} + """.trimMargin() + ) + } + + @Test + @Tag(WarningNames.FLOAT_IN_ACCURATE_CALCULATIONS) + fun `should detect different operations with operators`() { + lintMethod( + """ + |class Example { + | fun foo() { + | val x = 1.0 + | x == 1 + | x + 2 + | // x++ + | x += 2 + | x - 2 + | // x-- + | x -= 2 + | x * 2 + | x *= 2 + | x / 2 + | x /= 2 + | } + |} + """.trimMargin(), + LintError(4, 9, ruleId, warnText("x", "x == 1"), false), + LintError(5, 9, ruleId, warnText("x", "x + 2"), false), +// LintError(6, 9, ruleId, warnText("x", "x++"), false), + LintError(7, 9, ruleId, warnText("x", "x += 2"), false), + LintError(8, 9, ruleId, warnText("x", "x - 2"), false), +// LintError(9, 9, ruleId, warnText("x", "x--"), false), + LintError(10, 9, ruleId, warnText("x", "x -= 2"), false), + LintError(11, 9, ruleId, warnText("x", "x * 2"), false), + LintError(12, 9, ruleId, warnText("x", "x *= 2"), false), + LintError(13, 9, ruleId, warnText("x", "x / 2"), false), + LintError(14, 9, ruleId, warnText("x", "x /= 2"), false) + ) + } +} From c7ad221b18f02d44bc39c8aab1ba6b24a30be8fb Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Thu, 17 Sep 2020 16:29:49 +0300 Subject: [PATCH 2/9] Rule 4.1.1 ### What's done: * Update yml files and available-rules.md --- diktat-analysis.yml | 4 ++++ diktat-rules/src/main/resources/diktat-analysis-huawei.yml | 4 ++++ diktat-rules/src/main/resources/diktat-analysis.yml | 4 ++++ info/available-rules.md | 1 + 4 files changed, 13 insertions(+) diff --git a/diktat-analysis.yml b/diktat-analysis.yml index e5ff59780d..cef19ecdf2 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -229,5 +229,9 @@ enabled: true configuration: {} - name: LOCAL_VARIABLE_EARLY_DECLARATION + enabled: true + configuration: {} +# Checks that floating-point values are not used in arithmetic expressions +- name: FLOAT_IN_ACCURATE_CALCULATIONS enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index 691b282e55..3d09b07d06 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -229,5 +229,9 @@ enabled: true configuration: {} - name: LOCAL_VARIABLE_EARLY_DECLARATION + enabled: true + configuration: {} +# Checks that floating-point values are not used in arithmetic expressions +- name: FLOAT_IN_ACCURATE_CALCULATIONS enabled: true configuration: {} \ No newline at end of file diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml index c3e9b55351..22e4439c91 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -233,5 +233,9 @@ configuration: {} # Inspection that checks if local variables are declared close to the first usage site - name: LOCAL_VARIABLE_EARLY_DECLARATION + enabled: true + configuration: {} +# Checks that floating-point values are not used in arithmetic expressions +- name: FLOAT_IN_ACCURATE_CALCULATIONS enabled: true configuration: {} \ No newline at end of file diff --git a/info/available-rules.md b/info/available-rules.md index 08cfc83bbf..3b63562ceb 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -71,3 +71,4 @@ | 3 | 3.2 | WRONG_DECLARATIONS_ORDER | Check: if order of enum values or constant property inside companion isn't correct | yes | - | - | | 3 | 3.11 | ANNOTATION_NEW_LINE | Check: warns if annotation not on a new single line | yes | - | - | | 3 | 3.2 | WRONG_MULTIPLE_MODIFIERS_ORDER | Check: if multiple modifiers sequence is in the wrong order | yes | - | - | +| 4 | 4.1.1 | FLOAT_IN_ACCURATE_CALCULATIONS |Checks that floating-point values are not used in arithmetic expressions
Fix: no | no | - | - | \ No newline at end of file From e56106521c20831644affc5bc8ebecb5798f3d48 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Thu, 17 Sep 2020 16:55:57 +0300 Subject: [PATCH 3/9] Rule 4.1.1 ### What's done: * Added new rule to DiktatRuleSetProvider * Added maven central badge --- README.md | 1 + .../org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt | 2 ++ 2 files changed, 3 insertions(+) diff --git a/README.md b/README.md index 6afbef4717..8198ee3144 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ ![diKTat code style](https://github.com/cqfn/diKTat/workflows/Run%20diKTat/badge.svg) [![Releases](https://img.shields.io/github/v/release/cqfn/diKTat)](https://github.com/cqfn/diKTat/releases) +![Maven Central](https://img.shields.io/maven-central/v/org.cqfn.diktat/diktat-rules) [![License](https://img.shields.io/github/license/cqfn/diKtat)](https://github.com/cqfn/diKTat/blob/master/LICENSE) [![Hits-of-Code](https://hitsofcode.com/github/cqfn/diktat)](https://hitsofcode.com/view/github/cqfn/diktat) [![codecov](https://codecov.io/gh/cqfn/diKTat/branch/master/graph/badge.svg)](https://codecov.io/gh/cqfn/diKTat) 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 index 8a1d4b4414..a277d08d57 100644 --- 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 @@ -3,6 +3,7 @@ package org.cqfn.diktat.ruleset.rules import com.pinterest.ktlint.core.RuleSet import com.pinterest.ktlint.core.RuleSetProvider import org.cqfn.diktat.common.config.rules.RulesConfigReader +import org.cqfn.diktat.ruleset.rules.calculations.AccurateCalculationsRule import org.cqfn.diktat.ruleset.rules.comments.CommentsRule import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule import org.cqfn.diktat.ruleset.rules.files.BlankLinesRule @@ -48,6 +49,7 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy ::HeaderCommentRule, ::SortRule, ::StringConcatenationRule, + ::AccurateCalculationsRule, ::LineLength, ::BlankLinesRule, ::WhiteSpaceRule, From 9952c9f31cb3154540ac97b5a8cac6c0552f074e Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Tue, 22 Sep 2020 17:18:40 +0300 Subject: [PATCH 4/9] Rule 4.1.1 ### What's done: * Fixes --- .../calculations/AccurateCalculationsRule.kt | 61 +++++++++++++++---- .../chapter4/AccurateCalculationsWarnTest.kt | 29 ++++++++- info/available-rules.md | 2 +- 3 files changed, 77 insertions(+), 15 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt index 72ad7722c1..825a721187 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt @@ -5,15 +5,14 @@ import com.pinterest.ktlint.core.ast.ElementType import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.ruleset.constants.Warnings.FLOAT_IN_ACCURATE_CALCULATIONS import org.cqfn.diktat.ruleset.utils.findLocalDeclaration -import org.jetbrains.kotlin.backend.common.onlyIf import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtBinaryExpression -import org.jetbrains.kotlin.psi.KtBlockExpression +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtDotQualifiedExpression +import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtNameReferenceExpression -import org.jetbrains.kotlin.psi.KtProperty -import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.psi.psiUtil.startOffset /** @@ -22,25 +21,63 @@ import org.jetbrains.kotlin.psi.psiUtil.startOffset * Fixme: detect variables by type, not only floating-point literals */ class AccurateCalculationsRule(private val configRules: List) : Rule("accurate-calculations") { + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) + private var isFixMode: Boolean = false + companion object { private val arithmeticOperationTokens = listOf(KtTokens.PLUS, KtTokens.PLUSEQ, KtTokens.PLUSPLUS, KtTokens.MINUS, KtTokens.MINUSEQ, KtTokens.MINUSMINUS, - KtTokens.MUL, KtTokens.MULTEQ, KtTokens.DIV, KtTokens.DIVEQ, KtTokens.EQEQ + KtTokens.MUL, KtTokens.MULTEQ, KtTokens.DIV, KtTokens.DIVEQ, + KtTokens.GT, KtTokens.LT, KtTokens.LTEQ, KtTokens.GTEQ, + KtTokens.EQEQ ) + private val arithmeticOperationsFunctions = listOf("equals", "compareTo") } @Suppress("UnsafeCallOnNullableType") override fun visit(node: ASTNode, autoCorrect: Boolean, emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { - (node.psi as? KtBinaryExpression)?.onlyIf({ operationToken in arithmeticOperationTokens }) { expr -> - // !! is safe because `KtBinaryExpression#left` is annotated `Nullable IfNotParsed` - val floatValue = expr.left!!.takeIf { it.isFloatingPoint() } ?: expr.right!!.takeIf { it.isFloatingPoint() } - if (floatValue != null) { - // float value is used in comparison - FLOAT_IN_ACCURATE_CALCULATIONS.warn(configRules, emit, autoCorrect, - "float value of <${floatValue.text}> used in arithmetic expression in ${expr.text}", expr.startOffset, node) + emitWarn = emit + isFixMode = autoCorrect + + when (val psi = node.psi) { + is KtBinaryExpression -> handleBinaryExpression(psi) + is KtDotQualifiedExpression -> handleFunction(psi) + else -> return + } + } + + private fun handleBinaryExpression(expression: KtBinaryExpression) = expression + .takeIf { it.operationToken in arithmeticOperationTokens } + ?.let { expr -> + // !! is safe because `KtBinaryExpression#left` is annotated `Nullable IfNotParsed` + val floatValue = expr.left!!.takeIf { it.isFloatingPoint() } + ?: expr.right!!.takeIf { it.isFloatingPoint() } + checkFloatValue(floatValue, expr) } + + private fun handleFunction(expression: KtDotQualifiedExpression) = expression + .takeIf { it.selectorExpression is KtCallExpression } + ?.run { receiverExpression to selectorExpression as KtCallExpression } + ?.takeIf { + (it.second.calleeExpression as? KtNameReferenceExpression) + ?.getReferencedName() in arithmeticOperationsFunctions + } + ?.let { (receiverExpression, selectorExpression) -> + val floatValue = receiverExpression.takeIf { it.isFloatingPoint() } + ?: selectorExpression + .valueArguments + .find { it.getArgumentExpression()?.isFloatingPoint() ?: false } + + checkFloatValue(floatValue, expression) + } + + private fun checkFloatValue(floatValue: PsiElement?, expression: KtExpression) { + if (floatValue != null) { + // float value is used in comparison + FLOAT_IN_ACCURATE_CALCULATIONS.warn(configRules, emitWarn, isFixMode, + "float value of <${floatValue.text}> used in arithmetic expression in ${expression.text}", expression.startOffset, expression.node) } } } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt index 9c67105b59..e4a3b8312f 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt @@ -16,18 +16,43 @@ class AccurateCalculationsWarnTest : LintTestBase(::AccurateCalculationsRule) { @Test @Tag(WarningNames.FLOAT_IN_ACCURATE_CALCULATIONS) - fun `should detect comparison with float literal`() { + fun `should detect comparison (equals) with float literal`() { lintMethod( """ |class Example { | fun foo() { | x == 1.0 | 1.0 == x + | x.equals(1.0) + | 1.0.equals(x) | } |} """.trimMargin(), LintError(3, 9, ruleId, warnText("1.0", "x == 1.0"), false), - LintError(4, 9, ruleId, warnText("1.0", "1.0 == x"), false) + LintError(4, 9, ruleId, warnText("1.0", "1.0 == x"), false), + LintError(5, 9, ruleId, warnText("1.0", "x.equals(1.0)"), false), + LintError(6, 9, ruleId, warnText("1.0", "1.0.equals(x)"), false) + ) + } + + @Test + @Tag(WarningNames.FLOAT_IN_ACCURATE_CALCULATIONS) + fun `should detect comparison with float literal`() { + lintMethod( + """ + |class Example { + | fun foo() { + | x > 1.0 + | 1.0 > x + | x.compareTo(1.0) + | 1.0.compareTo(x) + | } + |} + """.trimMargin(), + LintError(3, 9, ruleId, warnText("1.0", "x > 1.0"), false), + LintError(4, 9, ruleId, warnText("1.0", "1.0 > x"), false), + LintError(5, 9, ruleId, warnText("1.0", "x.compareTo(1.0)"), false), + LintError(6, 9, ruleId, warnText("1.0", "1.0.compareTo(x)"), false) ) } diff --git a/info/available-rules.md b/info/available-rules.md index 3b63562ceb..76e5e7cabf 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -71,4 +71,4 @@ | 3 | 3.2 | WRONG_DECLARATIONS_ORDER | Check: if order of enum values or constant property inside companion isn't correct | yes | - | - | | 3 | 3.11 | ANNOTATION_NEW_LINE | Check: warns if annotation not on a new single line | yes | - | - | | 3 | 3.2 | WRONG_MULTIPLE_MODIFIERS_ORDER | Check: if multiple modifiers sequence is in the wrong order | yes | - | - | -| 4 | 4.1.1 | FLOAT_IN_ACCURATE_CALCULATIONS |Checks that floating-point values are not used in arithmetic expressions
Fix: no | no | - | - | \ No newline at end of file +| 4 | 4.1.1 | FLOAT_IN_ACCURATE_CALCULATIONS | Checks that floating-point values are not used in arithmetic expressions
Fix: no | no | - | Current implementation detects only floating-point constants | \ No newline at end of file From e9b304819e21740c40596413c65b69baa6307ec8 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Tue, 22 Sep 2020 18:17:29 +0300 Subject: [PATCH 5/9] Rule 4.1.1 ### What's done: * Fixes --- .../ruleset/rules/calculations/AccurateCalculationsRule.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt index 825a721187..4fd33a8b64 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt @@ -34,7 +34,6 @@ class AccurateCalculationsRule(private val configRules: List) : Rul private val arithmeticOperationsFunctions = listOf("equals", "compareTo") } - @Suppress("UnsafeCallOnNullableType") override fun visit(node: ASTNode, autoCorrect: Boolean, emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { @@ -48,6 +47,7 @@ class AccurateCalculationsRule(private val configRules: List) : Rul } } + @Suppress("UnsafeCallOnNullableType") private fun handleBinaryExpression(expression: KtBinaryExpression) = expression .takeIf { it.operationToken in arithmeticOperationTokens } ?.let { expr -> From f414adcaf35fc9b429ceca4a3319376cf6c136bf Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Thu, 24 Sep 2020 18:09:34 +0300 Subject: [PATCH 6/9] Rule 4.1.2 ### What's done: * Rule 4.1.2 --- .../calculations/AccurateCalculationsRule.kt | 30 ++++++++++++++++--- .../chapter4/AccurateCalculationsWarnTest.kt | 26 ++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt index 4fd33a8b64..7e69cc0a46 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt @@ -13,11 +13,13 @@ import org.jetbrains.kotlin.psi.KtCallExpression import org.jetbrains.kotlin.psi.KtDotQualifiedExpression import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf import org.jetbrains.kotlin.psi.psiUtil.startOffset /** * Rule that checks that floating-point numbers are not used for accurate calculations * 1. Checks that floating-point numbers are not used in arithmetic binary expressions + * Exception: allows arithmetic operations only when absolute value of result is immediately used in comparison * Fixme: detect variables by type, not only floating-point literals */ class AccurateCalculationsRule(private val configRules: List) : Rule("accurate-calculations") { @@ -31,7 +33,9 @@ class AccurateCalculationsRule(private val configRules: List) : Rul KtTokens.GT, KtTokens.LT, KtTokens.LTEQ, KtTokens.GTEQ, KtTokens.EQEQ ) + private val comparisonOperators = listOf(KtTokens.LT, KtTokens.LTEQ, KtTokens.GT, KtTokens.GTEQ) private val arithmeticOperationsFunctions = listOf("equals", "compareTo") + private val comparisonFunctions = listOf("compareTo") } override fun visit(node: ASTNode, @@ -50,11 +54,13 @@ class AccurateCalculationsRule(private val configRules: List) : Rul @Suppress("UnsafeCallOnNullableType") private fun handleBinaryExpression(expression: KtBinaryExpression) = expression .takeIf { it.operationToken in arithmeticOperationTokens } - ?.let { expr -> + ?.takeUnless { it.parentsWithSelf.any(::isComparisonWithAbs) } + ?.run { // !! is safe because `KtBinaryExpression#left` is annotated `Nullable IfNotParsed` - val floatValue = expr.left!!.takeIf { it.isFloatingPoint() } - ?: expr.right!!.takeIf { it.isFloatingPoint() } - checkFloatValue(floatValue, expr) + val floatValue = left!!.takeIf { it.isFloatingPoint() } + ?: right!!.takeIf { it.isFloatingPoint() } + checkFloatValue(floatValue, this) + println() } private fun handleFunction(expression: KtDotQualifiedExpression) = expression @@ -80,6 +86,22 @@ class AccurateCalculationsRule(private val configRules: List) : Rul "float value of <${floatValue.text}> used in arithmetic expression in ${expression.text}", expression.startOffset, expression.node) } } + + private fun isComparisonWithAbs(psiElement: PsiElement): Boolean = psiElement + .takeIf { + it is KtBinaryExpression && it.operationToken in comparisonOperators + } + ?.run { + if (this is KtBinaryExpression) { + (left as? KtCallExpression ?: right as? KtCallExpression) + } else { + this as KtCallExpression + } + } + ?.run { calleeExpression as? KtNameReferenceExpression } + ?.getReferencedName() + ?.equals("abs") + ?: false } private fun PsiElement.isFloatingPoint() = diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt index e4a3b8312f..7ee2a581c7 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt @@ -146,4 +146,30 @@ class AccurateCalculationsWarnTest : LintTestBase(::AccurateCalculationsRule) { LintError(14, 9, ruleId, warnText("x", "x /= 2"), false) ) } + + @Test + @Tag(WarningNames.FLOAT_IN_ACCURATE_CALCULATIONS) + fun `should allow arithmetic operations inside abs in comparison`() { + lintMethod( + """ + |import kotlin.math.abs + | + |fun foo() { + | if (abs(1.0 - 0.999) < 1e-6) { + | println("Comparison with tolerance") + | } + | + | if (abs(1.0 - 0.999) < eps) { + | println("Comparison with tolerance using epsilon") + | } + | + | val x = 1.0 + | val y = 0.999 + | if (abs(x - y) < eps) { + | println("Comparison with tolerance using epsilon") + | } + |} + """.trimMargin() + ) + } } From f2981725bb2fa3e99263534ac99f4e0ec6f2060f Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Fri, 25 Sep 2020 16:27:20 +0300 Subject: [PATCH 7/9] Rule 4.1.2 ### What's done: * Rule 4.1.2 --- .../calculations/AccurateCalculationsRule.kt | 63 ++++++++++++++----- .../org/cqfn/diktat/ruleset/utils/PsiUtils.kt | 2 + .../chapter4/AccurateCalculationsWarnTest.kt | 14 +++-- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt index 0d5502b7b9..43dcf5feb1 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt @@ -5,6 +5,7 @@ import com.pinterest.ktlint.core.ast.ElementType import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.ruleset.constants.Warnings.FLOAT_IN_ACCURATE_CALCULATIONS import org.cqfn.diktat.ruleset.utils.findLocalDeclaration +import org.cqfn.diktat.ruleset.utils.getFunctionName import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.lexer.KtTokens @@ -67,10 +68,8 @@ class AccurateCalculationsRule(private val configRules: List) : Rul private fun handleFunction(expression: KtDotQualifiedExpression) = expression .takeIf { it.selectorExpression is KtCallExpression } ?.run { receiverExpression to selectorExpression as KtCallExpression } - ?.takeIf { - (it.second.calleeExpression as? KtNameReferenceExpression) - ?.getReferencedName() in arithmeticOperationsFunctions - } + ?.takeIf { it.second.getFunctionName() in arithmeticOperationsFunctions } + ?.takeUnless { expression.parentsWithSelf.any(::isComparisonWithAbs) } ?.let { (receiverExpression, selectorExpression) -> val floatValue = receiverExpression.takeIf { it.isFloatingPoint() } ?: selectorExpression @@ -88,24 +87,55 @@ class AccurateCalculationsRule(private val configRules: List) : Rul } } - private fun isComparisonWithAbs(psiElement: PsiElement): Boolean = psiElement - .takeIf { - it is KtBinaryExpression && it.operationToken in comparisonOperators - } - ?.run { - if (this is KtBinaryExpression) { - (left as? KtCallExpression ?: right as? KtCallExpression) - } else { - this as KtCallExpression - } + private fun isComparisonWithAbs(psiElement: PsiElement) = + when (psiElement) { + is KtBinaryExpression -> psiElement.isComparisonWithAbs() + is KtDotQualifiedExpression -> psiElement.isComparisonWithAbs() + else -> false } + + private fun KtBinaryExpression.isComparisonWithAbs() = + takeIf { it.operationToken in comparisonOperators } + ?.run { left as? KtCallExpression ?: right as? KtCallExpression } ?.run { calleeExpression as? KtNameReferenceExpression } ?.getReferencedName() ?.equals("abs") ?: false + + private fun KtDotQualifiedExpression.isComparisonWithAbs() = + takeIf { + it.selectorExpression.run { + this is KtCallExpression && getFunctionName() in comparisonFunctions + } + } + ?.run { + (selectorExpression as KtCallExpression) + .valueArguments + .singleOrNull() + ?.let { it.getArgumentExpression() as? KtCallExpression } + ?.isAbsOfFloat() + ?: false || + (receiverExpression as? KtCallExpression).isAbsOfFloat() + } + ?: false + + private fun KtCallExpression?.isAbsOfFloat() = this + ?.run { + (calleeExpression as? KtNameReferenceExpression) + ?.getReferencedName() + ?.equals("abs") + ?.and( + valueArguments + .singleOrNull() + ?.getArgumentExpression() + ?.isFloatingPoint() + ?: false) + ?: false + } + ?: false } -private fun PsiElement.isFloatingPoint() = +private fun PsiElement.isFloatingPoint(): Boolean = node.elementType == ElementType.FLOAT_LITERAL || node.elementType == ElementType.FLOAT_CONSTANT || ((this as? KtNameReferenceExpression) @@ -113,4 +143,7 @@ private fun PsiElement.isFloatingPoint() = ?.initializer ?.node ?.run { elementType == ElementType.FLOAT_LITERAL || elementType == ElementType.FLOAT_CONSTANT } + ?: false) || + ((this as? KtBinaryExpression) + ?.run { left!!.isFloatingPoint() && right!!.isFloatingPoint() } ?: false) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt index 8a5a7ff52b..2578f1aa55 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/PsiUtils.kt @@ -116,3 +116,5 @@ fun KtNameReferenceExpression.findLocalDeclaration(): KtProperty? = parents } } .firstOrNull() + +fun KtCallExpression.getFunctionName() = (calleeExpression as? KtNameReferenceExpression)?.getReferencedName() diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt index de959b27ba..00ab876369 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt @@ -163,15 +163,17 @@ class AccurateCalculationsWarnTest : LintTestBase(::AccurateCalculationsRule) { | println("Comparison with tolerance") | } | - | if (abs(1.0 - 0.999) < eps) { - | println("Comparison with tolerance using epsilon") - | } + | 1e-6 > abs(1.0 - 0.999) + | abs(1.0 - 0.999).compareTo(1e-6) < 0 + | 1e-6.compareTo(abs(1.0 - 0.999)) < 0 + | + | abs(1.0 - 0.999) < eps + | eps > abs(1.0 - 0.999) | | val x = 1.0 | val y = 0.999 - | if (abs(x - y) < eps) { - | println("Comparison with tolerance using epsilon") - | } + | abs(x - y) < eps + | eps > abs(x - y) |} """.trimMargin() ) From 6bafb0ee9ef1927046998911a6e8831e2c54f357 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Fri, 25 Sep 2020 16:51:02 +0300 Subject: [PATCH 8/9] Rule 4.1.2 ### What's done: * Code style --- .../ruleset/rules/calculations/AccurateCalculationsRule.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt index 43dcf5feb1..d810f293e0 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt @@ -135,6 +135,7 @@ class AccurateCalculationsRule(private val configRules: List) : Rul ?: false } +@Suppress("UnsafeCallOnNullableType") private fun PsiElement.isFloatingPoint(): Boolean = node.elementType == ElementType.FLOAT_LITERAL || node.elementType == ElementType.FLOAT_CONSTANT || From d09d81f824531a7ab5bc94948aab15142d3778d4 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Wed, 30 Sep 2020 19:26:29 +0300 Subject: [PATCH 9/9] Rule 4.1.2 ### What's done: * Tests with == --- .../ruleset/chapter4/AccurateCalculationsWarnTest.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt index 00ab876369..964e264207 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/AccurateCalculationsWarnTest.kt @@ -166,6 +166,7 @@ class AccurateCalculationsWarnTest : LintTestBase(::AccurateCalculationsRule) { | 1e-6 > abs(1.0 - 0.999) | abs(1.0 - 0.999).compareTo(1e-6) < 0 | 1e-6.compareTo(abs(1.0 - 0.999)) < 0 + | abs(1.0 - 0.999) == 1e-6 | | abs(1.0 - 0.999) < eps | eps > abs(1.0 - 0.999) @@ -174,8 +175,12 @@ class AccurateCalculationsWarnTest : LintTestBase(::AccurateCalculationsRule) { | val y = 0.999 | abs(x - y) < eps | eps > abs(x - y) + | abs(1.0 - 0.999) == eps |} - """.trimMargin() + """.trimMargin(), + LintError(11, 5, ruleId, warnText("1e-6", "abs(1.0 - 0.999) == 1e-6"), false), + LintError(11, 9, ruleId, warnText("1.0", "1.0 - 0.999"), false), + LintError(20, 9, ruleId, warnText("1.0", "1.0 - 0.999"), false) ) } }