Skip to content

Commit

Permalink
Add prohibited_nan_comparison opt-in rule
Browse files Browse the repository at this point in the history
Fixes #2086
  • Loading branch information
marcelofabri committed Feb 8, 2020
1 parent bdede7b commit 8270227
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@
[Marcelo Fabri](https://github.com/marcelofabri)
[#2358](https://github.com/realm/SwiftLint/issues/2358)

* Add `prohibited_nan_comparison` opt-in rule to validate using `isNaN`
instead of comparing values to the `.nan` constant.
[Marcelo Fabri](https://github.com/marcelofabri)
[#2086](https://github.com/realm/SwiftLint/issues/2086)

#### Bug Fixes

* Fix `discarded_notification_center_observer` false positives when
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/MasterRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ public let masterRuleList = RuleList(rules: [
OverriddenSuperCallRule.self,
OverrideInExtensionRule.self,
PatternMatchingKeywordsRule.self,
PhohibitedNaNComparisonRule.self,
PreferSelfTypeOverTypeOfSelfRule.self,
PrefixedTopLevelConstantRule.self,
PrivateActionRule.self,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import Foundation
import SourceKittenFramework
#if canImport(SwiftSyntax)
import SwiftSyntax
#endif

public struct PhohibitedNaNComparisonRule: ConfigurationProviderRule, SyntaxRule, OptInRule, AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static let description = RuleDescription(
identifier: "prohibited_nan_comparison",
name: "Prohibited NaN Comparison",
description: "Use `isNaN` instead of comparing values to the `.nan` constant.",
kind: .lint,
minSwiftVersion: .fiveDotOne,
nonTriggeringExamples: [
Example("if number.isNaN {}"),
Example("if foo.nan == 3 {}") // allows using in non-types
],
triggeringExamples: [
Example("if number == .nan {}"),
Example("if number + 1 == .nan {}"),
Example("if number == Float.nan {}"),
Example("if .nan == number {}"),
Example("if Double.nan == number {}")
]
)

public func validate(file: SwiftLintFile) -> [StyleViolation] {
#if canImport(SwiftSyntax)
return validate(file: file, visitor: BinaryOperatorVisitor())
#else
return []
#endif
}
}

#if canImport(SwiftSyntax)
private class BinaryOperatorVisitor: SyntaxRuleVisitor {
private var positions = [AbsolutePosition]()
private let operators: Set = ["==", "!="]

func visit(_ node: BinaryOperatorExprSyntax) -> SyntaxVisitorContinueKind {
if operators.contains(node.operatorToken.withoutTrivia().text), let children = node.parent?.children {
let array = Array(children)
let before = array[array.index(before: node.indexInParent)]
let after = array[array.index(after: node.indexInParent)]

if before.isNaN || after.isNaN {
positions.append(node.operatorToken.positionAfterSkippingLeadingTrivia)
}
}

return .visitChildren
}

func violations(for rule: PhohibitedNaNComparisonRule, in file: SwiftLintFile) -> [StyleViolation] {
return positions.map { position in
StyleViolation(ruleDescription: type(of: rule).description,
severity: rule.configuration.severity,
location: Location(file: file, byteOffset: ByteCount(position.utf8Offset)))
}
}
}

private extension Syntax {
var isNaN: Bool {
guard let memberExpr = self as? MemberAccessExprSyntax else {
return false
}

let isNaNProperty = memberExpr.name.withoutTrivia().text == "nan"
return isNaNProperty && (memberExpr.base == nil || memberExpr.base?.referringToType == true)
}
}

private extension ExprSyntax {
var referringToType: Bool {
guard let expr = self as? IdentifierExprSyntax else {
return false
}

return expr.identifier.classifications.map { $0.kind } == [.identifier] &&
expr.identifier.withoutTrivia().text.isTypeLike
}
}

private extension String {
var isTypeLike: Bool {
guard let firstLetter = unicodeScalars.first else {
return false
}

return CharacterSet.uppercaseLetters.contains(firstLetter)
}
}
#endif
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@
D40AD08A1E032F9700F48C30 /* UnusedClosureParameterRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D40AD0891E032F9700F48C30 /* UnusedClosureParameterRule.swift */; };
D40E041C1F46E3B30043BC4E /* SuperfluousDisableCommandRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D40E041B1F46E3B30043BC4E /* SuperfluousDisableCommandRule.swift */; };
D40F83881DE9179200524C62 /* TrailingCommaConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D40F83871DE9179200524C62 /* TrailingCommaConfiguration.swift */; };
D40F8B6723E6D60E00A5218E /* PhohibitedNaNComparisonRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D40F8B6623E6D60E00A5218E /* PhohibitedNaNComparisonRule.swift */; };
D40FE89D1F867BFF006433E2 /* OverrideInExtensionRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D40FE89C1F867BFF006433E2 /* OverrideInExtensionRule.swift */; };
D4130D971E16183F00242361 /* IdentifierNameRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4130D961E16183F00242361 /* IdentifierNameRuleExamples.swift */; };
D4130D991E16CC1300242361 /* TypeNameRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4130D981E16CC1300242361 /* TypeNameRuleExamples.swift */; };
Expand Down Expand Up @@ -793,6 +794,7 @@
D40AD0891E032F9700F48C30 /* UnusedClosureParameterRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnusedClosureParameterRule.swift; sourceTree = "<group>"; };
D40E041B1F46E3B30043BC4E /* SuperfluousDisableCommandRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SuperfluousDisableCommandRule.swift; sourceTree = "<group>"; };
D40F83871DE9179200524C62 /* TrailingCommaConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingCommaConfiguration.swift; sourceTree = "<group>"; };
D40F8B6623E6D60E00A5218E /* PhohibitedNaNComparisonRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PhohibitedNaNComparisonRule.swift; sourceTree = "<group>"; };
D40FE89C1F867BFF006433E2 /* OverrideInExtensionRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OverrideInExtensionRule.swift; sourceTree = "<group>"; };
D4130D961E16183F00242361 /* IdentifierNameRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = IdentifierNameRuleExamples.swift; sourceTree = "<group>"; };
D4130D981E16CC1300242361 /* TypeNameRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TypeNameRuleExamples.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1203,6 +1205,7 @@
094385021D5D4F78009168CF /* PrivateOutletRule.swift */,
B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */,
D44037962132730000FDA77B /* ProhibitedInterfaceBuilderRule.swift */,
D40F8B6623E6D60E00A5218E /* PhohibitedNaNComparisonRule.swift */,
009E09271DFEE4C200B588A7 /* ProhibitedSuperRule.swift */,
623E36EF1F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift */,
623E36F11F3DB988002E5B71 /* QuickDiscouragedCallRuleExamples.swift */,
Expand Down Expand Up @@ -2172,6 +2175,7 @@
E8EA41171C2D1DBE004F9930 /* CheckstyleReporter.swift in Sources */,
006ECFC41C44E99E00EF6364 /* LegacyConstantRule.swift in Sources */,
82FE254120F604CB00295958 /* VerticalWhitespaceClosingBracesRule.swift in Sources */,
D40F8B6723E6D60E00A5218E /* PhohibitedNaNComparisonRule.swift in Sources */,
429644B61FB0A9B400D75128 /* SortedFirstLastRule.swift in Sources */,
C3EF547821B5A4000009262F /* LegacyHashingRule.swift in Sources */,
31F1B6CC1F60BF4500A57456 /* SwitchCaseAlignmentRule.swift in Sources */,
Expand Down
7 changes: 7 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1063,6 +1063,12 @@ extension PatternMatchingKeywordsRuleTests {
]
}

extension PhohibitedNaNComparisonRuleTests {
static var allTests: [(String, (PhohibitedNaNComparisonRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
]
}

extension PreferSelfTypeOverTypeOfSelfRuleTests {
static var allTests: [(String, (PreferSelfTypeOverTypeOfSelfRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
Expand Down Expand Up @@ -1801,6 +1807,7 @@ XCTMain([
testCase(OverriddenSuperCallRuleTests.allTests),
testCase(OverrideInExtensionRuleTests.allTests),
testCase(PatternMatchingKeywordsRuleTests.allTests),
testCase(PhohibitedNaNComparisonRuleTests.allTests),
testCase(PreferSelfTypeOverTypeOfSelfRuleTests.allTests),
testCase(PrefixedTopLevelConstantRuleTests.allTests),
testCase(PrivateActionRuleTests.allTests),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,12 @@ class PatternMatchingKeywordsRuleTests: XCTestCase {
}
}

class PhohibitedNaNComparisonRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(PhohibitedNaNComparisonRule.description)
}
}

class PreferSelfTypeOverTypeOfSelfRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(PreferSelfTypeOverTypeOfSelfRule.description)
Expand Down

0 comments on commit 8270227

Please sign in to comment.