Skip to content

Commit

Permalink
Add Inline test failure messages (realm#3040)
Browse files Browse the repository at this point in the history
* Add Example wrapper in order to display test failures inline when running in Xcode.
* Stop using Swift 5.1-only features so we can compile on Xcode 10.2.
* Wrap strings in Example.
* Add Changelog entry.
* Wrap all examples in Example struct.
* Better and more complete capturing of line numbers.
* Fix broken test.
* Better test traceability.
* Address or disable linting warnings.
* Add documentation comments.
* Disable linter for a few cases.
* Limit mutability and add copy-and-mutate utility functions.
* Limit scope of mutability.
  • Loading branch information
ZevEisenberg authored Feb 2, 2020
1 parent ed54fd6 commit fcf8486
Show file tree
Hide file tree
Showing 246 changed files with 6,609 additions and 5,684 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
* Add `deinitializer` type content to `type_contents_order` rule instead of
grouping it with initializers.
[Steven Magdy](https://github.com/StevenMagdy)
* Inline test failure messages to make development of SwiftLint easier. Test
failures in triggering and non-triggering examples will appear inline in
their respective files so you can immediately see which cases are working
and which are not.
[ZevEisenberg](https://github.com/ZevEisenberg)
[#3040](https://github.com/realm/SwiftLint/pull/3040)

#### Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ private func detailsSummary(_ rule: Rule) -> String {
"""
}

private func formattedCode(_ code: String) -> String {
private func formattedCode(_ example: Example) -> String {
return """
```swift
\(code)
\(example.code)
```
"""
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import Foundation
import SourceKittenFramework

/// A collection of keys and values as parsed out of SourceKit, with many conveniences for accessing SwiftLint-specific
Expand Down Expand Up @@ -261,11 +260,13 @@ extension SourceKittenDictionary {
}
}

extension Dictionary where Key == String {
extension Dictionary where Key == Example {
/// Returns a dictionary with SwiftLint violation markers (↓) removed from keys.
///
/// - returns: A new `Dictionary`.
func removingViolationMarkers() -> [Key: Value] {
return Dictionary(uniqueKeysWithValues: map { ($0.replacingOccurrences(of: "", with: ""), $1) })
return Dictionary(uniqueKeysWithValues: map { key, value in
return (key.removingViolationMarkers(), value)
})
}
}
58 changes: 58 additions & 0 deletions Source/SwiftLintFramework/Models/Example.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/// Captures code and context information for an example of a triggering or
/// non-triggering style
public struct Example {
/// The contents of the example
public private(set) var code: String
/// The path to the file where the example was created
public private(set) var file: StaticString
/// The line in the file where the example was created
public var line: UInt
}

public extension Example {
/// Create a new Example with the specified code, file, and line
/// - Parameters:
/// - code: The contents of the example
/// - file: The path to the file where the example is located.
/// Defaults to the file where this initializer is called.
/// - line: The line in the file where the example is located.
/// Defaults to the line where this initializer is called.
init(_ code: String, file: StaticString = #file, line: UInt = #line) {
self.code = code
self.file = file
self.line = line
}

/// Returns the same example, but with the `code` that is passed in
/// - Parameter code: the new code to use in the modified example
func with(code: String) -> Example {
var new = self
new.code = code
return new
}

/// Returns a copy of the Example with all instances of the "↓" character removed.
func removingViolationMarkers() -> Example {
return with(code: code.replacingOccurrences(of: "", with: ""))
}
}

extension Example: Hashable {
public static func == (lhs: Example, rhs: Example) -> Bool {
// Ignoring file/line metadata because two Examples could represent
// the same idea, but captured at two different points in the code
return lhs.code == rhs.code
}

public func hash(into hasher: inout Hasher) {
// Ignoring file/line metadata because two Examples could represent
// the same idea, but captured at two different points in the code
hasher.combine(code)
}
}

extension Example: Comparable {
public static func < (lhs: Example, rhs: Example) -> Bool {
return lhs.code < rhs.code
}
}
12 changes: 6 additions & 6 deletions Source/SwiftLintFramework/Models/RuleDescription.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/// A detailed description for a SwiftLint rule. Used for both documentation and testing purposes.
public struct RuleDescription: Equatable, Codable {
public struct RuleDescription: Equatable {
/// The rule's unique identifier, to be used in configuration files and SwiftLint commands.
/// Should be short and only comprised of lowercase latin alphabet letters and underscores formatted in snake case.
public let identifier: String
Expand All @@ -20,7 +20,7 @@ public struct RuleDescription: Equatable, Codable {
///
/// These examples are also used for testing purposes if the rule conforms to `AutomaticTestableRule`. Tests will
/// validate that the rule does not trigger any violations for these examples.
public let nonTriggeringExamples: [String]
public let nonTriggeringExamples: [Example]

/// Swift source examples that do trigger one or more violations for this rule. Used for documentation purposes to
/// inform users of various samples of code that are considered invalid by this rule. Should be valid Swift syntax
Expand All @@ -30,7 +30,7 @@ public struct RuleDescription: Equatable, Codable {
///
/// These examples are also used for testing purposes if the rule conforms to `AutomaticTestableRule`. Tests will
/// validate that the rule triggers violations for these examples wherever `↓` markers are located.
public let triggeringExamples: [String]
public let triggeringExamples: [Example]

/// Pairs of Swift source examples, where keys are examples that trigger violations for this rule, and the values
/// are the expected value after applying corrections with the rule.
Expand All @@ -39,7 +39,7 @@ public struct RuleDescription: Equatable, Codable {
///
/// These examples are used for testing purposes if the rule conforms to `AutomaticTestableRule`. Tests will
/// validate that the rule corrects all keys to their corresponding values.
public let corrections: [String: String]
public let corrections: [Example: Example]

/// Any previous iteration of the rule's identifier that was previously shipped with SwiftLint.
public let deprecatedAliases: Set<String>
Expand Down Expand Up @@ -73,8 +73,8 @@ public struct RuleDescription: Equatable, Codable {
/// - parameter requiresFileOnDisk: Sets the description's `requiresFileOnDisk` property.
public init(identifier: String, name: String, description: String, kind: RuleKind,
minSwiftVersion: SwiftVersion = .three,
nonTriggeringExamples: [String] = [], triggeringExamples: [String] = [],
corrections: [String: String] = [:],
nonTriggeringExamples: [Example] = [], triggeringExamples: [Example] = [],
corrections: [Example: Example] = [:],
deprecatedAliases: Set<String> = [],
requiresFileOnDisk: Bool = false) {
self.identifier = identifier
Expand Down
38 changes: 32 additions & 6 deletions Source/SwiftLintFramework/Models/StyleViolation.swift
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
/// A value describing an instance of Swift source code that is considered invalid by a SwiftLint rule.
public struct StyleViolation: CustomStringConvertible, Equatable, Codable {
/// The identifier of the rule that generated this violation.
public let ruleIdentifier: String

/// The description of the rule that generated this violation.
public let ruleDescription: RuleDescription
public let ruleDescription: String

/// The name of the rule that generated this violation.
public let ruleName: String

/// The severity of this violation.
public let severity: ViolationSeverity
public private(set) var severity: ViolationSeverity

/// The location of this violation.
public let location: Location
public private(set) var location: Location

/// The justification for this violation.
public let reason: String
Expand All @@ -23,11 +29,31 @@ public struct StyleViolation: CustomStringConvertible, Equatable, Codable {
/// - parameter severity: The severity of this violation.
/// - parameter location: The location of this violation.
/// - parameter reason: The justification for this violation.
public init(ruleDescription: RuleDescription, severity: ViolationSeverity = .warning,
location: Location, reason: String? = nil) {
self.ruleDescription = ruleDescription
public init(ruleDescription: RuleDescription,
severity: ViolationSeverity = .warning,
location: Location,
reason: String? = nil) {
self.ruleIdentifier = ruleDescription.identifier
self.ruleDescription = ruleDescription.description
self.ruleName = ruleDescription.name
self.severity = severity
self.location = location
self.reason = reason ?? ruleDescription.description
}

/// Returns the same violation, but with the `severity` that is passed in
/// - Parameter severity: the new severity to use in the modified violation
public func with(severity: ViolationSeverity) -> StyleViolation {
var new = self
new.severity = severity
return new
}

/// Returns the same violation, but with the `location` that is passed in
/// - Parameter location: the new location to use in the modified violation
public func with(location: Location) -> StyleViolation {
var new = self
new.location = location
return new
}
}
4 changes: 2 additions & 2 deletions Source/SwiftLintFramework/Reporters/CSVReporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ public struct CSVReporter: Reporter {
violation.location.line?.description ?? "",
violation.location.character?.description ?? "",
violation.severity.rawValue.capitalized,
violation.ruleDescription.name.escapedForCSV(),
violation.ruleName.escapedForCSV(),
violation.reason.escapedForCSV(),
violation.ruleDescription.identifier
violation.ruleIdentifier
].joined(separator: ",")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public struct CheckstyleReporter: Reporter {
let col: Int = violation.location.character ?? 0
let severity: String = violation.severity.rawValue
let reason: String = violation.reason.escapedForXML()
let identifier: String = violation.ruleDescription.identifier
let identifier: String = violation.ruleIdentifier
let source: String = "swiftlint.rules.\(identifier)".escapedForXML()
return [
"\t\t<error line=\"\(line)\" ",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public struct GitHubActionsLoggingReporter: Reporter {
"line=\(violation.location.line ?? 1),",
"col=\(violation.location.character ?? 1)::",
violation.reason,
" (\(violation.ruleDescription.identifier))"
" (\(violation.ruleIdentifier))"
].joined()
}
}
4 changes: 2 additions & 2 deletions Source/SwiftLintFramework/Reporters/JSONReporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ public struct JSONReporter: Reporter {
"line": violation.location.line ?? NSNull() as Any,
"character": violation.location.character ?? NSNull() as Any,
"severity": violation.severity.rawValue.capitalized,
"type": violation.ruleDescription.name,
"rule_id": violation.ruleDescription.identifier,
"type": violation.ruleName,
"rule_id": violation.ruleIdentifier,
"reason": violation.reason
]
}
Expand Down
4 changes: 2 additions & 2 deletions Source/SwiftLintFramework/Reporters/MarkdownReporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ public struct MarkdownReporter: Reporter {
violation.location.file?.escapedForMarkdown() ?? "",
violation.location.line?.description ?? "",
severity(for: violation.severity),
violation.ruleDescription.name.escapedForMarkdown() + ": " + violation.reason.escapedForMarkdown(),
violation.ruleDescription.identifier
violation.ruleName.escapedForMarkdown() + ": " + violation.reason.escapedForMarkdown(),
violation.ruleIdentifier
].joined(separator: " | ")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public struct SonarQubeReporter: Reporter {
private static func dictionary(for violation: StyleViolation) -> [String: Any] {
return [
"engineId": "SwiftLint",
"ruleId": violation.ruleDescription.identifier,
"ruleId": violation.ruleIdentifier,
"primaryLocation": [
"message": violation.reason,
"filePath": violation.location.relativeFile ?? "",
Expand Down
4 changes: 2 additions & 2 deletions Source/SwiftLintFramework/Reporters/XcodeReporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ public struct XcodeReporter: Reporter {
return [
"\(violation.location): ",
"\(violation.severity.rawValue): ",
"\(violation.ruleDescription.name) Violation: ",
"\(violation.ruleName) Violation: ",
violation.reason,
" (\(violation.ruleDescription.identifier))"
" (\(violation.ruleIdentifier))"
].joined()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,27 @@ public struct BlockBasedKVORule: ASTRule, ConfigurationProviderRule, AutomaticTe
description: "Prefer the new block based KVO API with keypaths when using Swift 3.2 or later.",
kind: .idiomatic,
nonTriggeringExamples: [
"""
Example("""
let observer = foo.observe(\\.value, options: [.new]) { (foo, change) in
print(change.newValue)
}
"""
""")
],
triggeringExamples: [
"""
Example("""
class Foo: NSObject {
override ↓func observeValue(forKeyPath keyPath: String?, of object: Any?,
change: [NSKeyValueChangeKey : Any]?,
context: UnsafeMutableRawPointer?) {}
}
"""
,
"""
"""),
Example("""
class Foo: NSObject {
override ↓func observeValue(forKeyPath keyPath: String?, of object: Any?,
change: Dictionary<NSKeyValueChangeKey, Any>?,
context: UnsafeMutableRawPointer?) {}
}
"""
""")
]
)

Expand Down
30 changes: 15 additions & 15 deletions Source/SwiftLintFramework/Rules/Idiomatic/ConvenienceTypeRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,47 +13,47 @@ public struct ConvenienceTypeRule: ASTRule, OptInRule, ConfigurationProviderRule
kind: .idiomatic,
minSwiftVersion: .fourDotOne,
nonTriggeringExamples: [
"""
Example("""
enum Math { // enum
public static let pi = 3.14
}
""",
"""
"""),
Example("""
// class with inheritance
class MathViewController: UIViewController {
public static let pi = 3.14
}
""",
"""
"""),
Example("""
@objc class Math: NSObject { // class visible to Obj-C
public static let pi = 3.14
}
""",
"""
"""),
Example("""
struct Math { // type with non-static declarations
public static let pi = 3.14
public let randomNumber = 2
}
""",
"class DummyClass {}"
"""),
Example("class DummyClass {}")
],
triggeringExamples: [
"""
Example("""
↓struct Math {
public static let pi = 3.14
}
""",
"""
"""),
Example("""
↓class Math {
public static let pi = 3.14
}
""",
"""
"""),
Example("""
↓struct Math {
public static let pi = 3.14
@available(*, unavailable) init() {}
}
"""
""")
]
)

Expand Down
Loading

0 comments on commit fcf8486

Please sign in to comment.