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 1 commit
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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing two trailing whitespaces

[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
18 changes: 13 additions & 5 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, reversed: false, 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, reversed: true, isFraction: false),
!valid || !validFraction,
let range = file.contents.bridge().byteRangeToNSRange(start: token.offset,
length: token.length) else {
Expand Down Expand Up @@ -113,10 +113,18 @@ 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, reversed: Bool, isFraction: Bool) -> (Bool, String) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can remove the reversed parameter now, since it's always equivalent to !isFraction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right, for some reason my tests wouldn't pass locally even without changing anything. So I decided not to touch it.

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() {
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 = self.minimumFractionLength {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need to use self. here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, my bad. Code style from current project 😄

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
}
}