Enforce SourceKitFreeRule contract with fatal error (#6107)

jpsim · web-flow · commit 81474e36d012 · 2025-06-20T07:25:01.000-04:00
diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnusedDeclarationRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnusedDeclarationRule.swift
@@ -84,8 +84,7 @@ private extension SwiftLintFile {
     func index(compilerArguments: [String]) -> SourceKittenDictionary? {
         path
             .flatMap { path in
-                try? Request.index(file: path, arguments: compilerArguments)
-                            .send()
+                try? Request.index(file: path, arguments: compilerArguments).send()
             }
             .map(SourceKittenDictionary.init)
     }
diff --git a/Source/SwiftLintCore/Extensions/Request+SwiftLint.swift b/Source/SwiftLintCore/Extensions/Request+SwiftLint.swift
@@ -5,6 +5,35 @@ public extension Request {
     static let disableSourceKit = ProcessInfo.processInfo.environment["SWIFTLINT_DISABLE_SOURCEKIT"] != nil
 
     func sendIfNotDisabled() throws -> [String: any SourceKitRepresentable] {
+        // Skip safety checks if explicitly allowed (e.g., for testing or specific operations)
+        if !CurrentRule.allowSourceKitRequestWithoutRule {
+            // Check if we have a rule context
+            if let ruleID = CurrentRule.identifier {
+                // Skip registry check for mock test rules
+                if ruleID != "mock_test_rule_for_swiftlint_tests" {
+                    // Ensure the rule exists in the registry
+                    guard let ruleType = RuleRegistry.shared.rule(forID: ruleID) else {
+                        queuedFatalError("""
+                            Rule '\(ruleID)' not found in RuleRegistry. This indicates a configuration or wiring issue.
+                            """)
+                    }
+
+                    // Check if the current rule is a SourceKitFreeRule
+                    if ruleType is any SourceKitFreeRule.Type {
+                        queuedFatalError("""
+                            '\(ruleID)' is a SourceKitFreeRule and should not be making requests to SourceKit.
+                            """)
+                    }
+                }
+            } else {
+                // No rule context and not explicitly allowed
+                queuedFatalError("""
+                    SourceKit request made outside of rule execution context without explicit permission.
+                    Use CurrentRule.$allowSourceKitRequestWithoutRule.withValue(true) { ... } for allowed exceptions.
+                    """)
+            }
+        }
+
         guard !Self.disableSourceKit else {
             throw Self.Error.connectionInterrupted("SourceKit is disabled by `SWIFTLINT_DISABLE_SOURCEKIT`.")
         }
diff --git a/Source/SwiftLintCore/Models/CurrentRule.swift b/Source/SwiftLintCore/Models/CurrentRule.swift
@@ -0,0 +1,11 @@
+/// A task-local value that holds the identifier of the currently executing rule.
+/// This allows SourceKit request handling to determine if the current rule
+/// is a SourceKitFreeRule without modifying function signatures throughout the codebase.
+public enum CurrentRule {
+    /// The Rule ID for the currently executing rule.
+    @TaskLocal public static var identifier: String?
+
+    /// Allows specific SourceKit requests to be made outside of rule execution context.
+    /// This should only be used for essential operations like getting the Swift version.
+    @TaskLocal public static var allowSourceKitRequestWithoutRule = false
+}
diff --git a/Source/SwiftLintCore/Models/SwiftVersion.swift b/Source/SwiftLintCore/Models/SwiftVersion.swift
@@ -87,7 +87,11 @@ public extension SwiftVersion {
         if !Request.disableSourceKit {
             // This request was added in Swift 5.1
             let params: SourceKitObject = ["key.request": UID("source.request.compiler_version")]
-            if let result = try? Request.customRequest(request: params).send(),
+            // Allow this specific SourceKit request outside of rule execution context
+            let result = CurrentRule.$allowSourceKitRequestWithoutRule.withValue(true) {
+                try? Request.customRequest(request: params).sendIfNotDisabled()
+            }
+            if let result,
                 let major = result.versionMajor, let minor = result.versionMinor, let patch = result.versionPatch {
                 return SwiftVersion(rawValue: "\(major).\(minor).\(patch)")
             }
diff --git a/Source/SwiftLintFramework/Models/Linter.swift b/Source/SwiftLintFramework/Models/Linter.swift
@@ -1,5 +1,6 @@
 import Foundation
 import SourceKittenFramework
+import SwiftLintCore
 
 // swiftlint:disable file_length
 
@@ -95,6 +96,32 @@ private extension Rule {
               compilerArguments: [String]) -> LintResult {
         let ruleID = Self.identifier
 
+        // Wrap entire lint process including shouldRun check in rule context
+        return CurrentRule.$identifier.withValue(ruleID) {
+            guard shouldRun(onFile: file) else {
+                return LintResult(violations: [], ruleTime: nil, deprecatedToValidIDPairs: [])
+            }
+
+            return performLint(
+                file: file,
+                regions: regions,
+                benchmark: benchmark,
+                storage: storage,
+                superfluousDisableCommandRule: superfluousDisableCommandRule,
+                compilerArguments: compilerArguments
+            )
+        }
+    }
+
+    // swiftlint:disable:next function_parameter_count
+    private func performLint(file: SwiftLintFile,
+                             regions: [Region],
+                             benchmark: Bool,
+                             storage: RuleStorage,
+                             superfluousDisableCommandRule: SuperfluousDisableCommandRule?,
+                             compilerArguments: [String]) -> LintResult {
+        let ruleID = Self.identifier
+
         let violations: [StyleViolation]
         let ruleTime: (String, Double)?
         if benchmark {
@@ -116,11 +143,11 @@ private extension Rule {
         }
 
         let customRulesIDs: [String] = {
-             guard let customRules = self as? CustomRules else {
-                 return []
-             }
-             return customRules.customRuleIdentifiers
-         }()
+            guard let customRules = self as? CustomRules else {
+                return []
+            }
+            return customRules.customRuleIdentifiers
+        }()
         let ruleIDs = Self.description.allIdentifiers +
             customRulesIDs +
             (superfluousDisableCommandRule.map({ type(of: $0) })?.description.allIdentifiers ?? []) +
@@ -247,7 +274,11 @@ public struct Linter {
     /// - returns: A linter capable of checking for violations after running each rule's collection step.
     public func collect(into storage: RuleStorage) -> CollectedLinter {
         DispatchQueue.concurrentPerform(iterations: rules.count) { idx in
-            rules[idx].collectInfo(for: file, into: storage, compilerArguments: compilerArguments)
+            let rule = rules[idx]
+            let ruleID = type(of: rule).identifier
+            CurrentRule.$identifier.withValue(ruleID) {
+                rule.collectInfo(for: file, into: storage, compilerArguments: compilerArguments)
+            }
         }
         return CollectedLinter(from: self)
     }
@@ -306,15 +337,11 @@ public struct CollectedLinter {
         let superfluousDisableCommandRule = rules.first(where: {
             $0 is SuperfluousDisableCommandRule
         }) as? SuperfluousDisableCommandRule
-        let validationResults: [LintResult] = rules.parallelCompactMap {
-            guard $0.shouldRun(onFile: file) else {
-                return nil
-            }
-
-            return $0.lint(file: file, regions: regions, benchmark: benchmark,
-                           storage: storage,
-                           superfluousDisableCommandRule: superfluousDisableCommandRule,
-                           compilerArguments: compilerArguments)
+        let validationResults: [LintResult] = rules.parallelMap {
+            $0.lint(file: file, regions: regions, benchmark: benchmark,
+                    storage: storage,
+                    superfluousDisableCommandRule: superfluousDisableCommandRule,
+                    compilerArguments: compilerArguments)
         }
         let undefinedSuperfluousCommandViolations = self.undefinedSuperfluousCommandViolations(
             regions: regions, configuration: configuration,
@@ -381,17 +408,20 @@ public struct CollectedLinter {
         }
 
         var corrections = [String: Int]()
-        for rule in rules where rule.shouldRun(onFile: file) {
-            guard let rule = rule as? any CorrectableRule else {
-                continue
+        for rule in rules.compactMap({ $0 as? any CorrectableRule }) {
+            // Set rule context before checking shouldRun to allow file property access
+            let ruleCorrections = CurrentRule.$identifier.withValue(type(of: rule).identifier) { () -> Int? in
+                guard rule.shouldRun(onFile: file) else {
+                    return nil
+                }
+                return rule.correct(file: file, using: storage, compilerArguments: compilerArguments)
             }
-            let corrected = rule.correct(file: file, using: storage, compilerArguments: compilerArguments)
-            if corrected != 0 {
+            if let corrected = ruleCorrections, corrected != 0 {
                 corrections[type(of: rule).description.identifier] = corrected
                 if !file.isVirtual {
                     file.invalidateCache()
                 }
-			}
+            }
         }
         return corrections
     }
diff --git a/Tests/BuiltInRulesTests/FileHeaderRuleTests.swift b/Tests/BuiltInRulesTests/FileHeaderRuleTests.swift
@@ -5,6 +5,12 @@ import XCTest
 private let fixturesDirectory = "\(TestResources.path())/FileHeaderRuleFixtures"
 
 final class FileHeaderRuleTests: SwiftLintTestCase {
+    override func invokeTest() {
+        CurrentRule.$allowSourceKitRequestWithoutRule.withValue(true) {
+            super.invokeTest()
+        }
+    }
+
     private func validate(fileName: String, using configuration: Any) throws -> [StyleViolation] {
         let file = SwiftLintFile(path: fixturesDirectory.stringByAppendingPathComponent(fileName))!
         let rule = try FileHeaderRule(configuration: configuration)
diff --git a/Tests/FrameworkTests/CollectingRuleTests.swift b/Tests/FrameworkTests/CollectingRuleTests.swift
@@ -136,7 +136,7 @@ extension MockCollectingRule {
     @RuleConfigurationDescriptionBuilder
     var configurationDescription: some Documentable { RuleConfigurationOption.noOptions }
     static var description: RuleDescription {
-        RuleDescription(identifier: "test_rule", name: "", description: "", kind: .lint)
+        RuleDescription(identifier: "mock_test_rule_for_swiftlint_tests", name: "", description: "", kind: .lint)
     }
     static var configuration: Configuration? {
         Configuration(rulesMode: .onlyConfiguration([identifier]), ruleList: RuleList(rules: self))
diff --git a/Tests/FrameworkTests/CustomRulesTests.swift b/Tests/FrameworkTests/CustomRulesTests.swift
@@ -11,6 +11,12 @@ 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/EmptyFileTests.swift b/Tests/FrameworkTests/EmptyFileTests.swift
@@ -39,7 +39,7 @@ private struct DontLintEmptyFiles: ShouldLintEmptyFilesProtocol {
     static var shouldLintEmptyFiles: Bool { false }
 }
 
-private struct RuleMock<ShouldLintEmptyFiles: ShouldLintEmptyFilesProtocol>: CorrectableRule {
+private struct RuleMock<ShouldLintEmptyFiles: ShouldLintEmptyFilesProtocol>: CorrectableRule, SourceKitFreeRule {
     var configuration = SeverityConfiguration<Self>(.warning)
 
     static var description: RuleDescription {
diff --git a/Tests/FrameworkTests/SourceKitCrashTests.swift b/Tests/FrameworkTests/SourceKitCrashTests.swift
@@ -2,6 +2,12 @@
 import XCTest
 
 final class SourceKitCrashTests: SwiftLintTestCase {
+    override func invokeTest() {
+        CurrentRule.$allowSourceKitRequestWithoutRule.withValue(true) {
+            super.invokeTest()
+        }
+    }
+
     func testAssertHandlerIsNotCalledOnNormalFile() {
         let file = SwiftLintFile(contents: "A file didn't crash SourceKitService")
         file.sourcekitdFailed = false

Original file line number	Diff line number	Diff line change
`@@ -84,8 +84,7 @@ private extension SwiftLintFile {`
`84`	`84`	`func index(compilerArguments: [String]) -> SourceKittenDictionary? {`
`85`	`85`	`path`
`86`	`86`	`.flatMap { path in`
`87`		`- try? Request.index(file: path, arguments: compilerArguments)`
`88`		`- .send()`
	`87`	`+ try? Request.index(file: path, arguments: compilerArguments).send()`
`89`	`88`	`}`
`90`	`89`	`.map(SourceKittenDictionary.init)`
`91`	`90`	`}`
Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ extension MockCollectingRule {`
`136`	`136`	`@RuleConfigurationDescriptionBuilder`
`137`	`137`	`var configurationDescription: some Documentable { RuleConfigurationOption.noOptions }`
`138`	`138`	`static var description: RuleDescription {`
`139`		`- RuleDescription(identifier: "test_rule", name: "", description: "", kind: .lint)`
	`139`	`+ RuleDescription(identifier: "mock_test_rule_for_swiftlint_tests", name: "", description: "", kind: .lint)`
`140`	`140`	`}`
`141`	`141`	`static var configuration: Configuration? {`
`142`	`142`	`Configuration(rulesMode: .onlyConfiguration([identifier]), ruleList: RuleList(rules: self))`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ private struct DontLintEmptyFiles: ShouldLintEmptyFilesProtocol {`
`39`	`39`	`static var shouldLintEmptyFiles: Bool { false }`
`40`	`40`	`}`
`41`	`41`
`42`		`-private struct RuleMock<ShouldLintEmptyFiles: ShouldLintEmptyFilesProtocol>: CorrectableRule {`
	`42`	`+private struct RuleMock<ShouldLintEmptyFiles: ShouldLintEmptyFilesProtocol>: CorrectableRule, SourceKitFreeRule {`
`43`	`43`	`var configuration = SeverityConfiguration<Self>(.warning)`
`44`	`44`
`45`	`45`	`static var description: RuleDescription {`