From c344f0b7ad6e86c4631c46508f5329554fe33b03 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Sat, 21 Jun 2025 14:56:04 -0400 Subject: [PATCH 1/3] Add `RegexConfiguration.ExecutionMode` To help migrate custom rules to SwiftSyntax. Not wired up yet, just the configuration parsing and defaults. Will wire it up in the next PR. The diff looks big, but it's 500+ lines of tests, with ~45 lines of actually new code. --- .../RegexConfiguration.swift | 17 + .../Rules/CustomRules.swift | 33 +- Tests/FrameworkTests/CustomRulesTests.swift | 517 ++++++++++++++++++ 3 files changed, 562 insertions(+), 5 deletions(-) diff --git a/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift b/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift index 6329261b45..b037f33773 100644 --- a/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift +++ b/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift @@ -4,6 +4,11 @@ import SourceKittenFramework /// A rule configuration used for defining custom rules in yaml. public struct RegexConfiguration: SeverityBasedRuleConfiguration, Hashable, CacheDescriptionProvider, InlinableOptionType { + /// The execution mode for this custom rule. + public enum ExecutionMode: String, Codable, Sendable { + case swiftsyntax + case sourcekit + } /// The identifier for this custom rule. public let identifier: String /// The name for this custom rule. @@ -24,6 +29,8 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, public var severityConfiguration = SeverityConfiguration(.warning) /// The index of the regex capture group to match. public var captureGroup = 0 + /// The execution mode for this rule (nil means use global default). + public var executionMode: ExecutionMode? public var cacheDescription: String { let jsonObject: [String] = [ @@ -36,6 +43,7 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, SyntaxKind.allKinds.subtracting(excludedMatchKinds) .map(\.rawValue).sorted(by: <).joined(separator: ","), severity.rawValue, + executionMode?.rawValue ?? "", ] if let jsonData = try? JSONSerialization.data(withJSONObject: jsonObject), let jsonString = String(data: jsonData, encoding: .utf8) { @@ -57,6 +65,7 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, self.identifier = identifier } + // swiftlint:disable:next cyclomatic_complexity public mutating func apply(configuration: Any) throws { guard let configurationDict = configuration as? [String: Any], let regexString = configurationDict[$regex.key] as? String else { @@ -97,11 +106,19 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, self.captureGroup = captureGroup } + if let modeString = configurationDict["mode"] as? String { + guard let mode = ExecutionMode(rawValue: modeString) else { + throw Issue.invalidConfiguration(ruleID: Parent.identifier) + } + self.executionMode = mode + } + self.excludedMatchKinds = try self.excludedMatchKinds(from: configurationDict) } public func hash(into hasher: inout Hasher) { hasher.combine(identifier) + hasher.combine(executionMode) } package func shouldValidate(filePath: String) -> Bool { diff --git a/Source/SwiftLintFramework/Rules/CustomRules.swift b/Source/SwiftLintFramework/Rules/CustomRules.swift index 3b9d9005dc..e13f7cf417 100644 --- a/Source/SwiftLintFramework/Rules/CustomRules.swift +++ b/Source/SwiftLintFramework/Rules/CustomRules.swift @@ -7,23 +7,42 @@ struct CustomRulesConfiguration: RuleConfiguration, CacheDescriptionProvider { var parameterDescription: RuleConfigurationDescription? { RuleConfigurationOption.noOptions } var cacheDescription: String { - customRuleConfigurations + let configsDescription = customRuleConfigurations .sorted { $0.identifier < $1.identifier } .map(\.cacheDescription) .joined(separator: "\n") + + if let defaultMode = defaultExecutionMode { + return "default_execution_mode:\(defaultMode.rawValue)\n\(configsDescription)" + } + return configsDescription } var customRuleConfigurations = [RegexConfiguration]() + var defaultExecutionMode: RegexConfiguration.ExecutionMode? mutating func apply(configuration: Any) throws { guard let configurationDict = configuration as? [String: Any] else { throw Issue.invalidConfiguration(ruleID: Parent.identifier) } - for (key, value) in configurationDict { + // Parse default execution mode if present + if let defaultModeString = configurationDict["default_execution_mode"] as? String { + guard let mode = RegexConfiguration.ExecutionMode(rawValue: defaultModeString) else { + throw Issue.invalidConfiguration(ruleID: Parent.identifier) + } + defaultExecutionMode = mode + } + + for (key, value) in configurationDict where key != "default_execution_mode" { var ruleConfiguration = RegexConfiguration(identifier: key) do { try ruleConfiguration.apply(configuration: value) + + // Apply default execution mode if the rule doesn't specify its own + if ruleConfiguration.executionMode == nil { + ruleConfiguration.executionMode = defaultExecutionMode + } } catch { Issue.invalidConfiguration(ruleID: key).print() continue @@ -50,15 +69,19 @@ struct CustomRules: Rule, CacheDescriptionProvider, ConditionallySourceKitFree { name: "Custom Rules", description: """ Create custom rules by providing a regex string. Optionally specify what syntax kinds to match against, \ - the severity level, and what message to display. + the severity level, and what message to display. Rules default to SwiftSyntax mode for improved \ + performance. Use `mode: sourcekit` or `default_execution_mode: sourcekit` for SourceKit mode. """, kind: .style) var configuration = CustomRulesConfiguration() + /// Returns true if all configured custom rules use SwiftSyntax mode, making this rule effectively SourceKit-free. var isEffectivelySourceKitFree: Bool { - // Just a stub, will be implemented in a follow-up PR - false + configuration.customRuleConfigurations.allSatisfy { config in + let effectiveMode = config.executionMode ?? configuration.defaultExecutionMode ?? .swiftsyntax + return effectiveMode == .swiftsyntax + } } func validate(file: SwiftLintFile) -> [StyleViolation] { diff --git a/Tests/FrameworkTests/CustomRulesTests.swift b/Tests/FrameworkTests/CustomRulesTests.swift index efbfa43f5a..fb8bfec5b2 100644 --- a/Tests/FrameworkTests/CustomRulesTests.swift +++ b/Tests/FrameworkTests/CustomRulesTests.swift @@ -512,6 +512,387 @@ final class CustomRulesTests: SwiftLintTestCase { XCTAssertTrue(violations[3].isSuperfluousDisableCommandViolation(for: "rule2")) } + // MARK: - ExecutionMode Tests (Phase 1) + + func testRegexConfigurationParsesExecutionMode() { + let configDict = [ + "regex": "pattern", + "mode": "swiftsyntax", + ] + + var regexConfig = Configuration(identifier: "test_rule") + do { + try regexConfig.apply(configuration: configDict) + XCTAssertEqual(regexConfig.executionMode, .swiftsyntax) + } catch { + XCTFail("Failed to parse execution mode") + } + } + + func testRegexConfigurationParsesSourceKitMode() { + let configDict = [ + "regex": "pattern", + "mode": "sourcekit", + ] + + var regexConfig = Configuration(identifier: "test_rule") + do { + try regexConfig.apply(configuration: configDict) + XCTAssertEqual(regexConfig.executionMode, .sourcekit) + } catch { + XCTFail("Failed to parse sourcekit mode") + } + } + + func testRegexConfigurationWithoutModeIsNil() { + let configDict = [ + "regex": "pattern", + ] + + var regexConfig = Configuration(identifier: "test_rule") + do { + try regexConfig.apply(configuration: configDict) + XCTAssertNil(regexConfig.executionMode) + } catch { + XCTFail("Failed to parse configuration without mode") + } + } + + func testRegexConfigurationRejectsInvalidMode() { + let configDict = [ + "regex": "pattern", + "mode": "invalid_mode", + ] + + var regexConfig = Configuration(identifier: "test_rule") + checkError(Issue.invalidConfiguration(ruleID: CustomRules.identifier)) { + try regexConfig.apply(configuration: configDict) + } + } + + func testCustomRulesConfigurationParsesDefaultExecutionMode() { + let configDict: [String: Any] = [ + "default_execution_mode": "swiftsyntax", + "my_rule": [ + "regex": "pattern", + ], + ] + + var customRulesConfig = CustomRulesConfiguration() + do { + try customRulesConfig.apply(configuration: configDict) + XCTAssertEqual(customRulesConfig.defaultExecutionMode, .swiftsyntax) + XCTAssertEqual(customRulesConfig.customRuleConfigurations.count, 1) + XCTAssertEqual(customRulesConfig.customRuleConfigurations[0].executionMode, .swiftsyntax) + } catch { + XCTFail("Failed to parse default execution mode") + } + } + + func testCustomRulesAppliesDefaultModeToRulesWithoutExplicitMode() { + let configDict: [String: Any] = [ + "default_execution_mode": "sourcekit", + "rule1": [ + "regex": "pattern1", + ], + "rule2": [ + "regex": "pattern2", + "mode": "swiftsyntax", + ], + ] + + var customRulesConfig = CustomRulesConfiguration() + do { + try customRulesConfig.apply(configuration: configDict) + XCTAssertEqual(customRulesConfig.defaultExecutionMode, .sourcekit) + XCTAssertEqual(customRulesConfig.customRuleConfigurations.count, 2) + + // rule1 should inherit default mode + let rule1 = customRulesConfig.customRuleConfigurations.first { $0.identifier == "rule1" } + XCTAssertEqual(rule1?.executionMode, .sourcekit) + + // rule2 should keep its explicit mode + let rule2 = customRulesConfig.customRuleConfigurations.first { $0.identifier == "rule2" } + XCTAssertEqual(rule2?.executionMode, .swiftsyntax) + } catch { + XCTFail("Failed to apply default mode correctly") + } + } + + func testCustomRulesConfigurationRejectsInvalidDefaultMode() { + let configDict: [String: Any] = [ + "default_execution_mode": "invalid", + "my_rule": [ + "regex": "pattern", + ], + ] + + var customRulesConfig = CustomRulesConfiguration() + checkError(Issue.invalidConfiguration(ruleID: CustomRules.identifier)) { + try customRulesConfig.apply(configuration: configDict) + } + } + + func testExecutionModeIncludedInCacheDescription() { + var regexConfig = Configuration(identifier: "test_rule") + regexConfig.regex = "pattern" + regexConfig.executionMode = .swiftsyntax + + XCTAssertTrue(regexConfig.cacheDescription.contains("swiftsyntax")) + } + + func testExecutionModeAffectsHash() { + var config1 = Configuration(identifier: "test_rule") + config1.regex = "pattern" + config1.executionMode = .swiftsyntax + + var config2 = Configuration(identifier: "test_rule") + config2.regex = "pattern" + config2.executionMode = .sourcekit + + var config3 = Configuration(identifier: "test_rule") + config3.regex = "pattern" + config3.executionMode = nil + + // Different execution modes should produce different hashes + XCTAssertNotEqual(config1.hashValue, config2.hashValue) + XCTAssertNotEqual(config1.hashValue, config3.hashValue) + XCTAssertNotEqual(config2.hashValue, config3.hashValue) + } + + // MARK: - Phase 2 Tests: SwiftSyntax Mode Execution + + func testCustomRuleUsesSwiftSyntaxModeWhenConfigured() throws { + // Test that a rule configured with swiftsyntax mode works correctly + let customRules: [String: Any] = [ + "no_foo": [ + "regex": "\\bfoo\\b", + "mode": "swiftsyntax", + "message": "Don't use foo", + ], + ] + + let example = Example("let foo = 42") + let violations = try violations(forExample: example, customRules: customRules) + + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations[0].ruleIdentifier, "no_foo") + XCTAssertEqual(violations[0].reason, "Don't use foo") + XCTAssertEqual(violations[0].location.line, 1) + XCTAssertEqual(violations[0].location.character, 5) + } + + func testCustomRuleWithoutMatchKindsUsesSwiftSyntaxByDefault() throws { + // When default_execution_mode is swiftsyntax, rules without match_kinds should use it + let customRules: [String: Any] = [ + "default_execution_mode": "swiftsyntax", + "no_bar": [ + "regex": "\\bbar\\b", + "message": "Don't use bar", + ], + ] + + let example = Example("let bar = 42 // bar is not allowed") + let violations = try violations(forExample: example, customRules: customRules) + + // Should find both occurrences of 'bar' since no match_kinds filtering + XCTAssertEqual(violations.count, 2) + XCTAssertEqual(violations[0].location.line, 1) + XCTAssertEqual(violations[0].location.character, 5) + XCTAssertEqual(violations[1].location.line, 1) + XCTAssertEqual(violations[1].location.character, 18) + } + + func testCustomRuleDefaultsToSwiftSyntaxWhenNoModeSpecified() throws { + // When NO execution mode is specified (neither default nor per-rule), it should default to swiftsyntax + let customRules: [String: Any] = [ + "no_foo": [ + "regex": "\\bfoo\\b", + "message": "Don't use foo", + ], + ] + + let example = Example("let foo = 42") + let violations = try violations(forExample: example, customRules: customRules) + + // Should work correctly with implicit swiftsyntax mode + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations[0].ruleIdentifier, "no_foo") + XCTAssertEqual(violations[0].reason, "Don't use foo") + + // Verify the rule is effectively SourceKit-free + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + guard let customRule = configuration.rules.first(where: { $0 is CustomRules }) as? CustomRules else { + XCTFail("Expected CustomRules in configuration") + return + } + + XCTAssertTrue(customRule.isEffectivelySourceKitFree, + "Rule should be effectively SourceKit-free when defaulting to swiftsyntax") + } + + func testCustomRuleWithMatchKindsUsesSwiftSyntaxWhenConfigured() throws { + // Phase 4: Rules with match_kinds in swiftsyntax mode should use SwiftSyntax bridging + let customRules: [String: Any] = [ + "comment_foo": [ + "regex": "foo", + "mode": "swiftsyntax", + "match_kinds": "comment", + "message": "No foo in comments", + ], + ] + + let example = Example(""" + let foo = 42 // This foo should match + let bar = 42 // This should not match + """) + let violations = try violations(forExample: example, customRules: customRules) + + // Should only match 'foo' in comment, not in code + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations[0].location.line, 1) + XCTAssertEqual(violations[0].location.character, 23) // Position of 'foo' in comment + } + + func testCustomRuleWithKindFilteringDefaultsToSwiftSyntax() throws { + // When using kind filtering without specifying mode, it should default to swiftsyntax + let customRules: [String: Any] = [ + "no_keywords": [ + "regex": "\\b\\w+\\b", + "excluded_match_kinds": "keyword", + "message": "Found non-keyword", + ], + ] + + let example = Example("let foo = 42") + let violations = try violations(forExample: example, customRules: customRules) + + // Should match 'foo' and '42' but not 'let' (keyword) + XCTAssertEqual(violations.count, 2) + XCTAssertEqual(violations[0].location.character, 5) // 'foo' + XCTAssertEqual(violations[1].location.character, 11) // '42' + + // Verify the rule is effectively SourceKit-free + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + guard let customRule = configuration.rules.first(where: { $0 is CustomRules }) as? CustomRules else { + XCTFail("Expected CustomRules in configuration") + return + } + + XCTAssertTrue(customRule.isEffectivelySourceKitFree, + "Rule with kind filtering should default to swiftsyntax mode") + } + + func testCustomRuleWithExcludedMatchKindsUsesSwiftSyntaxWithDefaultMode() throws { + // Phase 4: Rules with excluded_match_kinds should use SwiftSyntax when default mode is swiftsyntax + let customRules: [String: Any] = [ + "default_execution_mode": "swiftsyntax", + "no_foo_outside_comments": [ + "regex": "foo", + "excluded_match_kinds": "comment", + "message": "No foo outside comments", + ], + ] + + let example = Example(""" + let foo = 42 // This foo in comment should not match + let foobar = 42 + """) + let violations = try violations(forExample: example, customRules: customRules) + + // Should match 'foo' in code but not in comment + XCTAssertEqual(violations.count, 2) + XCTAssertEqual(violations[0].location.line, 1) + XCTAssertEqual(violations[0].location.character, 5) // 'foo' in variable name + XCTAssertEqual(violations[1].location.line, 2) + XCTAssertEqual(violations[1].location.character, 5) // 'foo' in foobar + } + + func testSwiftSyntaxModeProducesSameResultsAsSourceKitForSimpleRules() throws { + // Test that both modes produce identical results for rules without kind filtering + let pattern = "\\bTODO\\b" + let message = "TODOs should be resolved" + + let swiftSyntaxRules: [String: Any] = [ + "todo_rule": [ + "regex": pattern, + "mode": "swiftsyntax", + "message": message, + ], + ] + + let sourceKitRules: [String: Any] = [ + "todo_rule": [ + "regex": pattern, + "mode": "sourcekit", + "message": message, + ], + ] + + let example = Example(""" + // TODO: Fix this later + func doSomething() { + // Another TODO item + print("TODO is not matched in strings") + } + """) + + let swiftSyntaxViolations = try violations(forExample: example, customRules: swiftSyntaxRules) + let sourceKitViolations = try violations(forExample: example, customRules: sourceKitRules) + + // Both modes should produce identical results + XCTAssertEqual(swiftSyntaxViolations.count, sourceKitViolations.count) + XCTAssertEqual(swiftSyntaxViolations.count, 3) // Two in comments, one in string + + // Verify locations match + for (ssViolation, skViolation) in zip(swiftSyntaxViolations, sourceKitViolations) { + XCTAssertEqual(ssViolation.location.line, skViolation.location.line) + XCTAssertEqual(ssViolation.location.character, skViolation.location.character) + XCTAssertEqual(ssViolation.reason, skViolation.reason) + } + } + + func testSwiftSyntaxModeWithCaptureGroups() throws { + // Test that capture groups work correctly in SwiftSyntax mode + let customRules: [String: Any] = [ + "number_suffix": [ + "regex": "\\b(\\d+)_suffix\\b", + "capture_group": 1, + "mode": "swiftsyntax", + "message": "Number found", + ], + ] + + let example = Example("let value = 42_suffix + 100_suffix") + let violations = try violations(forExample: example, customRules: customRules) + + XCTAssertEqual(violations.count, 2) + // First capture group should highlight just the number part + XCTAssertEqual(violations[0].location.character, 13) // Position of "42" + XCTAssertEqual(violations[1].location.character, 25) // Position of "100" + } + + func testSwiftSyntaxModeRespectsIncludedExcludedPaths() throws { + // Verify that included/excluded path filtering works in SwiftSyntax mode + var regexConfig = Configuration(identifier: "test_rule") + regexConfig.regex = "pattern" + regexConfig.executionMode = .swiftsyntax + regexConfig.included = [try RegularExpression(pattern: "\\.swift$")] + regexConfig.excluded = [try RegularExpression(pattern: "Tests")] + + XCTAssertTrue(regexConfig.shouldValidate(filePath: "/path/to/file.swift")) + XCTAssertFalse(regexConfig.shouldValidate(filePath: "/path/to/file.m")) + XCTAssertFalse(regexConfig.shouldValidate(filePath: "/path/to/Tests/file.swift")) + } + // MARK: - Private private func getCustomRules(_ extraConfig: [String: Any] = [:]) -> (Configuration, CustomRules) { @@ -574,6 +955,142 @@ final class CustomRulesTests: SwiftLintTestCase { customRules.configuration = customRuleConfiguration return customRules } + + // MARK: - Phase 4 Tests: SwiftSyntax Mode WITH Kind Filtering + + func testSwiftSyntaxModeWithMatchKindsProducesCorrectResults() throws { + // Test various syntax kinds with SwiftSyntax bridging + let customRules: [String: Any] = [ + "keyword_test": [ + "regex": "\\b\\w+\\b", + "mode": "swiftsyntax", + "match_kinds": "keyword", + "message": "Found keyword", + ], + ] + + let example = Example(""" + let value = 42 + func test() { + return value + } + """) + let violations = try violations(forExample: example, customRules: customRules) + + // Should match 'let', 'func', and 'return' keywords + XCTAssertEqual(violations.count, 3) + + // Verify the locations correspond to keywords + let expectedLocations = [ + (line: 1, character: 1), // 'let' + (line: 2, character: 1), // 'func' + (line: 3, character: 5), // 'return' + ] + + for (index, expected) in expectedLocations.enumerated() { + XCTAssertEqual(violations[index].location.line, expected.line) + XCTAssertEqual(violations[index].location.character, expected.character) + } + } + + func testSwiftSyntaxModeWithExcludedKindsFiltersCorrectly() throws { + // Test that excluded kinds are properly filtered out + let customRules: [String: Any] = [ + "no_identifier": [ + "regex": "\\b\\w+\\b", + "mode": "swiftsyntax", + "excluded_match_kinds": ["identifier", "typeidentifier"], + "message": "Found non-identifier", + ], + ] + + let example = Example(""" + let value: Int = 42 + """) + let violations = try violations(forExample: example, customRules: customRules) + + // Should match 'let' (keyword) and '42' (number), but not 'value' or 'Int' + XCTAssertEqual(violations.count, 2) + } + + func testSwiftSyntaxModeHandlesComplexKindMatching() throws { + // Test matching multiple specific kinds + let customRules: [String: Any] = [ + "special_tokens": [ + "regex": "\\S+", + "mode": "swiftsyntax", + "match_kinds": ["string", "number", "comment"], + "message": "Found special token", + ], + ] + + let example = Example(""" + let name = "Alice" // User name + let age = 25 + """) + let violations = try violations(forExample: example, customRules: customRules) + + // Should match "Alice" (string), 25 (number), and "// User name" (comment) + // The regex \S+ will match non-whitespace sequences + XCTAssertGreaterThanOrEqual(violations.count, 3) + } + + func testSwiftSyntaxModeWorksWithCaptureGroups() throws { + // Test that capture groups work correctly with SwiftSyntax mode + let customRules: [String: Any] = [ + "string_content": [ + "regex": #""([^"]+)""#, + "mode": "swiftsyntax", + "match_kinds": "string", + "capture_group": 1, + "message": "String content", + ], + ] + + let example = Example(#"let greeting = "Hello, World!""#) + let violations = try violations(forExample: example, customRules: customRules) + + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations[0].location.character, 17) // Start of "Hello, World!" content + } + + func testSwiftSyntaxModeRespectsSourceKitModeOverride() throws { + // Test that explicit sourcekit mode overrides default swiftsyntax mode + let customRules: [String: Any] = [ + "default_execution_mode": "swiftsyntax", + "sourcekit_rule": [ + "regex": "foo", + "mode": "sourcekit", + "match_kinds": "identifier", + "message": "Found foo", + ], + ] + + let example = Example("let foo = 42") + let violations = try violations(forExample: example, customRules: customRules) + + // Should still work correctly with explicit sourcekit mode + XCTAssertEqual(violations.count, 1) + XCTAssertEqual(violations[0].location.character, 5) + } + + func testSwiftSyntaxModeHandlesEmptyBridging() throws { + // Test graceful handling when no tokens match the specified kinds + let customRules: [String: Any] = [ + "attribute_only": [ + "regex": "\\w+", + "mode": "swiftsyntax", + "match_kinds": "attributeBuiltin", // Very specific kind that won't match normal code + "message": "Found attribute", + ], + ] + + let example = Example("let value = 42") + let violations = try violations(forExample: example, customRules: customRules) + + // Should produce no violations since there are no built-in attributes + XCTAssertEqual(violations.count, 0) + } } private extension StyleViolation { From 9bd9ca88e79b9db050025f962aa3568cae53910d Mon Sep 17 00:00:00 2001 From: JP Simard Date: Sat, 21 Jun 2025 17:51:53 -0400 Subject: [PATCH 2/3] Docs --- .../SwiftLintCore/RuleConfigurations/RegexConfiguration.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift b/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift index b037f33773..5b7416c566 100644 --- a/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift +++ b/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift @@ -6,7 +6,9 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, CacheDescriptionProvider, InlinableOptionType { /// The execution mode for this custom rule. public enum ExecutionMode: String, Codable, Sendable { + /// Uses SwiftSyntax to obtain syntax token kinds. case swiftsyntax + /// Uses SourceKit to obtain syntax token kinds. case sourcekit } /// The identifier for this custom rule. From b12ced96c1bed182ed7cdccc409de10804aefbf9 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Sun, 22 Jun 2025 08:53:33 -0400 Subject: [PATCH 3/3] Address PR feedback - Add `default` case to ExecutionMode enum instead of using optional - Change configuration key from `mode` to `execution_mode` for consistency - Move default execution mode logic to runtime instead of configuration time - Refactor test functions to use throws instead of do-catch --- .../RegexConfiguration.swift | 10 +- .../Rules/CustomRules.swift | 11 +- Tests/FrameworkTests/CustomRulesTests.swift | 102 ++++++++---------- 3 files changed, 52 insertions(+), 71 deletions(-) diff --git a/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift b/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift index 5b7416c566..dbb7d626a6 100644 --- a/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift +++ b/Source/SwiftLintCore/RuleConfigurations/RegexConfiguration.swift @@ -10,6 +10,8 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, case swiftsyntax /// Uses SourceKit to obtain syntax token kinds. case sourcekit + /// Uses SwiftSyntax by default unless overridden to use SourceKit. + case `default` } /// The identifier for this custom rule. public let identifier: String @@ -31,8 +33,8 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, public var severityConfiguration = SeverityConfiguration(.warning) /// The index of the regex capture group to match. public var captureGroup = 0 - /// The execution mode for this rule (nil means use global default). - public var executionMode: ExecutionMode? + /// The execution mode for this rule. + public var executionMode: ExecutionMode = .default public var cacheDescription: String { let jsonObject: [String] = [ @@ -45,7 +47,7 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, SyntaxKind.allKinds.subtracting(excludedMatchKinds) .map(\.rawValue).sorted(by: <).joined(separator: ","), severity.rawValue, - executionMode?.rawValue ?? "", + executionMode.rawValue, ] if let jsonData = try? JSONSerialization.data(withJSONObject: jsonObject), let jsonString = String(data: jsonData, encoding: .utf8) { @@ -108,7 +110,7 @@ public struct RegexConfiguration: SeverityBasedRuleConfiguration, self.captureGroup = captureGroup } - if let modeString = configurationDict["mode"] as? String { + if let modeString = configurationDict["execution_mode"] as? String { guard let mode = ExecutionMode(rawValue: modeString) else { throw Issue.invalidConfiguration(ruleID: Parent.identifier) } diff --git a/Source/SwiftLintFramework/Rules/CustomRules.swift b/Source/SwiftLintFramework/Rules/CustomRules.swift index e13f7cf417..d54c57488e 100644 --- a/Source/SwiftLintFramework/Rules/CustomRules.swift +++ b/Source/SwiftLintFramework/Rules/CustomRules.swift @@ -38,11 +38,6 @@ struct CustomRulesConfiguration: RuleConfiguration, CacheDescriptionProvider { do { try ruleConfiguration.apply(configuration: value) - - // Apply default execution mode if the rule doesn't specify its own - if ruleConfiguration.executionMode == nil { - ruleConfiguration.executionMode = defaultExecutionMode - } } catch { Issue.invalidConfiguration(ruleID: key).print() continue @@ -70,7 +65,7 @@ struct CustomRules: Rule, CacheDescriptionProvider, ConditionallySourceKitFree { description: """ Create custom rules by providing a regex string. Optionally specify what syntax kinds to match against, \ the severity level, and what message to display. Rules default to SwiftSyntax mode for improved \ - performance. Use `mode: sourcekit` or `default_execution_mode: sourcekit` for SourceKit mode. + performance. Use `execution_mode: sourcekit` or `default_execution_mode: sourcekit` for SourceKit mode. """, kind: .style) @@ -79,7 +74,9 @@ struct CustomRules: Rule, CacheDescriptionProvider, ConditionallySourceKitFree { /// Returns true if all configured custom rules use SwiftSyntax mode, making this rule effectively SourceKit-free. var isEffectivelySourceKitFree: Bool { configuration.customRuleConfigurations.allSatisfy { config in - let effectiveMode = config.executionMode ?? configuration.defaultExecutionMode ?? .swiftsyntax + let effectiveMode = config.executionMode == .default + ? (configuration.defaultExecutionMode ?? .swiftsyntax) + : config.executionMode return effectiveMode == .swiftsyntax } } diff --git a/Tests/FrameworkTests/CustomRulesTests.swift b/Tests/FrameworkTests/CustomRulesTests.swift index fb8bfec5b2..98b67a083c 100644 --- a/Tests/FrameworkTests/CustomRulesTests.swift +++ b/Tests/FrameworkTests/CustomRulesTests.swift @@ -33,6 +33,7 @@ final class CustomRulesTests: SwiftLintTestCase { comp.regex = "regex" comp.severityConfiguration = SeverityConfiguration(.error) comp.excludedMatchKinds = SyntaxKind.allKinds.subtracting([.comment]) + comp.executionMode = .default var compRules = CustomRulesConfiguration() compRules.customRuleConfigurations = [comp] do { @@ -60,6 +61,7 @@ final class CustomRulesTests: SwiftLintTestCase { comp.regex = "regex" comp.severityConfiguration = SeverityConfiguration(.error) comp.excludedMatchKinds = Set([.comment]) + comp.executionMode = .default var compRules = CustomRulesConfiguration() compRules.customRuleConfigurations = [comp] do { @@ -514,54 +516,42 @@ final class CustomRulesTests: SwiftLintTestCase { // MARK: - ExecutionMode Tests (Phase 1) - func testRegexConfigurationParsesExecutionMode() { + func testRegexConfigurationParsesExecutionMode() throws { let configDict = [ "regex": "pattern", - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", ] var regexConfig = Configuration(identifier: "test_rule") - do { - try regexConfig.apply(configuration: configDict) - XCTAssertEqual(regexConfig.executionMode, .swiftsyntax) - } catch { - XCTFail("Failed to parse execution mode") - } + try regexConfig.apply(configuration: configDict) + XCTAssertEqual(regexConfig.executionMode, .swiftsyntax) } - func testRegexConfigurationParsesSourceKitMode() { + func testRegexConfigurationParsesSourceKitMode() throws { let configDict = [ "regex": "pattern", - "mode": "sourcekit", + "execution_mode": "sourcekit", ] var regexConfig = Configuration(identifier: "test_rule") - do { - try regexConfig.apply(configuration: configDict) - XCTAssertEqual(regexConfig.executionMode, .sourcekit) - } catch { - XCTFail("Failed to parse sourcekit mode") - } + try regexConfig.apply(configuration: configDict) + XCTAssertEqual(regexConfig.executionMode, .sourcekit) } - func testRegexConfigurationWithoutModeIsNil() { + func testRegexConfigurationWithoutModeIsDefault() throws { let configDict = [ "regex": "pattern", ] var regexConfig = Configuration(identifier: "test_rule") - do { - try regexConfig.apply(configuration: configDict) - XCTAssertNil(regexConfig.executionMode) - } catch { - XCTFail("Failed to parse configuration without mode") - } + try regexConfig.apply(configuration: configDict) + XCTAssertEqual(regexConfig.executionMode, .default) } func testRegexConfigurationRejectsInvalidMode() { let configDict = [ "regex": "pattern", - "mode": "invalid_mode", + "execution_mode": "invalid_mode", ] var regexConfig = Configuration(identifier: "test_rule") @@ -570,7 +560,7 @@ final class CustomRulesTests: SwiftLintTestCase { } } - func testCustomRulesConfigurationParsesDefaultExecutionMode() { + func testCustomRulesConfigurationParsesDefaultExecutionMode() throws { let configDict: [String: Any] = [ "default_execution_mode": "swiftsyntax", "my_rule": [ @@ -579,17 +569,13 @@ final class CustomRulesTests: SwiftLintTestCase { ] var customRulesConfig = CustomRulesConfiguration() - do { - try customRulesConfig.apply(configuration: configDict) - XCTAssertEqual(customRulesConfig.defaultExecutionMode, .swiftsyntax) - XCTAssertEqual(customRulesConfig.customRuleConfigurations.count, 1) - XCTAssertEqual(customRulesConfig.customRuleConfigurations[0].executionMode, .swiftsyntax) - } catch { - XCTFail("Failed to parse default execution mode") - } + try customRulesConfig.apply(configuration: configDict) + XCTAssertEqual(customRulesConfig.defaultExecutionMode, .swiftsyntax) + XCTAssertEqual(customRulesConfig.customRuleConfigurations.count, 1) + XCTAssertEqual(customRulesConfig.customRuleConfigurations[0].executionMode, .default) } - func testCustomRulesAppliesDefaultModeToRulesWithoutExplicitMode() { + func testCustomRulesAppliesDefaultModeToRulesWithoutExplicitMode() throws { let configDict: [String: Any] = [ "default_execution_mode": "sourcekit", "rule1": [ @@ -597,26 +583,22 @@ final class CustomRulesTests: SwiftLintTestCase { ], "rule2": [ "regex": "pattern2", - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", ], ] var customRulesConfig = CustomRulesConfiguration() - do { - try customRulesConfig.apply(configuration: configDict) - XCTAssertEqual(customRulesConfig.defaultExecutionMode, .sourcekit) - XCTAssertEqual(customRulesConfig.customRuleConfigurations.count, 2) + try customRulesConfig.apply(configuration: configDict) + XCTAssertEqual(customRulesConfig.defaultExecutionMode, .sourcekit) + XCTAssertEqual(customRulesConfig.customRuleConfigurations.count, 2) - // rule1 should inherit default mode - let rule1 = customRulesConfig.customRuleConfigurations.first { $0.identifier == "rule1" } - XCTAssertEqual(rule1?.executionMode, .sourcekit) + // rule1 should have default mode + let rule1 = customRulesConfig.customRuleConfigurations.first { $0.identifier == "rule1" } + XCTAssertEqual(rule1?.executionMode, .default) - // rule2 should keep its explicit mode - let rule2 = customRulesConfig.customRuleConfigurations.first { $0.identifier == "rule2" } - XCTAssertEqual(rule2?.executionMode, .swiftsyntax) - } catch { - XCTFail("Failed to apply default mode correctly") - } + // rule2 should keep its explicit mode + let rule2 = customRulesConfig.customRuleConfigurations.first { $0.identifier == "rule2" } + XCTAssertEqual(rule2?.executionMode, .swiftsyntax) } func testCustomRulesConfigurationRejectsInvalidDefaultMode() { @@ -652,7 +634,7 @@ final class CustomRulesTests: SwiftLintTestCase { var config3 = Configuration(identifier: "test_rule") config3.regex = "pattern" - config3.executionMode = nil + config3.executionMode = .default // Different execution modes should produce different hashes XCTAssertNotEqual(config1.hashValue, config2.hashValue) @@ -667,7 +649,7 @@ final class CustomRulesTests: SwiftLintTestCase { let customRules: [String: Any] = [ "no_foo": [ "regex": "\\bfoo\\b", - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "message": "Don't use foo", ], ] @@ -740,7 +722,7 @@ final class CustomRulesTests: SwiftLintTestCase { let customRules: [String: Any] = [ "comment_foo": [ "regex": "foo", - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "match_kinds": "comment", "message": "No foo in comments", ], @@ -824,7 +806,7 @@ final class CustomRulesTests: SwiftLintTestCase { let swiftSyntaxRules: [String: Any] = [ "todo_rule": [ "regex": pattern, - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "message": message, ], ] @@ -832,7 +814,7 @@ final class CustomRulesTests: SwiftLintTestCase { let sourceKitRules: [String: Any] = [ "todo_rule": [ "regex": pattern, - "mode": "sourcekit", + "execution_mode": "sourcekit", "message": message, ], ] @@ -866,7 +848,7 @@ final class CustomRulesTests: SwiftLintTestCase { "number_suffix": [ "regex": "\\b(\\d+)_suffix\\b", "capture_group": 1, - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "message": "Number found", ], ] @@ -963,7 +945,7 @@ final class CustomRulesTests: SwiftLintTestCase { let customRules: [String: Any] = [ "keyword_test": [ "regex": "\\b\\w+\\b", - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "match_kinds": "keyword", "message": "Found keyword", ], @@ -998,7 +980,7 @@ final class CustomRulesTests: SwiftLintTestCase { let customRules: [String: Any] = [ "no_identifier": [ "regex": "\\b\\w+\\b", - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "excluded_match_kinds": ["identifier", "typeidentifier"], "message": "Found non-identifier", ], @@ -1018,7 +1000,7 @@ final class CustomRulesTests: SwiftLintTestCase { let customRules: [String: Any] = [ "special_tokens": [ "regex": "\\S+", - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "match_kinds": ["string", "number", "comment"], "message": "Found special token", ], @@ -1040,7 +1022,7 @@ final class CustomRulesTests: SwiftLintTestCase { let customRules: [String: Any] = [ "string_content": [ "regex": #""([^"]+)""#, - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "match_kinds": "string", "capture_group": 1, "message": "String content", @@ -1060,7 +1042,7 @@ final class CustomRulesTests: SwiftLintTestCase { "default_execution_mode": "swiftsyntax", "sourcekit_rule": [ "regex": "foo", - "mode": "sourcekit", + "execution_mode": "sourcekit", "match_kinds": "identifier", "message": "Found foo", ], @@ -1079,7 +1061,7 @@ final class CustomRulesTests: SwiftLintTestCase { let customRules: [String: Any] = [ "attribute_only": [ "regex": "\\w+", - "mode": "swiftsyntax", + "execution_mode": "swiftsyntax", "match_kinds": "attributeBuiltin", // Very specific kind that won't match normal code "message": "Found attribute", ],