Skip to content

Commit

Permalink
Avoid making SourceKit requests to get parser diagnostics (realm#4286)
Browse files Browse the repository at this point in the history
By using SwiftSyntax instead, we avoid the overhead of a SourceKit
request, which has a significant impact if none of the rules being
corrected use SourceKit.
  • Loading branch information
jpsim authored Oct 5, 2022
1 parent 41b8834 commit 44382ea
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 21 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@
[SimplyDanny](https://github.com/SimplyDanny)
[#4202](https://github.com/realm/SwiftLint/issues/4202)

* Use SwiftSyntax instead of SourceKit to determine if a file has parser errors
before applying corrections. This speeds up corrections significantly when
none of the rules use SourceKit.
[JP Simard](https://github.com/jpims)

#### Bug Fixes

* Respect `validates_start_with_lowercase` option when linting function names.
Expand Down
10 changes: 6 additions & 4 deletions Source/SwiftLintFramework/Extensions/SwiftLintFile+Cache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,20 +155,22 @@ extension SwiftLintFile {
}
}

internal var parserDiagnostics: [[String: SourceKitRepresentable]]? {
internal var parserDiagnostics: [String]? {
if parserDiagnosticsDisabledForTests {
return nil
}

guard let response = responseCache.get(self) else {
guard let syntaxTree = syntaxTree else {
if let handler = assertHandler {
handler()
return nil
}
queuedFatalError("Never call this for file that sourcekitd fails.")
queuedFatalError("Could not get diagnostics for file.")
}

return response["key.diagnostics"] as? [[String: SourceKitRepresentable]]
return ParseDiagnosticsGenerator.diagnostics(for: syntaxTree)
.filter { $0.diagMessage.severity == .error }
.map(\.message)
}

internal var structure: Structure {
Expand Down
18 changes: 6 additions & 12 deletions Source/SwiftLintFramework/Models/Linter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -283,18 +283,12 @@ public struct CollectedLinter {
return []
}

if let parserDiagnostics = file.parserDiagnostics {
let errorDiagnostics = parserDiagnostics.filter { diagnostic in
diagnostic["key.severity"] as? String == "source.diagnostic.severity.error"
}

if errorDiagnostics.isNotEmpty {
queuedPrintError(
"Skipping correcting file because it produced Swift parser errors: \(file.path ?? "<nopath>")"
)
queuedPrintError(toJSON(["diagnostics": errorDiagnostics]))
return []
}
if let parserDiagnostics = file.parserDiagnostics, parserDiagnostics.isNotEmpty {
queuedPrintError(
"Skipping correcting file because it produced Swift parser errors: \(file.path ?? "<nopath>")"
)
queuedPrintError(toJSON(["diagnostics": parserDiagnostics]))
return []
}

var corrections = [Correction]()
Expand Down
9 changes: 4 additions & 5 deletions Tests/SwiftLintFrameworkTests/ParserDiagnosticsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ final class ParserDiagnosticsTests: XCTestCase {

func testFileWithParserErrorDiagnosticsDoesntAutocorrect() throws {
let contents = """
importz Foundation
print(CGPointZero)
print(CGPointZero))
"""
XCTAssertNotNil(SwiftLintFile(contents: contents).parserDiagnostics)
XCTAssertEqual(SwiftLintFile(contents: contents).parserDiagnostics, ["extraneous \')\' at top level"])

let ruleDescription = LegacyConstantRule.description
.with(corrections: [Example(contents): Example(contents)])
Expand All @@ -33,7 +32,7 @@ final class ParserDiagnosticsTests: XCTestCase {
func foo(bar bar: String) -> Int { 0 }
"""

XCTAssertNotNil(SwiftLintFile(contents: original).parserDiagnostics)
XCTAssertEqual(SwiftLintFile(contents: original).parserDiagnostics, [])

let ruleDescription = ReturnArrowWhitespaceRule.description
.with(corrections: [Example(original): Example(corrected)])
Expand All @@ -45,6 +44,6 @@ final class ParserDiagnosticsTests: XCTestCase {

func testFileWithoutParserDiagnostics() {
parserDiagnosticsDisabledForTests = false
XCTAssertNil(SwiftLintFile(contents: "import Foundation").parserDiagnostics)
XCTAssertEqual(SwiftLintFile(contents: "import Foundation").parserDiagnostics, [])
}
}

0 comments on commit 44382ea

Please sign in to comment.