Skip to content

Commit

Permalink
Allow to set configuration elements as deprecated (realm#5540)
Browse files Browse the repository at this point in the history
Automatically print an appropriate warning to the console.
  • Loading branch information
SimplyDanny authored Apr 25, 2024
1 parent 04678a1 commit 5bbdf7f
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 5 deletions.
32 changes: 31 additions & 1 deletion Source/SwiftLintCore/Models/Issue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ public enum Issue: LocalizedError, Equatable {
/// The configuration didn't match internal expectations.
case invalidConfiguration(ruleID: String)

/// Issued when an option is deprecated. Suggests an alternative optionally.
case deprecatedConfigurationOption(ruleID: String, key: String, alternative: String? = nil)

/// Used in configuration parsing when no changes have been applied. Use only internally!
case nothingApplied(ruleID: String)

Expand Down Expand Up @@ -71,6 +74,23 @@ public enum Issue: LocalizedError, Equatable {
/// Flag to enable warnings for deprecations being printed to the console. Printing is enabled by default.
public static var printDeprecationWarnings = true

/// Hook used to capture all messages normally printed to stdout and return them back to the caller.
///
/// > Warning: Shall only be used in tests to verify console output.
///
/// - parameter runner: The code to run. Messages printed during the execution are collected.
///
/// - returns: The collected messages produced while running the code in the runner.
static func captureConsole(runner: () throws -> Void) rethrows -> String {
var console = ""
messageConsumer = { console += $0 }
defer { messageConsumer = nil }
try runner()
return console
}

private static var messageConsumer: ((String) -> Void)?

/// Wraps any `Error` into a `SwiftLintError.genericWarning` if it is not already a `SwiftLintError`.
///
/// - parameter error: Any `Error`.
Expand Down Expand Up @@ -102,13 +122,23 @@ public enum Issue: LocalizedError, Equatable {
if case .ruleDeprecated = self, !Self.printDeprecationWarnings {
return
}
queuedPrintError(errorDescription)
if let consumer = Self.messageConsumer {
consumer(errorDescription)
} else {
queuedPrintError(errorDescription)
}
}

private var message: String {
switch self {
case let .invalidConfiguration(id):
return "Invalid configuration for '\(id)' rule. Falling back to default."
case let .deprecatedConfigurationOption(id, key, alternative):
let baseMessage = "Configuration option '\(key)' in '\(id)' rule is deprecated."
if let alternative {
return baseMessage + " Use the option '\(alternative)' instead."
}
return baseMessage
case let .nothingApplied(ruleID: id):
return Self.invalidConfiguration(ruleID: id).message
case let .listedMultipleTime(id, times):
Expand Down
23 changes: 22 additions & 1 deletion Source/SwiftLintCore/Models/RuleConfigurationDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,18 @@ public protocol InlinableOptionType: AcceptableByConfigurationElement {}
///
@propertyWrapper
public struct ConfigurationElement<T: AcceptableByConfigurationElement & Equatable>: Equatable {
/// A deprecation notice.
public enum DeprecationNotice {
/// Warning suggesting an alternative option.
case suggestAlternative(ruleID: String, name: String)
}

/// Wrapped option value.
public var wrappedValue: T {
didSet {
if case let .suggestAlternative(id, name) = deprecationNotice {
Issue.deprecatedConfigurationOption(ruleID: id, key: key, alternative: name).print()
}
if wrappedValue != oldValue {
postprocessor(&wrappedValue)
}
Expand All @@ -437,18 +446,28 @@ public struct ConfigurationElement<T: AcceptableByConfigurationElement & Equatab
/// Whether this configuration element will be inlined into its description.
public let inline: Bool

private let deprecationNotice: DeprecationNotice?
private let postprocessor: (inout T) -> Void

/// Default constructor.
///
/// - Parameters:
/// - value: Value to be wrapped.
/// - key: Optional name of the option. If not specified, it will be inferred from the attributed property.
/// - deprecationNotice: An optional deprecation notice in case an option is outdated and/or has been replaced by
/// an alternative.
/// - postprocessor: Function to be applied to the wrapped value after parsing to validate and modify it.
public init(wrappedValue value: T,
key: String,
deprecationNotice: DeprecationNotice? = nil,
postprocessor: @escaping (inout T) -> Void = { _ in }) {
self.init(wrappedValue: value, key: key, inline: false, postprocessor: postprocessor)
self.init(
wrappedValue: value,
key: key,
inline: false,
deprecationNotice: deprecationNotice,
postprocessor: postprocessor
)

// Modify the set value immediately.
postprocessor(&wrappedValue)
Expand Down Expand Up @@ -488,10 +507,12 @@ public struct ConfigurationElement<T: AcceptableByConfigurationElement & Equatab
private init(wrappedValue: T,
key: String,
inline: Bool,
deprecationNotice: DeprecationNotice? = nil,
postprocessor: @escaping (inout T) -> Void = { _ in }) {
self.wrappedValue = wrappedValue
self.key = key
self.inline = inline
self.deprecationNotice = deprecationNotice
self.postprocessor = postprocessor
}

Expand Down
8 changes: 6 additions & 2 deletions Tests/SwiftLintFrameworkTests/IndentationWidthRuleTests.swift
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
@testable import SwiftLintBuiltInRules
@testable import SwiftLintCore
import SwiftLintTestHelpers
import XCTest

class IndentationWidthRuleTests: SwiftLintTestCase {
func testInvalidIndentation() throws {
var testee = IndentationWidthConfiguration()
let defaultValue = testee.indentationWidth
for indentation in [0, -1, -5] {
try testee.apply(configuration: ["indentation_width": indentation])

for indentation in [0, -1, -5] {
XCTAssertEqual(
try Issue.captureConsole { try testee.apply(configuration: ["indentation_width": indentation]) },
"warning: Invalid configuration for 'indentation_width' rule. Falling back to default."
)
// Value remains the default.
XCTAssertEqual(testee.indentationWidth, defaultValue)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class RuleConfigurationDescriptionTests: XCTestCase {
postprocessor: { list in list = list.map { $0.uppercased() } }
)
var list = ["string1", "string2"]
@ConfigurationElement(key: "set")
@ConfigurationElement(key: "set", deprecationNotice: .suggestAlternative(ruleID: "my_rule", name: "other_opt"))
var set: Set<Int> = [1, 2, 3]
@ConfigurationElement(inline: true)
var severityConfig = SeverityConfiguration<Parent>(.error)
Expand Down Expand Up @@ -490,6 +490,15 @@ class RuleConfigurationDescriptionTests: XCTestCase {
XCTAssertEqual(configuration.nestedSeverityLevels, SeverityLevelsConfiguration(warning: 6, error: 7))
}

func testDeprecationWarning() throws {
var configuration = TestConfiguration()

XCTAssertEqual(
try Issue.captureConsole { try configuration.apply(configuration: ["set": [6, 7]]) },
"warning: Configuration option 'set' in 'my_rule' rule is deprecated. Use the option 'other_opt' instead."
)
}

func testInvalidKeys() throws {
var configuration = TestConfiguration()

Expand Down

0 comments on commit 5bbdf7f

Please sign in to comment.