-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Update legacy_constant
to support CGFloat.pi
#1205
Conversation
Resolves #1198 `Prefer CGFloat.pi to CGFloat(M_PI)`.
Generated by 🚫 danger |
@marcelofabri @jpsim Could you re-run the Travis build without cache? They're all passing for me locally. |
@aamctustwo It's failing when using the |
Current coverage is 82.16% (diff: 100%)@@ master #1205 diff @@
==========================================
Files 166 166
Lines 8255 8249 -6
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 6784 6778 -6
Misses 1471 1471
Partials 0 0
|
|
||
let pattern = "\\b(" + constants.joined(separator: "|") + ")\\b" | ||
private static let legacyConstants: [String] = { | ||
switch SwiftVersion.current { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this be refactored to return Array(legacyPatterns.keys)
?
}() | ||
|
||
public func validate(file: File) -> [StyleViolation] { | ||
let pattern = "\\b(" + LegacyConstantRule.legacyConstants.joined(separator: "|") + ")" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this was already in this way, but you can use a non-capture group here for a performance boost
@@ -245,7 +245,7 @@ extension File { | |||
typealias RangePatternTemplate = (NSRange, String, String) | |||
let matches: [RangePatternTemplate] | |||
matches = patterns.flatMap({ pattern, template -> [RangePatternTemplate] in | |||
return match(pattern: pattern).filter { range, kinds in | |||
return match(pattern: "\\b" + pattern).filter { range, kinds in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be changed on the caller?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for implementing this! 🙇
Resolves #1198
Prefer CGFloat.pi to CGFloat(M_PI)
.Summary
legacy_constant
into its own file, similar totype_name
, to allow for Swift 2 vs Swift 3 configurationsCGFloat.pi
andFloat.pi
support tolegacy_constant
CGFloat(M_PI)
legacy_constant
in addition to the regular Swift 3 ones