dnrops
/
SwiftLint
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Let `number_separator` rule trigger on misplaced separators (#4685)

This commit is contained in:
Danny Mösch 2023-02-06 22:22:31 +01:00 committed by GitHub
parent 60d0dd8a05
commit d3ebfc5567
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 95 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG.md
View File

@ -42,6 +42,10 @@
[David Steinacher](https://github.com/stonko1994)
[#4626](https://github.com/realm/SwiftLint/issues/4626)
* Let `number_separator` rule trigger on misplaced separators, e.g. `10_00`.
[SimplyDanny](https://github.com/SimplyDanny)
[#4637](https://github.com/realm/SwiftLint/issues/4637)
#### Bug Fixes
* Report violations in all `<scope>_length` rules when the error threshold is

56
Source/SwiftLintFramework/Rules/Style/NumberSeparatorRule.swift
View File

@ -13,13 +13,24 @@ struct NumberSeparatorRule: OptInRule, SwiftSyntaxCorrectableRule, Configuration
static let description = RuleDescription(
identifier: "number_separator",
name: "Number Separator",
description: "Underscores should be used as thousand separator in large decimal numbers",
description: """
Underscores should be used as thousand separator in large numbers with a configurable number of digits. In \
other words, there should be an underscore after every 3 digits in the integral as well as the fractional \
part of a number.
""",
kind: .style,
nonTriggeringExamples: NumberSeparatorRuleExamples.nonTriggeringExamples,
triggeringExamples: NumberSeparatorRuleExamples.triggeringExamples,
corrections: NumberSeparatorRuleExamples.corrections
)
static let missingSeparatorsReason = """
Underscores should be used as thousand separators
"""
static let misplacedSeparatorsReason = """
Underscore(s) used as thousand separator(s) should be added after every 3 digits only
"""
func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
Visitor(configuration: configuration)
}
@ -44,13 +55,13 @@ private extension NumberSeparatorRule {
override func visitPost(_ node: FloatLiteralExprSyntax) {
if let violation = violation(token: node.floatingDigits) {
violations.append(violation.position)
violations.append(ReasonedRuleViolation(position: violation.position, reason: violation.reason))
}
}
override func visitPost(_ node: IntegerLiteralExprSyntax) {
if let violation = violation(token: node.digits) {
violations.append(violation.position)
violations.append(ReasonedRuleViolation(position: violation.position, reason: violation.reason))
}
}
}
@ -101,8 +112,34 @@ private protocol NumberSeparatorValidator {
var configuration: NumberSeparatorConfiguration { get }
}
extension NumberSeparatorValidator {
func violation(token: TokenSyntax) -> (position: AbsolutePosition, correction: String)? {
private enum NumberSeparatorViolation {
case missingSeparator(position: AbsolutePosition, correction: String)
case misplacedSeparator(position: AbsolutePosition, correction: String)
var reason: String {
switch self {
case .missingSeparator: return NumberSeparatorRule.missingSeparatorsReason
case .misplacedSeparator: return NumberSeparatorRule.misplacedSeparatorsReason
}
}
var position: AbsolutePosition {
switch self {
case let .missingSeparator(position, _): return position
case let .misplacedSeparator(position, _): return position
}
}
var correction: String {
switch self {
case let .missingSeparator(_, correction): return correction
case let .misplacedSeparator(_, correction): return correction
}
}
}
private extension NumberSeparatorValidator {
func violation(token: TokenSyntax) -> NumberSeparatorViolation? {
let content = token.withoutTrivia().text
guard isDecimal(number: content),
!isInValidRanges(number: content)
@ -121,9 +158,7 @@ extension NumberSeparatorValidator {
var validFraction = true
var expectedFraction: String?
if components.count == 2, let fractionSubstring = components.last {
let result = isValid(number: fractionSubstring, isFraction: true)
validFraction = result.0
expectedFraction = result.1
(validFraction, expectedFraction) = isValid(number: fractionSubstring, isFraction: true)
}
guard let integerSubstring = components.first,
@ -143,7 +178,10 @@ extension NumberSeparatorValidator {
corrected += exponentialSymbol + exponential
}
return (token.positionAfterSkippingLeadingTrivia, corrected)
if content.contains("_") {
return .misplacedSeparator(position: token.positionAfterSkippingLeadingTrivia, correction: corrected)
}
return .missingSeparator(position: token.positionAfterSkippingLeadingTrivia, correction: corrected)
}
private func isDecimal(number: String) -> Bool {

44
Tests/SwiftLintFrameworkTests/NumberSeparatorRuleTests.swift
View File

@ -1,4 +1,5 @@
@testable import SwiftLintFramework
import SwiftParser
import XCTest
class NumberSeparatorRuleTests: XCTestCase {
@ -97,4 +98,47 @@ class NumberSeparatorRuleTests: XCTestCase {
]
)
}
func testSpecificViolationReasons() {
XCTAssertEqual(
violations(in: "1_000"),
[]
)
XCTAssertEqual(
violations(in: "1000"),
[NumberSeparatorRule.missingSeparatorsReason]
)
XCTAssertEqual(
violations(in: "1.000000", config: ["minimum_fraction_length": 5]),
[NumberSeparatorRule.missingSeparatorsReason]
)
XCTAssertEqual(
violations(in: "10_00"),
[NumberSeparatorRule.misplacedSeparatorsReason]
)
XCTAssertEqual(
violations(in: "1_000_0"),
[NumberSeparatorRule.misplacedSeparatorsReason]
)
XCTAssertEqual(
violations(in: "1000.0_00"),
[NumberSeparatorRule.misplacedSeparatorsReason]
)
XCTAssertEqual(
violations(in: "10_00", config: ["minimum_length": 5]),
[NumberSeparatorRule.misplacedSeparatorsReason]
)
XCTAssertEqual(
violations(in: "1000.0_00", config: ["minimum_fraction_length": 5]),
[NumberSeparatorRule.misplacedSeparatorsReason]
)
}
private func violations(in code: String, config: Any = []) -> [String] {
var rule = NumberSeparatorRule()
try? rule.configuration.apply(configuration: config)
let visitor = rule.makeVisitor(file: SwiftLintFile(contents: ""))
visitor.walk(Parser.parse(source: "let a = " + code))
return visitor.violations.compactMap(\.reason)
}
}