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

Unsigned integer literal is perhaps incorrectly marked as a MAGIC_NUMBER #1276

Merged
merged 8 commits into from
Apr 28, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ import org.cqfn.diktat.ruleset.constants.Warnings.PACKAGE_NAME_INCORRECT_PATH
import org.cqfn.diktat.ruleset.constants.Warnings.PACKAGE_NAME_INCORRECT_PREFIX
import org.cqfn.diktat.ruleset.constants.Warnings.PACKAGE_NAME_INCORRECT_SYMBOLS
import org.cqfn.diktat.ruleset.constants.Warnings.PACKAGE_NAME_MISSING
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.rules.chapter2.kdoc.CommentsFormatting
import org.cqfn.diktat.ruleset.rules.chapter2.kdoc.KdocComments
import org.cqfn.diktat.ruleset.utils.*

import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import org.cqfn.diktat.ruleset.constants.Warnings.HEADER_MISSING_OR_WRONG_COPYRI
import org.cqfn.diktat.ruleset.constants.Warnings.HEADER_NOT_BEFORE_PACKAGE
import org.cqfn.diktat.ruleset.constants.Warnings.HEADER_WRONG_FORMAT
import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_COPYRIGHT_YEAR
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.copyrightWords
import org.cqfn.diktat.ruleset.utils.findChildAfter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import org.cqfn.diktat.ruleset.constants.Warnings.COMMENT_WHITE_SPACE
import org.cqfn.diktat.ruleset.constants.Warnings.FIRST_COMMENT_NO_BLANK_LINE
import org.cqfn.diktat.ruleset.constants.Warnings.IF_ELSE_COMMENTS
import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_NEWLINES_AROUND_KDOC
import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.*

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class MagicNumberRule(configRules: List<RulesConfig>) : DiktatRule(
/**
* List of ignored numbers
*/
val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() }?.filter { it.isNumber() } ?: ignoreNumbersList
val ignoreNumbers = config["ignoreNumbers"]?.split(",")?.map { it.trim() }?.filter { it.isNumber() || it.isOtherNumberType() } ?: ignoreNumbersList

/**
* @param param parameter from config
Expand All @@ -102,13 +102,20 @@ class MagicNumberRule(configRules: List<RulesConfig>) : DiktatRule(
/**
* Check if string is number
*/
// || ((this.last().uppercase() == "L" || this.last().uppercase() == "U") && this.substring(0, this.lastIndex-1).isNumber())
Copy link
Member

Choose a reason for hiding this comment

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

Probably we can finally improve our commented_out_code rule, to ignore leading whitespaces and make trigger on such cases 😐

private fun String.isNumber() = (this.toLongOrNull() ?: this.toFloatOrNull()) != null

/**
* Check if string include a char of number type
*/
private fun String.isOtherNumberType(): Boolean = ((this.last().uppercase() == "L" || this.last().uppercase() == "U") && this.substring(0, this.lastIndex).isNumber()) ||
(this.substring(this.lastIndex - 1).uppercase() == "UL" && this.substring(0, this.lastIndex - 1).isNumber())
}

companion object {
const val IGNORE_TEST = true
const val NAME_ID = "aca-magic-number"
val ignoreNumbersList = listOf("-1", "1", "0", "2")
val ignoreNumbersList = listOf("-1", "1", "0", "2", "0U", "1U", "2U", "-1L", "0L", "1L", "2L", "0UL", "1UL", "2UL")
Copy link
Member

Choose a reason for hiding this comment

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

hope that we really compare strings in the logic

Copy link
Member Author

Choose a reason for hiding this comment

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

I had to change the logic a bit

val mapConfiguration = mapOf(
"ignoreHashCodeFunction" to true,
"ignorePropertyDeclaration" to false,
Expand Down
2 changes: 1 addition & 1 deletion diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@
# Ignore numbers from test
ignoreTest: "true"
# Ignore numbers
ignoreNumbers: "-1, 1, 0, 2"
ignoreNumbers: "-1, 1, 0, 2, 0U, 1U, 2U, -1L, 0L, 1L, 2L, 0UL, 1UL, 2UL"
# Is ignore override hashCode function
ignoreHashCodeFunction: "true"
# Is ignore property
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class MagicNumberRuleWarnTest : LintTestBase(::MagicNumberRule) {
| val b = 128L
| val e = 3.4f
| val g = 4/3
| val h = 0U
| val r = 1UL
| val f = "qwe\$\{12\}hhe"
|}
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import org.cqfn.diktat.ruleset.rules.chapter3.files.TopLevelOrderRule
import org.cqfn.diktat.util.FixTestBase

import generated.WarningNames
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@
</sourceDirs>
<args>
<arg>-Werror</arg>
<arg>-Xopt-in=kotlin.RequiresOptIn</arg>
<arg>-opt-in=kotlin.RequiresOptIn</arg>
</args>
</configuration>
<inherited>true</inherited>
Expand Down