Skip to content

Commit

Permalink
Avoid NSRegularExpression in configurations (#5921)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimplyDanny authored Dec 28, 2024
1 parent dd157e2 commit bef8acf
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ struct FileHeaderConfiguration: SeverityBasedRuleConfiguration {
@ConfigurationElement(key: "forbidden_pattern")
private var forbiddenPattern: String?

private var _forbiddenRegex: NSRegularExpression?
private var _requiredRegex: NSRegularExpression?
private var _forbiddenRegex: RegularExpression?
private var _requiredRegex: RegularExpression?

private static let defaultRegex = regex("\\bCopyright\\b", options: [.caseInsensitive])

Expand All @@ -37,26 +37,32 @@ struct FileHeaderConfiguration: SeverityBasedRuleConfiguration {
if let requiredString = configuration[$requiredString.key] {
self.requiredString = requiredString
if !requiredString.contains(Self.fileNamePlaceholder) {
_requiredRegex = try NSRegularExpression(pattern: requiredString,
options: Self.stringRegexOptions)
_requiredRegex = try .from(
pattern: requiredString,
options: Self.stringRegexOptions,
for: Parent.identifier
)
}
} else if let requiredPattern = configuration[$requiredPattern.key] {
self.requiredPattern = requiredPattern
if !requiredPattern.contains(Self.fileNamePlaceholder) {
_requiredRegex = try .cached(pattern: requiredPattern)
_requiredRegex = try .from(pattern: requiredPattern, for: Parent.identifier)
}
}

if let forbiddenString = configuration[$forbiddenString.key] {
self.forbiddenString = forbiddenString
if !forbiddenString.contains(Self.fileNamePlaceholder) {
_forbiddenRegex = try NSRegularExpression(pattern: forbiddenString,
options: Self.stringRegexOptions)
_forbiddenRegex = try .from(
pattern: forbiddenString,
options: Self.stringRegexOptions,
for: Parent.identifier
)
}
} else if let forbiddenPattern = configuration[$forbiddenPattern.key] {
self.forbiddenPattern = forbiddenPattern
if !forbiddenPattern.contains(Self.fileNamePlaceholder) {
_forbiddenRegex = try .cached(pattern: forbiddenPattern)
_forbiddenRegex = try .from(pattern: forbiddenPattern, for: Parent.identifier)
}
}

Expand Down Expand Up @@ -96,7 +102,7 @@ struct FileHeaderConfiguration: SeverityBasedRuleConfiguration {

func forbiddenRegex(for file: SwiftLintFile) -> NSRegularExpression? {
if _forbiddenRegex != nil {
return _forbiddenRegex
return _forbiddenRegex?.regex
}

if let regex = forbiddenString.flatMap({ regexFromString(for: file, using: $0) }) {
Expand All @@ -116,7 +122,7 @@ struct FileHeaderConfiguration: SeverityBasedRuleConfiguration {

func requiredRegex(for file: SwiftLintFile) -> NSRegularExpression? {
if _requiredRegex != nil {
return _requiredRegex
return _requiredRegex?.regex
}

if let regex = requiredString.flatMap({ regexFromString(for: file, using: $0) }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ private nonisolated(unsafe) var regexCache = [RegexCacheKey: NSRegularExpression
public struct RegularExpression: Hashable, Comparable, ExpressibleByStringLiteral, Sendable {
public let regex: NSRegularExpression

public init(pattern: String, options _: NSRegularExpression.Options? = nil) throws {
regex = try .cached(pattern: pattern)
public init(pattern: String, options: NSRegularExpression.Options? = nil) throws {
regex = try .cached(pattern: pattern, options: options)
}

public init(stringLiteral value: String) {
// swiftlint:disable:next force_try
try! self.init(pattern: value)
Expand All @@ -22,6 +23,23 @@ public struct RegularExpression: Hashable, Comparable, ExpressibleByStringLitera
public static func < (lhs: Self, rhs: Self) -> Bool {
lhs.pattern < rhs.pattern
}

/// Creates a `RegularExpression` from a pattern and options.
///
/// - Parameters:
/// - pattern: The pattern to compile into a regular expression.
/// - options: The options to use when compiling the regular expression.
/// - ruleID: The identifier of the rule that is using this regular expression in its configuration.
/// - Returns: A `RegularExpression` instance.
public static func from(pattern: String,
options: NSRegularExpression.Options? = nil,
for ruleID: String) throws -> Self {
do {
return try Self(pattern: pattern, options: options)
} catch {
throw Issue.invalidRegexPattern(ruleID: ruleID, pattern: pattern)
}
}
}

private struct RegexCacheKey: Hashable {
Expand Down
5 changes: 5 additions & 0 deletions 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, message: String? = nil)

/// Issued when a regular expression pattern is invalid.
case invalidRegexPattern(ruleID: String, pattern: String)

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

Expand Down Expand Up @@ -151,6 +154,8 @@ public enum Issue: LocalizedError, Equatable {
case let .invalidConfiguration(id, message):
let message = if let message { ": \(message)" } else { "." }
return "Invalid configuration for '\(id)' rule\(message) Falling back to default."
case let .invalidRegexPattern(id, pattern):
return "Invalid regular expression pattern '\(pattern)' used to configure '\(id)' rule."
case let .deprecatedConfigurationOption(id, key, alternative):
let baseMessage = "Configuration option '\(key)' in '\(id)' rule is deprecated."
if let alternative {
Expand Down
18 changes: 9 additions & 9 deletions Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ public struct RegexConfiguration<Parent: Rule>: SeverityBasedRuleConfiguration,
@ConfigurationElement(key: "regex")
package var regex: RegularExpression! // swiftlint:disable:this implicitly_unwrapped_optional
/// Regular expressions to include when matching the file path.
public var included: [NSRegularExpression] = []
public var included: [RegularExpression] = []
/// Regular expressions to exclude when matching the file path.
public var excluded: [NSRegularExpression] = []
public var excluded: [RegularExpression] = []
/// The syntax kinds to exclude from matches. If the regex matched syntax kinds from this list, it would
/// be ignored and not count as a rule violation.
public var excludedMatchKinds = Set<SyntaxKind>()
Expand Down Expand Up @@ -63,21 +63,21 @@ public struct RegexConfiguration<Parent: Rule>: SeverityBasedRuleConfiguration,
throw Issue.invalidConfiguration(ruleID: Parent.identifier)
}

regex = try RegularExpression(pattern: regexString)
regex = try .from(pattern: regexString, for: Parent.identifier)

if let includedString = configurationDict["included"] as? String {
included = [try .cached(pattern: includedString)]
included = [try .from(pattern: includedString, for: Parent.identifier)]
} else if let includedArray = configurationDict["included"] as? [String] {
included = try includedArray.map { pattern in
try .cached(pattern: pattern)
try .from(pattern: pattern, for: Parent.identifier)
}
}

if let excludedString = configurationDict["excluded"] as? String {
excluded = [try .cached(pattern: excludedString)]
excluded = [try .from(pattern: excludedString, for: Parent.identifier)]
} else if let excludedArray = configurationDict["excluded"] as? [String] {
excluded = try excludedArray.map { pattern in
try .cached(pattern: pattern)
try .from(pattern: pattern, for: Parent.identifier)
}
}

Expand Down Expand Up @@ -107,15 +107,15 @@ public struct RegexConfiguration<Parent: Rule>: SeverityBasedRuleConfiguration,
package func shouldValidate(filePath: String) -> Bool {
let pathRange = filePath.fullNSRange
let isIncluded = included.isEmpty || included.contains { regex in
regex.firstMatch(in: filePath, range: pathRange) != nil
regex.regex.firstMatch(in: filePath, range: pathRange) != nil
}

guard isIncluded else {
return false
}

return excluded.allSatisfy { regex in
regex.firstMatch(in: filePath, range: pathRange) == nil
regex.regex.firstMatch(in: filePath, range: pathRange) == nil
}
}

Expand Down

0 comments on commit bef8acf

Please sign in to comment.