Skip to content

When creation of a syntax node using string interpolation failed, log the diagnostics #1790

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,41 +17,8 @@ import Utils

let syntaxExpressibleByStringInterpolationConformancesFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {
DeclSyntax("import SwiftSyntax")
DeclSyntax("import SwiftParser")
DeclSyntax("import SwiftParserDiagnostics")

try! ExtensionDeclSyntax("extension SyntaxParseable") {
DeclSyntax("public typealias StringInterpolation = SyntaxStringInterpolation")

DeclSyntax(
"""
public init(stringInterpolation: SyntaxStringInterpolation) {
self = performParse(source: stringInterpolation.sourceText, parse: { parser in
return Self.parse(from: &parser)
})
}
"""
)
}

for node in SYNTAX_NODES where node.parserFunction != nil {
DeclSyntax("extension \(node.kind.syntaxType): SyntaxExpressibleByStringInterpolation {}")
}

DeclSyntax(
"""
// TODO: This should be inlined in SyntaxParseable.init(stringInterpolation:),
// but is currently used in `ConvenienceInitializers.swift`.
// See the corresponding TODO there.
func performParse<SyntaxType: SyntaxProtocol>(source: [UInt8], parse: (inout Parser) -> SyntaxType) -> SyntaxType {
return source.withUnsafeBufferPointer { buffer in
var parser = Parser(buffer)
// FIXME: When the parser supports incremental parsing, put the
// interpolatedSyntaxNodes in so we don't have to parse them again.
let result = parse(&parser)
return result
}
}
"""
)
}
18 changes: 14 additions & 4 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
import PackageDescription
import Foundation

var swiftSyntaxSwiftSettings: [SwiftSetting] = []
var swiftSyntaxBuilderSwiftSettings: [SwiftSetting] = []
var swiftParserSwiftSettings: [SwiftSetting] = []

/// If we are in a controlled CI environment, we can use internal compiler flags
/// to speed up the build or improve it.
var swiftSyntaxSwiftSettings: [SwiftSetting] = []
if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil {
let groupFile = URL(fileURLWithPath: #file)
.deletingLastPathComponent()
Expand All @@ -21,14 +24,20 @@ if ProcessInfo.processInfo.environment["SWIFT_BUILD_SCRIPT_ENVIRONMENT"] != nil
"-enforce-exclusivity=unchecked",
]),
]
swiftSyntaxBuilderSwiftSettings += [
.define("SWIFTSYNTAX_NO_OSLOG_DEPENDENCY")
]
swiftParserSwiftSettings += [
.define("SWIFTSYNTAX_NO_OSLOG_DEPENDENCY")
]
}

if ProcessInfo.processInfo.environment["SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION"] != nil {
swiftSyntaxSwiftSettings += [
.define("SWIFTSYNTAX_ENABLE_RAWSYNTAX_VALIDATION")
]
}

var swiftParserSwiftSettings: [SwiftSetting] = []
if ProcessInfo.processInfo.environment["SWIFTPARSER_ENABLE_ALTERNATE_TOKEN_INTROSPECTION"] != nil {
swiftParserSwiftSettings += [
.define("SWIFTPARSER_ENABLE_ALTERNATE_TOKEN_INTROSPECTION")
Expand Down Expand Up @@ -162,8 +171,9 @@ let package = Package(

.target(
name: "SwiftSyntaxBuilder",
dependencies: ["SwiftBasicFormat", "SwiftParser", "SwiftParserDiagnostics", "SwiftSyntax"],
exclude: ["CMakeLists.txt"]
dependencies: ["SwiftBasicFormat", "SwiftParser", "SwiftDiagnostics", "SwiftParserDiagnostics", "SwiftSyntax"],
exclude: ["CMakeLists.txt"],
swiftSettings: swiftSyntaxBuilderSwiftSettings
),

.testTarget(
Expand Down
6 changes: 6 additions & 0 deletions Sources/SwiftSyntaxBuilder/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ add_swift_host_library(SwiftSyntaxBuilder
ResultBuilderExtensions.swift
Syntax+StringInterpolation.swift
SyntaxNodeWithBody.swift
SyntaxParsable+ExpressibleByStringInterpolation.swift
ValidatingSyntaxNodes.swift
WithTrailingCommaSyntax+EnsuringTrailingComma.swift

Expand All @@ -22,6 +23,11 @@ add_swift_host_library(SwiftSyntaxBuilder
generated/SyntaxExpressibleByStringInterpolationConformances.swift
)

# Don't depend on OSLog when we are building for the compiler.
# In that case we don't want any dependencies on the SDK.
target_compile_options(SwiftSyntaxBuilder PRIVATE
$<$<COMPILE_LANGUAGE:Swift>:-D;SWIFTSYNTAX_NO_OSLOG_DEPENDENCY>)

target_link_libraries(SwiftSyntaxBuilder PUBLIC
SwiftBasicFormat
SwiftParser
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import SwiftDiagnostics
import SwiftSyntax
import SwiftParser
import SwiftParserDiagnostics
// Don't introduce a dependency on OSLog when building SwiftSyntax using CMake
// for the compiler.
#if canImport(OSLog) && !SWIFTSYNTAX_NO_OSLOG_DEPENDENCY
import OSLog
#endif

extension SyntaxParseable {
public typealias StringInterpolation = SyntaxStringInterpolation

/// Assuming that this node contains a syntax error, log it using OSLog if we
/// are on a platform that supports OSLog, otherwise don't do anything.
private func logStringInterpolationParsingError() {
#if canImport(OSLog) && !SWIFTSYNTAX_NO_OSLOG_DEPENDENCY
if #available(macOS 11.0, *) {
let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: self)
let formattedDiagnostics = DiagnosticsFormatter().annotatedSource(tree: self, diags: diagnostics)
Logger(subsystem: "SwiftSyntax", category: "ParseError").fault(
"""
Parsing a `\(Self.self)` node from string interpolation produced the following parsing errors.
Set a breakpoint in `SyntaxParseable.logStringInterpolationParsingError()` to debug the failure.
\(formattedDiagnostics)
"""
)
}
#endif
}

/// Initialize the syntax node from a string interpolation.
///
/// - Important: This asssumes that the string interpolation produces a valid
/// syntax tree. If the syntax tree is not valid, a fault will
/// be logged using OSLog on Darwin platforms.
public init(stringInterpolation: SyntaxStringInterpolation) {
self = stringInterpolation.sourceText.withUnsafeBufferPointer { buffer in
var parser = Parser(buffer)
// FIXME: When the parser supports incremental parsing, put the
// interpolatedSyntaxNodes in so we don't have to parse them again.
let result = Self.parse(from: &parser)
return result
}
if self.hasError {
self.logStringInterpolationParsingError()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,6 @@
//===----------------------------------------------------------------------===//

import SwiftSyntax
import SwiftParser
import SwiftParserDiagnostics

extension SyntaxParseable {
public typealias StringInterpolation = SyntaxStringInterpolation

public init(stringInterpolation: SyntaxStringInterpolation) {
self = performParse(source: stringInterpolation.sourceText, parse: { parser in
return Self.parse(from: &parser)
})
}
}

extension AccessorDeclSyntax: SyntaxExpressibleByStringInterpolation {}

Expand Down Expand Up @@ -57,16 +45,3 @@ extension StmtSyntax: SyntaxExpressibleByStringInterpolation {}
extension SwitchCaseSyntax: SyntaxExpressibleByStringInterpolation {}

extension TypeSyntax: SyntaxExpressibleByStringInterpolation {}

// TODO: This should be inlined in SyntaxParseable.init(stringInterpolation:),
// but is currently used in `ConvenienceInitializers.swift`.
// See the corresponding TODO there.
func performParse<SyntaxType: SyntaxProtocol>(source: [UInt8], parse: (inout Parser) -> SyntaxType) -> SyntaxType {
return source.withUnsafeBufferPointer { buffer in
var parser = Parser(buffer)
// FIXME: When the parser supports incremental parsing, put the
// interpolatedSyntaxNodes in so we don't have to parse them again.
let result = parse(&parser)
return result
}
}
43 changes: 41 additions & 2 deletions Tests/SwiftParserTest/LinkageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,27 @@ final class LinkageTest: XCTestCase {
.library("-lswiftCompatibilityConcurrency"),
.library("-lswiftCompatibilityPacks", condition: .mayBeAbsent("Only in newer compilers")),
.library("-lswiftCore"),
.library("-lswiftCoreFoundation", condition: .when(compilationCondition: .osLogDependency)),
Copy link
Contributor

Choose a reason for hiding this comment

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

We could just only run this check when we don't have the oslog dependency. We don't really care what's linked if we do.

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean put the entire test behind #if canImport(OSLog) && !SWIFTSYNTAX_NO_OSLOG_DEPENDENCY?

This test is starting to get a little odd because in practice we only ever run this test using swift test on macOS where we don’t set SWIFTSYNTAX_NO_OSLOG_DEPENDENCY, so it does raise a question about its value.

.library("-lswiftDarwin", condition: .when(compilationCondition: .osLogDependency)),
.library("-lswiftDispatch", condition: .when(compilationCondition: .osLogDependency)),
.library("-lswiftFoundation", condition: .when(compilationCondition: .osLogDependency)),
.library("-lswiftIOKit", condition: .when(compilationCondition: .osLogDependency)),
.library("-lswiftOSLog", condition: .when(compilationCondition: .osLogDependency)),
.library("-lswiftObjectiveC", condition: .when(compilationCondition: .osLogDependency)),
.library("-lswiftSwiftOnoneSupport", condition: .when(configuration: .debug)),
.library("-lswiftXPC", condition: .when(compilationCondition: .osLogDependency)),
.library("-lswift_Concurrency"),
.library("-lswift_StringProcessing", condition: .mayBeAbsent("Starting in Xcode 14 this library is not always autolinked")),
.library("-lswiftos", condition: .when(compilationCondition: .osLogDependency)),
.framework("CFNetwork", condition: .when(compilationCondition: .osLogDependency)),
.framework("Combine", condition: .when(compilationCondition: .osLogDependency)),
.framework("CoreFoundation", condition: .when(compilationCondition: .osLogDependency)),
.framework("CoreServices", condition: .when(compilationCondition: .osLogDependency)),
.framework("DiskArbitration", condition: .when(compilationCondition: .osLogDependency)),
.framework("Foundation", condition: .when(compilationCondition: .osLogDependency)),
.framework("IOKit", condition: .when(compilationCondition: .osLogDependency)),
.framework("OSLog", condition: .when(compilationCondition: .osLogDependency)),
.framework("Security", condition: .when(compilationCondition: .osLogDependency)),
]
)

Expand Down Expand Up @@ -120,7 +138,9 @@ extension LinkageTest {
private func assertLinkage(
of library: String,
in bundle: EnclosingTestBundle,
assertions: [Linkage.Assertion]
assertions: [Linkage.Assertion],
file: StaticString = #file,
line: UInt = #line
) throws {
let linkages = try bundle.objectFiles(for: library).reduce(into: []) { acc, next in
acc += try self.extractAutolinkingHints(in: next)
Expand Down Expand Up @@ -175,7 +195,11 @@ extension LinkageTest {
}

for superfluousLinkage in sortedLinkages[expectedLinkagesIdx..<sortedLinkages.endIndex] {
XCTFail("Found unasserted link-time dependency: \(superfluousLinkage.linkage)")
XCTFail(
"Found unasserted link-time dependency: \(superfluousLinkage.linkage)",
file: file,
line: line
)
}
}

Expand Down Expand Up @@ -314,6 +338,7 @@ extension Linkage.Assertion {
fileprivate enum Condition {
case swiftVersionAtLeast(versionBound: SwiftVersion)
case configuration(ProductConfiguration)
case compilationCondition(CompilationCondition)
case flaky

enum SwiftVersion: Comparable {
Expand All @@ -328,6 +353,10 @@ extension Linkage.Assertion {
case release
}

enum CompilationCondition {
case osLogDependency
}

fileprivate static func when(swiftVersionAtLeast version: SwiftVersion) -> Condition {
return .swiftVersionAtLeast(versionBound: version)
}
Expand All @@ -336,6 +365,10 @@ extension Linkage.Assertion {
return .configuration(configuration)
}

fileprivate static func when(compilationCondition: CompilationCondition) -> Condition {
return .compilationCondition(compilationCondition)
}

fileprivate static func mayBeAbsent(_ reason: StaticString) -> Condition {
return .flaky
}
Expand All @@ -360,6 +393,12 @@ extension Linkage.Assertion {
let config: ProductConfiguration = .release
#endif
return config == expectation
case .compilationCondition(.osLogDependency):
#if SWIFTSYNTAX_NO_OSLOG_DEPENDENCY
return false
#else
return true
#endif
case .flaky:
return true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ public class StringLiteralRepresentedLiteralValueTests: XCTestCase {
// MARK: literal value not available

func testMissingQuoteStringLiteral() throws {
let stringLiteral = StringLiteralExprSyntax(#""a"# as ExprSyntax)!
var parser = Parser(#""a"#)
let stringLiteral = StringLiteralExprSyntax(ExprSyntax.parse(from: &parser))!
XCTAssertNil(stringLiteral.representedLiteralValue, "only fully parsed string literals should produce a literal value")
}

Expand All @@ -260,7 +261,8 @@ public class StringLiteralRepresentedLiteralValueTests: XCTestCase {
}

func testMalformedMultiLineStringLiteral() throws {
let stringLiteral = StringLiteralExprSyntax(#""""a""""# as ExprSyntax)!
var parser = Parser(#""""a""""#)
let stringLiteral = StringLiteralExprSyntax(ExprSyntax.parse(from: &parser))!
XCTAssertNil(stringLiteral.representedLiteralValue, "missing newline in multiline string literal cannot produce a literal value")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import SwiftBasicFormat
import SwiftRefactor
import SwiftParser
import SwiftSyntax
import SwiftSyntaxBuilder
import XCTest
Expand Down Expand Up @@ -100,7 +101,8 @@ fileprivate func assertRefactorPlaceholder(
if wrap {
token = "\(raw: ExpandEditorPlaceholder.wrapInPlaceholder(placeholder))"
} else {
let expr: ExprSyntax = "\(raw: placeholder)"
var parser = Parser(placeholder)
let expr = ExprSyntax.parse(from: &parser)
token = try XCTUnwrap(expr.as(EditorPlaceholderExprSyntax.self)?.identifier, file: file, line: line)
}

Expand Down
6 changes: 4 additions & 2 deletions Tests/SwiftRefactorTest/ExpandEditorPlaceholdersTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ fileprivate func assertRefactorPlaceholderCall(
file: StaticString = #file,
line: UInt = #line
) throws {
let call = try XCTUnwrap(ExprSyntax("\(raw: expr)").as(FunctionCallExprSyntax.self), file: file, line: line)
var parser = Parser(expr)
let call = try XCTUnwrap(ExprSyntax.parse(from: &parser).as(FunctionCallExprSyntax.self), file: file, line: line)
let arg = try XCTUnwrap(call.argumentList[placeholder].as(TupleExprElementSyntax.self), file: file, line: line)
let token: TokenSyntax = try XCTUnwrap(arg.expression.as(EditorPlaceholderExprSyntax.self), file: file, line: line).identifier

Expand All @@ -123,7 +124,8 @@ fileprivate func assertRefactorPlaceholderToken(
file: StaticString = #file,
line: UInt = #line
) throws {
let call = try XCTUnwrap(ExprSyntax("\(raw: expr)").as(FunctionCallExprSyntax.self), file: file, line: line)
var parser = Parser(expr)
let call = try XCTUnwrap(ExprSyntax.parse(from: &parser).as(FunctionCallExprSyntax.self), file: file, line: line)
let arg = try XCTUnwrap(call.argumentList[placeholder].as(TupleExprElementSyntax.self), file: file, line: line)
let token: TokenSyntax = try XCTUnwrap(arg.expression.as(EditorPlaceholderExprSyntax.self), file: file, line: line).identifier

Expand Down
Loading