diff --git a/CHANGELOG.md b/CHANGELOG.md index d6eab7f239..09e52eefde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ * SwiftLint now requires macOS 13 or higher to run. [JP Simard](https://github.com/jpsim) +* Custom rules now default to SwiftSyntax mode for pattern matching instead of SourceKit. + This may result in subtle behavioral differences. While performance is significantly improved, + rules that rely on specific SourceKit behaviors may need adjustment. Users can temporarily + revert to the legacy SourceKit behavior by setting `default_execution_mode: sourcekit` in + their custom rules configuration or `execution_mode: sourcekit` for individual rules. + The SourceKit mode is deprecated and will be removed in a future version. + [JP Simard](https://github.com/jpsim) + ### Experimental * None. diff --git a/Source/SwiftLintCore/Extensions/StringView+SwiftLint.swift b/Source/SwiftLintCore/Extensions/StringView+SwiftLint.swift index bf1ded7211..7cb0a88d63 100644 --- a/Source/SwiftLintCore/Extensions/StringView+SwiftLint.swift +++ b/Source/SwiftLintCore/Extensions/StringView+SwiftLint.swift @@ -1,3 +1,4 @@ +import Foundation import SourceKittenFramework public extension StringView { @@ -12,4 +13,17 @@ public extension StringView { } return lines[Int(line) - 1].byteRange.location + ByteCount(bytePosition - 1) } + + /// Matches a pattern in the string view and returns ranges for the specified capture group. + /// This method does not use SourceKit and is suitable for SwiftSyntax mode. + /// - Parameters: + /// - pattern: The regular expression pattern to match. + /// - captureGroup: The capture group index to extract (0 for the full match). + /// - Returns: An array of NSRange objects for the matched capture groups. + func match(pattern: String, captureGroup: Int = 0) -> [NSRange] { + regex(pattern).matches(in: self).compactMap { match in + let range = match.range(at: captureGroup) + return range.location != NSNotFound ? range : nil + } + } } diff --git a/Source/SwiftLintFramework/Rules/CustomRules.swift b/Source/SwiftLintFramework/Rules/CustomRules.swift index d54c57488e..34f59931f2 100644 --- a/Source/SwiftLintFramework/Rules/CustomRules.swift +++ b/Source/SwiftLintFramework/Rules/CustomRules.swift @@ -1,4 +1,5 @@ import Foundation +import SourceKittenFramework // MARK: - CustomRulesConfiguration @@ -103,7 +104,49 @@ struct CustomRules: Rule, CacheDescriptionProvider, ConditionallySourceKitFree { let pattern = configuration.regex.pattern let captureGroup = configuration.captureGroup let excludingKinds = configuration.excludedMatchKinds - return file.match(pattern: pattern, excludingSyntaxKinds: excludingKinds, captureGroup: captureGroup).map({ + + // Determine effective execution mode (defaults to swiftsyntax if not specified) + let effectiveMode = configuration.executionMode == .default + ? (self.configuration.defaultExecutionMode ?? .swiftsyntax) + : configuration.executionMode + let needsKindMatching = !excludingKinds.isEmpty + + let matches: [NSRange] + if effectiveMode == .swiftsyntax { + if needsKindMatching { + // SwiftSyntax mode WITH kind filtering + // CRITICAL: This path must not trigger any SourceKit requests + guard let bridgedTokens = file.swiftSyntaxDerivedSourceKittenTokens else { + // Log error/warning: Bridging failed + queuedPrintError( + "Warning: SwiftSyntax bridging failed for custom rule '\(configuration.identifier)'" + ) + return [] + } + let syntaxMapFromBridgedTokens = SwiftLintSyntaxMap( + value: SyntaxMap(tokens: bridgedTokens.map(\.value)) + ) + + // Use the performMatchingWithSyntaxMap helper that operates on stringView and syntaxMap ONLY + matches = performMatchingWithSyntaxMap( + stringView: file.stringView, + syntaxMap: syntaxMapFromBridgedTokens, + pattern: pattern, + excludingSyntaxKinds: excludingKinds, + captureGroup: captureGroup + ) + } else { + // SwiftSyntax mode WITHOUT kind filtering + // This path must not trigger any SourceKit requests + matches = file.stringView.match(pattern: pattern, captureGroup: captureGroup) + } + } else { + // SourceKit mode + // SourceKit calls ARE EXPECTED AND PERMITTED here because CustomRules is not SourceKitFreeRule + matches = file.match(pattern: pattern, excludingSyntaxKinds: excludingKinds, captureGroup: captureGroup) + } + + return matches.map({ StyleViolation(ruleDescription: configuration.description, severity: configuration.severity, location: Location(file: file, characterOffset: $0.location), @@ -134,3 +177,42 @@ struct CustomRules: Rule, CacheDescriptionProvider, ConditionallySourceKitFree { && !region.disabledRuleIdentifiers.contains(.all) } } + +// MARK: - Helpers + +private func performMatchingWithSyntaxMap( + stringView: StringView, + syntaxMap: SwiftLintSyntaxMap, + pattern: String, + excludingSyntaxKinds: Set, + captureGroup: Int +) -> [NSRange] { + // This helper method must not access any part of SwiftLintFile that could trigger SourceKit requests + // It operates only on the provided stringView and syntaxMap + + let regex = regex(pattern) + let range = stringView.range + let matches = regex.matches(in: stringView, options: [], range: range) + + return matches.compactMap { match in + let matchRange = match.range(at: captureGroup) + + // Get tokens in the match range + guard let byteRange = stringView.NSRangeToByteRange( + start: matchRange.location, + length: matchRange.length + ) else { + return nil + } + + let tokensInRange = syntaxMap.tokens(inByteRange: byteRange) + let kindsInRange = Set(tokensInRange.kinds) + + // Check if any excluded kinds are present + if excludingSyntaxKinds.isDisjoint(with: kindsInRange) { + return matchRange + } + + return nil + } +} diff --git a/Tests/FrameworkTests/CustomRulesTests.swift b/Tests/FrameworkTests/CustomRulesTests.swift index 98b67a083c..1be6d568c9 100644 --- a/Tests/FrameworkTests/CustomRulesTests.swift +++ b/Tests/FrameworkTests/CustomRulesTests.swift @@ -11,12 +11,6 @@ final class CustomRulesTests: SwiftLintTestCase { private var testFile: SwiftLintFile { SwiftLintFile(path: "\(TestResources.path())/test.txt")! } - override func invokeTest() { - CurrentRule.$allowSourceKitRequestWithoutRule.withValue(true) { - super.invokeTest() - } - } - func testCustomRuleConfigurationSetsCorrectlyWithMatchKinds() { let configDict = [ "my_custom_rule": [ diff --git a/Tests/FrameworkTests/CustomRulesValidationTests.swift b/Tests/FrameworkTests/CustomRulesValidationTests.swift new file mode 100644 index 0000000000..6e7e27ce9e --- /dev/null +++ b/Tests/FrameworkTests/CustomRulesValidationTests.swift @@ -0,0 +1,366 @@ +import SourceKittenFramework +@testable import SwiftLintCore +@testable import SwiftLintFramework +import TestHelpers +import XCTest + +/// Tests to ensure CustomRules properly handles SourceKit validation system +final class CustomRulesValidationTests: SwiftLintTestCase { + private typealias Configuration = RegexConfiguration + + // MARK: - Validation System Tests + + func testSwiftSyntaxModeWithoutKindsMakesNoSourceKitCalls() throws { + // This test verifies that SwiftSyntax mode without kind matching makes no SourceKit calls + // If it did, the validation system would fatal error (but we're allowing it for test setup) + let customRules: [String: Any] = [ + "simple_rule": [ + "regex": "TODO", + "mode": "swiftsyntax", + "message": "Found TODO", + ], + ] + + let example = Example("// TODO: Fix this") + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + // This should complete without fatal errors + let violations = TestHelpers.violations(example, config: configuration) + XCTAssertEqual(violations.count, 1) + } + + func testSwiftSyntaxModeWithKindsMakesNoSourceKitCalls() throws { + // This test verifies that SwiftSyntax mode with kind matching uses bridged tokens + // and makes no SourceKit calls + let customRules: [String: Any] = [ + "keyword_rule": [ + "regex": "\\b\\w+\\b", + "mode": "swiftsyntax", + "match_kinds": "keyword", + "message": "Found keyword", + ], + ] + + let example = Example("let value = 42") + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + // This should complete without fatal errors + let violations = TestHelpers.violations(example, config: configuration) + XCTAssertGreaterThanOrEqual(violations.count, 1) // At least 'let' keyword + } + + func testSourceKitModeCanMakeSourceKitCalls() throws { + // This test verifies that SourceKit mode is allowed to make SourceKit calls + // because CustomRules is not a SourceKitFreeRule + let customRules: [String: Any] = [ + "identifier_rule": [ + "regex": "\\b\\w+\\b", + "mode": "sourcekit", + "match_kinds": "identifier", + "message": "Found identifier", + ], + ] + + let example = Example("let myVariable = 42") + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + // This should complete without fatal errors even though it makes SourceKit calls + let violations = TestHelpers.violations(example, config: configuration) + XCTAssertGreaterThanOrEqual(violations.count, 1) // At least 'myVariable' + } + + func testDefaultSwiftSyntaxModeWithKindFilteringMakesNoSourceKitCalls() throws { + // Test that default swiftsyntax mode with kind filtering doesn't trigger SourceKit + let customRules: [String: Any] = [ + "default_execution_mode": "swiftsyntax", + "comment_rule": [ + "regex": "\\b\\w+\\b", + "excluded_match_kinds": "comment", + "message": "Found non-comment word", + ], + ] + + let example = Example(""" + let value = 42 // This is a comment + """) + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + // This should complete without fatal errors + let violations = TestHelpers.violations(example, config: configuration) + XCTAssertGreaterThanOrEqual(violations.count, 3) // 'let', 'value', '42' + } + + func testMixedModeRulesWorkCorrectly() throws { + // Test that having both SwiftSyntax and SourceKit rules in the same configuration works + let customRules: [String: Any] = [ + "syntax_rule": [ + "regex": "TODO", + "mode": "swiftsyntax", + "message": "SwiftSyntax TODO", + ], + "sourcekit_rule": [ + "regex": "FIXME", + "mode": "sourcekit", + "message": "SourceKit FIXME", + ], + ] + + let example = Example(""" + // TODO: Do this + // FIXME: Fix that + """) + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + let violations = TestHelpers.violations(example, config: configuration) + XCTAssertEqual(violations.count, 2) + XCTAssertTrue(violations.contains { $0.reason == "SwiftSyntax TODO" }) + XCTAssertTrue(violations.contains { $0.reason == "SourceKit FIXME" }) + } + + // MARK: - Bridging Validation Tests + + func testBridgedTokensProduceEquivalentResults() throws { + // Compare results between SwiftSyntax bridged tokens and SourceKit tokens + let pattern = "\\b\\w+\\b" + let kinds = ["keyword", "identifier"] + + let swiftSyntaxRules: [String: Any] = [ + "test_rule": [ + "regex": pattern, + "mode": "swiftsyntax", + "match_kinds": kinds, + "message": "Match", + ], + ] + + let sourceKitRules: [String: Any] = [ + "test_rule": [ + "regex": pattern, + "mode": "sourcekit", + "match_kinds": kinds, + "message": "Match", + ], + ] + + let example = Example(""" + func testFunction() { + let value = 42 + } + """) + + let ssConfig = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": swiftSyntaxRules, + ]) + + let skConfig = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": sourceKitRules, + ]) + + let ssViolations = TestHelpers.violations(example, config: ssConfig) + let skViolations = TestHelpers.violations(example, config: skConfig) + + // Both should find similar matches (exact count may vary due to classification differences) + XCTAssertGreaterThan(ssViolations.count, 0) + XCTAssertGreaterThan(skViolations.count, 0) + + // The difference should be reasonable (within classification mapping differences) + let countDifference = abs(ssViolations.count - skViolations.count) + XCTAssertLessThanOrEqual(countDifference, 2, "SwiftSyntax and SourceKit results differ too much") + } + + func testBridgingHandlesEdgeCases() throws { + // Test edge cases like empty files, whitespace-only files, etc. + let customRules: [String: Any] = [ + "any_token": [ + "regex": "\\S+", + "mode": "swiftsyntax", + "match_kinds": ["keyword", "identifier", "string", "number"], + "message": "Found token", + ], + ] + + let testCases = [ + "", // Empty file + " \n\t ", // Whitespace only + "// Only a comment", // Comment only (should not match since we're looking for specific kinds) + ] + + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + for testCase in testCases { + let example = Example(testCase) + // Should not crash or fatal error + _ = TestHelpers.violations(example, config: configuration) + } + } + + // MARK: - Phase 6 Tests: Conditional SourceKit-Free Behavior + + func testCustomRulesIsEffectivelySourceKitFreeWithAllSwiftSyntaxRules() { + var customRules = CustomRules() + customRules.configuration.defaultExecutionMode = .swiftsyntax + customRules.configuration.customRuleConfigurations = [ + { + var config = RegexConfiguration(identifier: "rule1") + config.regex = "pattern1" + config.executionMode = .swiftsyntax + return config + }(), + { + var config = RegexConfiguration(identifier: "rule2") + config.regex = "pattern2" + // Uses default mode (swiftsyntax) + return config + }(), + ] + + XCTAssertTrue(customRules.isEffectivelySourceKitFree) + XCTAssertFalse(customRules.requiresSourceKit) + } + + func testCustomRulesRequiresSourceKitWithMixedModes() { + var customRules = CustomRules() + customRules.configuration.defaultExecutionMode = .swiftsyntax + customRules.configuration.customRuleConfigurations = [ + { + var config = RegexConfiguration(identifier: "rule1") + config.regex = "pattern1" + config.executionMode = .swiftsyntax + return config + }(), + { + var config = RegexConfiguration(identifier: "rule2") + config.regex = "pattern2" + config.executionMode = .sourcekit // One SourceKit rule + return config + }(), + ] + + XCTAssertFalse(customRules.isEffectivelySourceKitFree) + XCTAssertTrue(customRules.requiresSourceKit) + } + + func testCustomRulesDefaultsToSwiftSyntaxWithoutExplicitMode() { + var customRules = CustomRules() + // No default mode set (now defaults to swiftsyntax) + customRules.configuration.customRuleConfigurations = [ + { + var config = RegexConfiguration(identifier: "rule1") + config.regex = "pattern1" + // No explicit mode, uses default (swiftsyntax) + return config + }(), + ] + + XCTAssertTrue(customRules.isEffectivelySourceKitFree) + XCTAssertFalse(customRules.requiresSourceKit) + } + + func testSourceKitNotInitializedForSourceKitFreeCustomRules() throws { + // This test verifies that when all custom rules use SwiftSyntax mode, + // SourceKit is not initialized at all + let customRules: [String: Any] = [ + "default_execution_mode": "swiftsyntax", + "simple_rule": [ + "regex": "TODO", + "message": "Found TODO", + ], + "keyword_rule": [ + "regex": "\\b\\w+\\b", + "match_kinds": "keyword", + "message": "Found keyword", + ], + ] + + let example = Example("let x = 42 // TODO: Fix this") + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + // Create a new file to ensure no cached SourceKit response + let file = SwiftLintFile(contents: example.code) + + // Get the configured custom rules + guard let customRule = configuration.rules.first(where: { $0 is CustomRules }) as? CustomRules else { + XCTFail("Expected CustomRules in configuration") + return + } + + // Verify it's effectively SourceKit-free + XCTAssertTrue(customRule.isEffectivelySourceKitFree) + XCTAssertFalse(customRule.requiresSourceKit) + + // Run validation - this should not trigger SourceKit + let violations = customRule.validate(file: file) + + // Should find violations without using SourceKit + XCTAssertGreaterThanOrEqual(violations.count, 2) // 'let' keyword and 'TODO' + + // Verify SourceKit was never accessed + // Note: In a real test environment, we'd check that no SourceKit requests were made + // For now, we just verify the rule ran successfully + } + + func testSourceKitNotInitializedWithImplicitSwiftSyntaxDefault() throws { + // This test verifies that when NO execution mode is specified, + // custom rules default to SwiftSyntax and don't initialize SourceKit + let customRules: [String: Any] = [ + // Note: NO default_execution_mode specified + "simple_rule": [ + "regex": "TODO", + "message": "Found TODO", + ], + "keyword_rule": [ + "regex": "\\b\\w+\\b", + "match_kinds": "keyword", // Kind filtering without explicit mode + "message": "Found keyword", + ], + ] + + let example = Example("let x = 42 // TODO: Fix this") + let configuration = try SwiftLintFramework.Configuration(dict: [ + "only_rules": ["custom_rules"], + "custom_rules": customRules, + ]) + + // Create a new file to ensure no cached SourceKit response + let file = SwiftLintFile(contents: example.code) + + // Get the configured custom rules + guard let customRule = configuration.rules.first(where: { $0 is CustomRules }) as? CustomRules else { + XCTFail("Expected CustomRules in configuration") + return + } + + // Verify it's effectively SourceKit-free (defaulting to swiftsyntax) + XCTAssertTrue(customRule.isEffectivelySourceKitFree) + XCTAssertFalse(customRule.requiresSourceKit) + + // Run validation - this should not trigger SourceKit + let violations = customRule.validate(file: file) + + // Should find violations without using SourceKit + XCTAssertGreaterThanOrEqual(violations.count, 2) // 'let' keyword and 'TODO' + } +}