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

Add minimum_fraction_length to number_separator #1202

Merged
merged 5 commits into from
Jan 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
[Aaron McTavish](https://github.com/aamctustwo)
[#1169](https://github.com/realm/SwiftLint/issues/1169)

* Update `number_separator` rule to allow for specifying
minimum length of fraction.
[Bjarke Søndergaard](https://github.com/bjarkehs)
[#1200](https://github.com/realm/SwiftLint/issues/1200)

##### Bug Fixes

* Fix false positives on `shorthand_operator` rule.
Expand Down
22 changes: 15 additions & 7 deletions Source/SwiftLintFramework/Rules/NumberSeparatorRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Foundation
import SourceKittenFramework

public struct NumberSeparatorRule: OptInRule, CorrectableRule, ConfigurationProviderRule {
public var configuration = NumberSeparatorConfiguration(minimumLength: 0)
public var configuration = NumberSeparatorConfiguration(minimumLength: 0, minimumFractionLength: nil)

public init() {}

Expand Down Expand Up @@ -51,13 +51,13 @@ public struct NumberSeparatorRule: OptInRule, CorrectableRule, ConfigurationProv
var validFraction = true
var expectedFraction: String?
if components.count == 2, let fractionSubstring = components.last {
let result = isValid(number: fractionSubstring, reversed: false)
let result = isValid(number: fractionSubstring, isFraction: true)
validFraction = result.0
expectedFraction = result.1
}

guard let integerSubstring = components.first,
case let (valid, expected) = isValid(number: integerSubstring, reversed: true),
case let (valid, expected) = isValid(number: integerSubstring, isFraction: false),
!valid || !validFraction,
let range = file.contents.bridge().byteRangeToNSRange(start: token.offset,
length: token.length) else {
Expand Down Expand Up @@ -113,21 +113,29 @@ public struct NumberSeparatorRule: OptInRule, CorrectableRule, ConfigurationProv
return prefixes.filter { lowercased.hasPrefix($0) }.isEmpty
}

private func isValid(number: String, reversed: Bool) -> (Bool, String) {
private func isValid(number: String, isFraction: Bool) -> (Bool, String) {
var correctComponents = [String]()
let clean = number.replacingOccurrences(of: "_", with: "")
let shouldAddSeparators = clean.characters.count >= configuration.minimumLength

let minimumLength: Int
if isFraction {
minimumLength = configuration.minimumFractionLength ?? configuration.minimumLength
} else {
minimumLength = configuration.minimumLength
}

let shouldAddSeparators = clean.characters.count >= minimumLength

for (idx, char) in reversedIfNeeded(Array(clean.characters),
reversed: reversed).enumerated() {
reversed: !isFraction).enumerated() {
if idx % 3 == 0 && idx > 0 && shouldAddSeparators {
correctComponents.append("_")
}

correctComponents.append(String(char))
}

let expected = reversedIfNeeded(correctComponents, reversed: reversed).joined()
let expected = reversedIfNeeded(correctComponents, reversed: !isFraction).joined()
return (expected == number, expected)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,23 @@
public struct NumberSeparatorConfiguration: RuleConfiguration, Equatable {
private(set) var severityConfiguration = SeverityConfiguration(.warning)
private(set) var minimumLength: Int
private(set) var minimumFractionLength: Int?

public var consoleDescription: String {
return severityConfiguration.consoleDescription + ", minimum_length: \(minimumLength)"
let minimumFractionLengthDescription: String
if let minimumFractionLength = minimumFractionLength {
minimumFractionLengthDescription = ", minimum_fraction_length: \(minimumFractionLength)"
} else {
minimumFractionLengthDescription = ""
}
return severityConfiguration.consoleDescription
+ ", minimum_length: \(minimumLength)"
+ minimumFractionLengthDescription
}

public init(minimumLength: Int) {
public init(minimumLength: Int, minimumFractionLength: Int?) {
self.minimumLength = minimumLength
self.minimumFractionLength = minimumFractionLength
}

public mutating func apply(configuration: Any) throws {
Expand All @@ -27,6 +37,10 @@ public struct NumberSeparatorConfiguration: RuleConfiguration, Equatable {
self.minimumLength = minimumLength
}

if let minimumFractionLength = configuration["minimum_fraction_length"] as? Int {
self.minimumFractionLength = minimumFractionLength
}

if let severityString = configuration["severity"] as? String {
try severityConfiguration.apply(configuration: severityString)
}
Expand All @@ -35,6 +49,7 @@ public struct NumberSeparatorConfiguration: RuleConfiguration, Equatable {
public static func == (lhs: NumberSeparatorConfiguration,
rhs: NumberSeparatorConfiguration) -> Bool {
return lhs.minimumLength == rhs.minimumLength &&
lhs.minimumFractionLength == rhs.minimumFractionLength &&
lhs.severityConfiguration == rhs.severityConfiguration
}
}
9 changes: 6 additions & 3 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,10 @@
D4C4A34C1DEA4FF000E0E04C /* AttributesConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4C4A34A1DEA4FD700E0E04C /* AttributesConfiguration.swift */; };
D4C4A34E1DEA877200E0E04C /* FileHeaderRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4C4A34D1DEA877200E0E04C /* FileHeaderRule.swift */; };
D4C4A3521DEFBBB700E0E04C /* FileHeaderConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4C4A3511DEFBBB700E0E04C /* FileHeaderConfiguration.swift */; };
D4DA1DF81E175E8A0037413D /* LinterCache+CommandLine.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DA1DF71E175E8A0037413D /* LinterCache+CommandLine.swift */; };
D4CA758F1E2DEEA500A40E8A /* NumberSeparatorRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4CA758E1E2DEEA500A40E8A /* NumberSeparatorRuleTests.swift */; };
D4D5A5FF1E1F3A1C00D15E0C /* ShorthandOperatorRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4D5A5FE1E1F3A1C00D15E0C /* ShorthandOperatorRule.swift */; };
D4DA1DF41E17511D0037413D /* CompilerProtocolInitRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DA1DF31E17511D0037413D /* CompilerProtocolInitRule.swift */; };
D4DA1DF81E175E8A0037413D /* LinterCache+CommandLine.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DA1DF71E175E8A0037413D /* LinterCache+CommandLine.swift */; };
D4DA1DFA1E18D6200037413D /* LargeTupleRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DA1DF91E18D6200037413D /* LargeTupleRule.swift */; };
D4DA1DFE1E1A10DB0037413D /* NumberSeparatorConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DA1DFD1E1A10DB0037413D /* NumberSeparatorConfiguration.swift */; };
D4DAE8BC1DE14E8F00B0AE7A /* NimbleOperatorRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4DAE8BB1DE14E8F00B0AE7A /* NimbleOperatorRule.swift */; };
Expand Down Expand Up @@ -398,9 +399,10 @@
D4C4A34A1DEA4FD700E0E04C /* AttributesConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AttributesConfiguration.swift; sourceTree = "<group>"; };
D4C4A34D1DEA877200E0E04C /* FileHeaderRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileHeaderRule.swift; sourceTree = "<group>"; };
D4C4A3511DEFBBB700E0E04C /* FileHeaderConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileHeaderConfiguration.swift; sourceTree = "<group>"; };
D4DA1DF71E175E8A0037413D /* LinterCache+CommandLine.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "LinterCache+CommandLine.swift"; sourceTree = "<group>"; };
D4CA758E1E2DEEA500A40E8A /* NumberSeparatorRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NumberSeparatorRuleTests.swift; sourceTree = "<group>"; };
D4D5A5FE1E1F3A1C00D15E0C /* ShorthandOperatorRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ShorthandOperatorRule.swift; sourceTree = "<group>"; };
D4DA1DF31E17511D0037413D /* CompilerProtocolInitRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CompilerProtocolInitRule.swift; sourceTree = "<group>"; };
D4DA1DF71E175E8A0037413D /* LinterCache+CommandLine.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "LinterCache+CommandLine.swift"; sourceTree = "<group>"; };
D4DA1DF91E18D6200037413D /* LargeTupleRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LargeTupleRule.swift; sourceTree = "<group>"; };
D4DA1DFD1E1A10DB0037413D /* NumberSeparatorConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NumberSeparatorConfiguration.swift; sourceTree = "<group>"; };
D4DAE8BB1DE14E8F00B0AE7A /* NimbleOperatorRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NimbleOperatorRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -714,9 +716,9 @@
D4998DE81DF194F20006E05D /* FileHeaderRuleTests.swift */,
3B63D46C1E1F05160057BE35 /* LineLengthConfigurationTests.swift */,
3B63D46E1E1F09DF0057BE35 /* LineLengthRuleTests.swift */,
C9802F2E1E0C8AEE008AB27F /* TrailingCommaRuleTests.swift */,
E832F10C1B17E725003F265F /* IntegrationTests.swift */,
D4C27BFF1E12DFF500DF713E /* LinterCacheTests.swift */,
D4CA758E1E2DEEA500A40E8A /* NumberSeparatorRuleTests.swift */,
E86396C61BADAFE6002C9E88 /* ReporterTests.swift */,
E8BB8F9B1B17DE3B00199606 /* RulesTests.swift */,
E81224991B04F85B001783D2 /* TestHelpers.swift */,
Expand Down Expand Up @@ -1284,6 +1286,7 @@
D4998DE91DF194F20006E05D /* FileHeaderRuleTests.swift in Sources */,
006204DE1E1E4E0A00FFFBE1 /* VerticalWhitespaceRuleTests.swift in Sources */,
02FD8AEF1BFC18D60014BFFB /* ExtendedNSStringTests.swift in Sources */,
D4CA758F1E2DEEA500A40E8A /* NumberSeparatorRuleTests.swift in Sources */,
D4348EEA1C46122C007707FB /* FunctionBodyLengthRuleTests.swift in Sources */,
3B63D46D1E1F05160057BE35 /* LineLengthConfigurationTests.swift in Sources */,
6C7045441C6ADA450003F15A /* SourceKitCrashTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
Identifier = "IntegrationTests">
</Test>
<Test
Identifier = "RulesTests/testNumberSeparator()">
Identifier = "NumberSeparatorRuleTests/testNumberSeparatorWithDefaultConfiguration()">
</Test>
<Test
Identifier = "RulesTests/testTypeName()">
Expand Down
1 change: 1 addition & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ XCTMain([
testCase(LineLengthConfigurationTests.allTests),
testCase(LineLengthRuleTests.allTests),
testCase(LinterCacheTests.allTests),
testCase(NumberSeparatorRuleTests.allTests),
testCase(ReporterTests.allTests),
testCase(RuleConfigurationsTests.allTests),
testCase(RulesTests.allTests),
Expand Down
80 changes: 80 additions & 0 deletions Tests/SwiftLintFrameworkTests/NumberSeparatorRuleTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
//
// NumberSeparatorRuleTests.swift
// SwiftLint
//
// Created by Marcelo Fabri on 01/17/17.
// Copyright © 2017 Realm. All rights reserved.
//

import SwiftLintFramework
import XCTest

class NumberSeparatorRuleTests: XCTestCase {

func testNumberSeparatorWithDefaultConfiguration() {
verifyRule(NumberSeparatorRule.description)
}

func testNumberSeparatorWithMinimumLength() {
let description = RuleDescription(
identifier: NumberSeparatorRule.description.identifier,
name: NumberSeparatorRule.description.name,
description: NumberSeparatorRule.description.description,
nonTriggeringExamples: [
"let foo = 10_000",
"let foo = 1000",
"let foo = 1000.0001",
"let foo = 10_000.0001",
"let foo = 1000.000_01"
],
triggeringExamples: [
"let foo = ↓1_000",
"let foo = ↓1.000_1",
"let foo = ↓1_000.000_1"
],
corrections: [
"let foo = ↓1_000": "let foo = 1000",
"let foo = ↓1.000_1": "let foo = 1.0001",
"let foo = ↓1_000.000_1": "let foo = 1000.0001"
]
)

verifyRule(description, ruleConfiguration: ["minimum_length": 5])
}

func testNumberSeparatorWithMinimumFractionLength() {
let description = RuleDescription(
identifier: NumberSeparatorRule.description.identifier,
name: NumberSeparatorRule.description.name,
description: NumberSeparatorRule.description.description,
nonTriggeringExamples: [
"let foo = 1_000.000_000_1",
"let foo = 1.000_001",
"let foo = 100.0001",
"let foo = 1_000.000_01"
],
triggeringExamples: [
"let foo = ↓1000",
"let foo = ↓1.000_1",
"let foo = ↓1_000.000_1"
],
corrections: [
"let foo = ↓1000": "let foo = 1_000",
"let foo = ↓1.000_1": "let foo = 1.0001",
"let foo = ↓1_000.000_1": "let foo = 1_000.0001"
]
)

verifyRule(description, ruleConfiguration: ["minimum_fraction_length": 5])
}
}

extension NumberSeparatorRuleTests {
static var allTests: [(String, (NumberSeparatorRuleTests) -> () throws -> Void)] {
return [
("testNumberSeparatorWithDefaultConfiguration", testNumberSeparatorWithDefaultConfiguration),
("testNumberSeparatorWithMinimumLength", testNumberSeparatorWithMinimumLength),
("testNumberSeparatorWithMinimumFractionLength", testNumberSeparatorWithMinimumFractionLength)
]
}
}
5 changes: 0 additions & 5 deletions Tests/SwiftLintFrameworkTests/RulesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,6 @@ class RulesTests: XCTestCase {
verifyRule(NimbleOperatorRule.description)
}

func testNumberSeparator() {
verifyRule(NumberSeparatorRule.description)
}

func testObjectLiteral() {
verifyRule(ObjectLiteralRule.description)
}
Expand Down Expand Up @@ -376,7 +372,6 @@ extension RulesTests {
("testMark", testMark),
("testNesting", testNesting),
("testNimbleOperator", testNimbleOperator),
("testNumberSeparator", testNumberSeparator),
("testObjectLiteral", testObjectLiteral),
("testOpeningBrace", testOpeningBrace),
("testOperatorFunctionWhitespace", testOperatorFunctionWhitespace),
Expand Down