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

Add syntax to swiftlint_version to allow for specifying minimum and maximum versions #5694

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@

#### Enhancements

* Linting got up to 30% faster due to the praisworthy performance
* Add new syntax to `swiftlint_version` configuration option to allow for minimum
and maximum versions.
[alex-taffe](https://github.com/alex-taffe)

* Linting got around 30% faster due to the praisworthy performance
improvements done in the [SwiftSyntax](https://github.com/swiftlang/swift-syntax)
library.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ extension Configuration {
warningThreshold: dict[Key.warningThreshold.rawValue] as? Int,
reporter: dict[Key.reporter.rawValue] as? String ?? XcodeReporter.identifier,
cachePath: cachePath ?? dict[Key.cachePath.rawValue] as? String,
pinnedVersion: dict[Key.swiftlintVersion.rawValue].map { ($0 as? String) ?? String(describing: $0) },
versionConstraint: dict[Key.swiftlintVersion.rawValue].map { ($0 as? String) ?? String(describing: $0) },
allowZeroLintableFiles: dict[Key.allowZeroLintableFiles.rawValue] as? Bool ?? false,
strict: dict[Key.strict.rawValue] as? Bool ?? false,
baseline: dict[Key.baseline.rawValue] as? String,
Expand Down
107 changes: 98 additions & 9 deletions Source/SwiftLintCore/Models/Configuration.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// swiftlint:disable file_length
// Everything in this file makes sense to be in this file

import Foundation
import SourceKittenFramework

Expand Down Expand Up @@ -124,7 +127,7 @@ public struct Configuration {

/// Creates a `Configuration` by specifying its properties directly,
/// except that rules are still to be synthesized from rulesMode, ruleList & allRulesWrapped
/// and a check against the pinnedVersion is performed if given.
/// and a check against the version in use is performed if a constraint is configured.
///
/// - parameter rulesMode: The `RulesMode` for this configuration.
/// - parameter allRulesWrapped: The rules with their own configurations already applied.
Expand All @@ -139,7 +142,7 @@ public struct Configuration {
/// lint as having failed.
/// - parameter reporter: The identifier for the `Reporter` to use to report style violations.
/// - parameter cachePath: The location of the persisted cache to use with this configuration.
/// - parameter pinnedVersion: The SwiftLint version defined in this configuration.
/// - parameter versionConstraint: The SwiftLint version constraint defined in this configuration.
/// - parameter allowZeroLintableFiles: Allow SwiftLint to exit successfully when passed ignored or unlintable
/// files.
/// - parameter strict: Treat warnings as errors.
Expand All @@ -157,19 +160,15 @@ public struct Configuration {
warningThreshold: Int? = nil,
reporter: String? = nil,
cachePath: String? = nil,
pinnedVersion: String? = nil,
versionConstraint: String? = nil,
allowZeroLintableFiles: Bool = false,
strict: Bool = false,
baseline: String? = nil,
writeBaseline: String? = nil,
checkForUpdates: Bool = false
) {
if let pinnedVersion, pinnedVersion != Version.current.value {
queuedPrintError(
"warning: Currently running SwiftLint \(Version.current.value) but " +
"configuration specified version \(pinnedVersion)."
)
exit(2)
if let versionConstraint {
Self.compareVersion(to: versionConstraint)
}

self.init(
Expand Down Expand Up @@ -286,6 +285,96 @@ public struct Configuration {
$0.bridge().absolutePathRepresentation(rootDirectory: previousBasePath).path(relativeTo: newBasePath)
}
}

/// Compares a supplied version to the version SwiftLint is currently running with. If the version does not match,
/// or the version syntax is invalid, the method will abort execution with an error. The syntax for valid version
/// strings is as follows:
///
/// * `0.54.0`
/// * `>0.54.0`
/// * `>=0.54.0`
/// * `<0.54.0`
/// * `<=0.54.0`
///
/// - Parameter configurationVersion: The configuration version to compare against.
static func checkVersion(against versionConstraint: String) { // swiftlint:disable:this function_body_length
func showInvalidVersionStringError() -> Never {
let invalidVersionString = """
error: swiftlint_version syntax invalid.
Please specify a version as follows:
0.54.0
>0.54.0
>=0.54.0
<0.54.0
<=0.54.0
"""

queuedFatalError(invalidVersionString)
}

func compareVersionString(_ versionString: String) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if "version constraint" should be a struct that implements the comparison logic. At least this struct can be tested then.

var versionString = versionString
let comparator: (Version, Version) -> Bool
let errorDifferentiatorString: String

if versionString.starts(with: ">=") {
SimplyDanny marked this conversation as resolved.
Show resolved Hide resolved
comparator = (>=)
versionString = String(versionString.dropFirst(2))
errorDifferentiatorString = "at least"
} else if versionString.starts(with: ">") {
comparator = (>)
versionString = String(versionString.dropFirst(1))
errorDifferentiatorString = "greater than"
} else if versionString.starts(with: "<=") {
comparator = (<=)
versionString = String(versionString.dropFirst(2))
errorDifferentiatorString = "at most"
} else if versionString.starts(with: "<") {
comparator = (<)
versionString = String(versionString.dropFirst(1))
errorDifferentiatorString = "less than"
} else {
comparator = (==)
errorDifferentiatorString = "exactly"
}

// make sure the remaining string is just a version string of numeral.numeral.numeral
versionString = versionString.trimmingCharacters(in: .whitespacesAndNewlines)
guard versionString.range(of: #"^\d+\.\d+\.\d+$"#, options: .regularExpression) != nil else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels like the Version initializer should check this.

showInvalidVersionStringError()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be more specific for the error case of an invalid version number format.

}

let configVersion = Version(value: versionString)

if !comparator(Version.current, configVersion) {
queuedPrintError(
"""
warning: Currently running SwiftLint \(Version.current.value) but \
configuration specified \(errorDifferentiatorString) \(versionString).
"""
)
exit(2)
}
}

let splitVersion = versionConstraint
.trimmingCharacters(in: .whitespacesAndNewlines)
.components(separatedBy: .whitespaces)

if splitVersion.count == 2 {
// Assume the user put a space in between the operator and the version
let `operator` = splitVersion[0]
let version = splitVersion[1]
compareVersionString(`operator` + version)
} else if splitVersion.count == 1 {
// Assume we're going for a discrete version or gt/gte/lt/lte scenario
let versionString = splitVersion[0]
compareVersionString(versionString)
} else {
// The user typed in too much
showInvalidVersionStringError()
}
}
}

// MARK: - Hashable
Expand Down
Loading