UnderscoresInNumericLiterals: Allow numbers with non standard groupin…

…gs (#4280)

* Allow numbers without groups of 3

* add allowNonStandardGrouping config property

* Update detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnderscoresInNumericLiterals.kt

Co-authored-by: Brais Gabín <braisgabin@gmail.com>

Co-authored-by: Markus Schwarz <post@markus-schwarz.net>
Co-authored-by: Brais Gabín <braisgabin@gmail.com>

detekt-core/src/main/resources/default-detekt-config.yml

@@ -747,6 +747,7 @@ style:
   UnderscoresInNumericLiterals:
     active: false
     acceptableLength: 4
+    allowNonStandardGrouping: false
   UnnecessaryAbstractClass:
     active: true
   UnnecessaryAnnotationUseSiteTarget:

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnderscoresInNumericLiterals.kt

@@ -20,8 +20,7 @@ import java.util.Locale
 
 /**
  * This rule detects and reports long base 10 numbers which should be separated with underscores
- * for readability. Underscores that do not make groups of 3 digits are also reported even if their length is
- * under the configured `acceptableLength`. For `Serializable` classes or objects, the field `serialVersionUID` is
+ * for readability. For `Serializable` classes or objects, the field `serialVersionUID` is
  * explicitly ignored. For floats and doubles, anything to the right of the decimal point is ignored.
  *
  * <noncompliant>
@@ -38,8 +37,7 @@ class UnderscoresInNumericLiterals(config: Config = Config.empty) : Rule(config)
         javaClass.simpleName,
         Severity.Style,
         "Report missing or invalid underscores in base 10 numbers. Numeric literals " +
-            "should be underscore separated to increase readability. Underscores that do not make groups of " +
-            "3 digits are also reported.",
+            "should be underscore separated to increase readability.",
         Debt.FIVE_MINS
     )
 
@@ -49,31 +47,37 @@ class UnderscoresInNumericLiterals(config: Config = Config.empty) : Rule(config)
 
     @Suppress("DEPRECATION")
     @OptIn(UnstableApi::class)
-    @Configuration("Maximum number of digits that a number can have and not use underscores")
+    @Configuration("Maximum number of consecutive digits that a numeric literal can have without using an underscore")
     private val acceptableLength: Int by configWithFallback(::acceptableDecimalLength, 4)
 
+    @Configuration("If set to false, groups of exactly three digits must be used. If set to true, 100_00 is allowed.")
+    private val allowNonStandardGrouping: Boolean by config(false)
+
+    private val nonCompliantRegex: Regex = """\d{${acceptableLength + 1},}""".toRegex()
+
     override fun visitConstantExpression(expression: KtConstantExpression) {
         val normalizedText = normalizeForMatching(expression.text)
+        checkNormalized(normalizedText, expression)
+    }
 
-        if (isNotDecimalNumber(normalizedText) || expression.isSerialUidProperty()) {
+    private fun checkNormalized(normalizedText: String, expression: KtConstantExpression) {
+        if (isNotDecimalNumber(normalizedText)) {
+            return
+        }
+        if (expression.isSerialUidProperty()) {
             return
         }
 
         val numberString = normalizedText.split('.').first()
 
-        if (numberString.length > acceptableLength || numberString.contains('_')) {
-            reportIfInvalidUnderscorePattern(expression, numberString)
+        if (!allowNonStandardGrouping && numberString.hasNonStandardGrouping()) {
+            return doReport(expression, "The number contains a non standard grouping.")
         }
-    }
 
-    private fun reportIfInvalidUnderscorePattern(expression: KtConstantExpression, numberString: String) {
-        if (!numberString.matches(UNDERSCORE_NUMBER_REGEX)) {
-            report(
-                CodeSmell(
-                    issue,
-                    Entity.from(expression),
-                    "This number should be separated by underscores in order to increase readability."
-                )
+        if (numberString.contains(nonCompliantRegex)) {
+            return doReport(
+                expression,
+                "This number should be separated by underscores in order to increase readability."
             )
         }
     }
@@ -107,8 +111,14 @@ class UnderscoresInNumericLiterals(config: Config = Config.empty) : Rule(config)
             .removeSuffix("f")
     }
 
+    private fun doReport(expression: KtConstantExpression, message: String) {
+        report(CodeSmell(issue, Entity.from(expression), message))
+    }
+
+    private fun String.hasNonStandardGrouping(): Boolean = contains('_') && !matches(HAS_ONLY_STANDARD_GROUPING)
+
     companion object {
-        private val UNDERSCORE_NUMBER_REGEX = Regex("[0-9]{1,3}(_[0-9]{3})*")
+        private val HAS_ONLY_STANDARD_GROUPING = """\d{1,3}(?:_\d{3})*""".toRegex()
         private const val HEX_PREFIX = "0x"
         private const val BIN_PREFIX = "0b"
         private const val SERIALIZABLE = "Serializable"

detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnderscoresInNumericLiteralsSpec.kt

@@ -9,6 +9,7 @@ import org.spekframework.spek2.style.specification.describe
 
 private const val ACCEPTABLE_DECIMAL_LENGTH = "acceptableDecimalLength"
 private const val ACCEPTABLE_LENGTH = "acceptableLength"
+private const val ALLOW_NON_STANDARD_GROUPING = "allowNonStandardGrouping"
 
 class UnderscoresInNumericLiteralsSpec : Spek({
 
@@ -160,20 +161,27 @@ class UnderscoresInNumericLiteralsSpec : Spek({
         }
     }
 
-    describe("an Int of 10_00_00") {
-        val code = "val myInt = 10_00_00"
+    describe("an Int of 1000_00_00") {
+        val code = "val myInt = 1000_00_00"
 
         it("should be reported by default") {
             val findings = UnderscoresInNumericLiterals().compileAndLint(code)
             assertThat(findings).isNotEmpty
         }
 
-        it("should still be reported even if acceptableLength is 6") {
+        it("should still be reported even if acceptableLength is 99") {
             val findings = UnderscoresInNumericLiterals(
-                TestConfig(mapOf(ACCEPTABLE_LENGTH to "6"))
+                TestConfig(mapOf(ACCEPTABLE_LENGTH to "99"))
             ).compileAndLint(code)
             assertThat(findings).isNotEmpty
         }
+
+        it("should not be reported if allowNonStandardGrouping is true") {
+            val findings = UnderscoresInNumericLiterals(
+                TestConfig(mapOf(ALLOW_NON_STANDARD_GROUPING to true))
+            ).compileAndLint(code)
+            assertThat(findings).isEmpty()
+        }
     }
 
     describe("a binary Int of 0b1011") {
@@ -360,4 +368,13 @@ class UnderscoresInNumericLiteralsSpec : Spek({
             assertThat(findings).isEmpty()
         }
     }
+
+    describe("a String of 1000000.3141592") {
+        val code = """val myString = "1000000.3141592""""
+
+        it("should not be reported by default") {
+            val findings = UnderscoresInNumericLiterals().compileAndLint(code)
+            assertThat(findings).isEmpty()
+        }
+    }
 })