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

Inline test failure messages #3040

Merged
merged 28 commits into from
Feb 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
77ddb0b
Add Example wrapper in order to display test failures inline when run…
ZevEisenberg Feb 1, 2020
16ae3e4
Stop using Swift 5.1-only features so we can compile on Xcode 10.2.
ZevEisenberg Feb 1, 2020
c8ae950
Add workaround to make test valid.
ZevEisenberg Feb 1, 2020
d450701
Wrap strings in Example.
ZevEisenberg Feb 1, 2020
4cb9cf3
Add Changelog entry.
ZevEisenberg Feb 1, 2020
3349b4d
Delete typo'd quote mark.
ZevEisenberg Feb 1, 2020
ef7c1f0
Cleanup.
ZevEisenberg Feb 1, 2020
53d3eb1
Fix incorrect examples.
ZevEisenberg Feb 1, 2020
4d98c0b
Fix incorrect quote.
ZevEisenberg Feb 1, 2020
b1d7df1
Fix incorrect use of description in interpolated string.
ZevEisenberg Feb 1, 2020
e9ca45a
Add comment.
ZevEisenberg Feb 1, 2020
e9401c2
Wrap all examples in Example struct.
ZevEisenberg Feb 1, 2020
c2b5a84
Better and more complete capturing of line numbers.
ZevEisenberg Feb 1, 2020
9b6c100
Fix broken test.
ZevEisenberg Feb 1, 2020
ee82639
Better test traceability.
ZevEisenberg Feb 1, 2020
dd45e24
Fix typo.
ZevEisenberg Feb 1, 2020
312fd3a
Address or disable linting warnings.
ZevEisenberg Feb 1, 2020
e165d96
Fix failing tests.
ZevEisenberg Feb 1, 2020
49eec02
Simplify.
ZevEisenberg Feb 1, 2020
27e2e31
Remove unused import.
ZevEisenberg Feb 1, 2020
a474d26
Add documentation comments.
ZevEisenberg Feb 1, 2020
4b9482c
Fix line lengths.
ZevEisenberg Feb 1, 2020
6cd264c
Remove requirement that Example be Codable.
ZevEisenberg Feb 1, 2020
44a7ebe
Remove optionals now that we do not need Codable conformance.
ZevEisenberg Feb 1, 2020
80f97c8
Disable linter for a few cases.
ZevEisenberg Feb 1, 2020
dace1f2
Fix typo.
ZevEisenberg Feb 2, 2020
0ec30db
Limit mutability and add copy-and-mutate utility functions.
ZevEisenberg Feb 2, 2020
f1f613b
Limit scope of mutability.
ZevEisenberg Feb 2, 2020
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
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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@ZevEisenberg ZevEisenberg Jan 17, 2020

Choose a reason for hiding this comment

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

I'm confused. That page says:

  1. A list of Markdown hyperlinks to the issues the change addresses. One entry per line. Usually just one. If there was no issue tracking this change, you may instead link to the change's pull request.

Many other changelog entries link to the issue or PR. Why drop this line?


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