Enforce SourceKitFreeRule contract with fatal error (#6107)

This commit is contained in:
JP Simard
2025-06-20 07:25:01 -04:00
committed by GitHub
parent 873b2b66e6
commit 81474e36d0
10 changed files with 117 additions and 26 deletions
@@ -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)
}
@@ -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`.")
}
@@ -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
}
@@ -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)")
}
+51 -21
View File
@@ -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
}
@@ -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)
@@ -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))
@@ -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": [
+1 -1
View File
@@ -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 {
@@ -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