Skip to content

Fix ignore-unparsable-files flag so that it works. #425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 12, 2022
Merged

Fix ignore-unparsable-files flag so that it works. #425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,6 @@ let package = Package(
]
),

.testTarget(
name: "SwiftFormatTests",
dependencies: [
"SwiftFormat",
.product(name: "SwiftSyntax", package: "swift-syntax"),
.product(name: "SwiftParser", package: "swift-syntax"),
]
),
.testTarget(
name: "SwiftFormatConfigurationTests",
dependencies: ["SwiftFormatConfiguration"]
Expand Down
6 changes: 5 additions & 1 deletion Sources/SwiftFormat/Parsing.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,19 @@ func parseAndEmitDiagnostics(
let sourceFile =
try operatorTable.foldAll(Parser.parse(source: source)) { _ in }.as(SourceFileSyntax.self)!

let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: sourceFile)
if let parsingDiagnosticHandler = parsingDiagnosticHandler {
let expectedConverter =
SourceLocationConverter(file: url?.path ?? "<unknown>", tree: sourceFile)
let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: sourceFile)
for diagnostic in diagnostics {
let location = diagnostic.location(converter: expectedConverter)
parsingDiagnosticHandler(diagnostic, location)
}
}

guard diagnostics.isEmpty else {
throw SwiftFormatError.fileContainsInvalidSyntax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should eventually ditch the parsingDiagnosticsConsumer model and just throw the array of diagnostics as part of this error, since we don't plan on doing anything else after that.

That's a bigger API change though so we can save it for later, to make sure there aren't any other weird side effects.

}

return sourceFile
}
2 changes: 1 addition & 1 deletion Sources/SwiftFormat/SwiftFormatError.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ public enum SwiftFormatError: Error {
case isDirectory

/// The file contains invalid or unrecognized Swift syntax and cannot be handled safely.
case fileContainsInvalidSyntax(position: AbsolutePosition)
case fileContainsInvalidSyntax
}
4 changes: 0 additions & 4 deletions Sources/SwiftFormat/SwiftFormatter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,6 @@ public final class SwiftFormatter {
syntax: SourceFileSyntax, operatorTable: OperatorTable,
assumingFileURL url: URL?, source: String?, to outputStream: inout Output
) throws {
if let position = _firstInvalidSyntaxPosition(in: Syntax(syntax)) {
throw SwiftFormatError.fileContainsInvalidSyntax(position: position)
}

let assumedURL = url ?? URL(fileURLWithPath: "source")
let context = Context(
configuration: configuration, operatorTable: operatorTable, findingConsumer: findingConsumer,
Expand Down
4 changes: 0 additions & 4 deletions Sources/SwiftFormat/SwiftLinter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,6 @@ public final class SwiftLinter {
assumingFileURL url: URL,
source: String?
) throws {
if let position = _firstInvalidSyntaxPosition(in: Syntax(syntax)) {
throw SwiftFormatError.fileContainsInvalidSyntax(position: position)
}

let context = Context(
configuration: configuration, operatorTable: operatorTable, findingConsumer: findingConsumer,
fileURL: url, sourceFileSyntax: syntax, source: source, ruleNameCache: ruleNameCache)
Expand Down
75 changes: 0 additions & 75 deletions Sources/SwiftFormat/SyntaxValidatingVisitor.swift
View file
Open in desktop

This file was deleted.

18 changes: 12 additions & 6 deletions Sources/swift-format/Frontend/FormatFrontend.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//

import Foundation
import SwiftDiagnostics
import SwiftFormat
import SwiftFormatConfiguration
import SwiftSyntax
Expand Down Expand Up @@ -39,6 +40,13 @@ class FormatFrontend: Frontend {
return
}

let diagnosticHandler: (Diagnostic, SourceLocation) -> () = { (diagnostic, location) in
guard !self.lintFormatOptions.ignoreUnparsableFiles else {
// No diagnostics should be emitted in this mode.
return
}
self.diagnosticsEngine.consumeParserDiagnostic(diagnostic, location)
}
var stdoutStream = FileHandleTextOutputStream(FileHandle.standardOutput)
do {
if inPlace {
Expand All @@ -47,7 +55,7 @@ class FormatFrontend: Frontend {
source: source,
assumingFileURL: url,
to: &buffer,
parsingDiagnosticHandler: diagnosticsEngine.consumeParserDiagnostic)
parsingDiagnosticHandler: diagnosticHandler)

if buffer != source {
let bufferData = buffer.data(using: .utf8)! // Conversion to UTF-8 cannot fail
Expand All @@ -58,13 +66,13 @@ class FormatFrontend: Frontend {
source: source,
assumingFileURL: url,
to: &stdoutStream,
parsingDiagnosticHandler: diagnosticsEngine.consumeParserDiagnostic)
parsingDiagnosticHandler: diagnosticHandler)
}
} catch SwiftFormatError.fileNotReadable {
diagnosticsEngine.emitError(
"Unable to format \(url.relativePath): file is not readable or does not exist.")
return
} catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
} catch SwiftFormatError.fileContainsInvalidSyntax {
guard !lintFormatOptions.ignoreUnparsableFiles else {
guard !inPlace else {
// For in-place mode, nothing is expected to stdout and the file shouldn't be modified.
Expand All @@ -73,9 +81,7 @@ class FormatFrontend: Frontend {
stdoutStream.write(source)
return
}
let location = SourceLocationConverter(file: url.path, source: source).location(for: position)
diagnosticsEngine.emitError(
"file contains invalid or unrecognized Swift syntax.", location: location)
// Otherwise, relevant diagnostics about the problematic nodes have been emitted.
return
} catch {
diagnosticsEngine.emitError("Unable to format \(url.relativePath): \(error)")
Expand Down
17 changes: 11 additions & 6 deletions Sources/swift-format/Frontend/LintFrontend.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//

import Foundation
import SwiftDiagnostics
import SwiftFormat
import SwiftFormatConfiguration
import SwiftSyntax
Expand All @@ -32,20 +33,24 @@ class LintFrontend: Frontend {
do {
try linter.lint(
source: source,
assumingFileURL: url,
parsingDiagnosticHandler: diagnosticsEngine.consumeParserDiagnostic)
assumingFileURL: url) { (diagnostic, location) in
guard !self.lintFormatOptions.ignoreUnparsableFiles else {
// No diagnostics should be emitted in this mode.
return
}
self.diagnosticsEngine.consumeParserDiagnostic(diagnostic, location)
}

} catch SwiftFormatError.fileNotReadable {
diagnosticsEngine.emitError(
"Unable to lint \(url.relativePath): file is not readable or does not exist.")
return
} catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
} catch SwiftFormatError.fileContainsInvalidSyntax {
guard !lintFormatOptions.ignoreUnparsableFiles else {
// The caller wants to silently ignore this error.
return
}
let location = SourceLocationConverter(file: url.path, source: source).location(for: position)
diagnosticsEngine.emitError(
"file contains invalid or unrecognized Swift syntax.", location: location)
// Otherwise, relevant diagnostics about the problematic nodes have been emitted.
return
} catch {
diagnosticsEngine.emitError("Unable to lint \(url.relativePath): \(error)")
Expand Down
28 changes: 0 additions & 28 deletions Tests/SwiftFormatTests/SyntaxValidatingVisitorTests.swift
View file
Open in desktop

This file was deleted.