Skip to content

Disallow whitespace between @, attribute name and ( #2466

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
merged 1 commit into from
Feb 24, 2024
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
18 changes: 16 additions & 2 deletions Sources/SwiftParser/Attributes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,14 @@ extension Parser {
argumentMode: AttributeArgumentMode,
parseArguments: (inout Parser) -> RawAttributeSyntax.Arguments
) -> RawAttributeListSyntax.Element {
let (unexpectedBeforeAtSign, atSign) = self.expect(.atSign)
var (unexpectedBeforeAtSign, atSign) = self.expect(.atSign)
if atSign.trailingTriviaByteLength > 0 || self.currentToken.leadingTriviaByteLength > 0 {
let diagnostic = TokenDiagnostic(
self.swiftVersion < .v6 ? .extraneousTrailingWhitespaceWarning : .extraneousTrailingWhitespaceError,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should implement .warnUntilSwiftVersion() in SwiftSyntax too?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don’t think that’s realistically possible. We need to have a different case in SwiftSyntax.TokenDiagnostic so that one of them can be marked as an error and the other one as a warning.

byteOffset: atSign.leadingTriviaByteLength + atSign.tokenText.count
)
atSign = atSign.tokenView.withTokenDiagnostic(tokenDiagnostic: diagnostic, arena: self.arena)
}
let attributeName = self.parseAttributeName()
let shouldParseArgument: Bool
switch argumentMode {
Expand All @@ -190,7 +197,14 @@ extension Parser {
shouldParseArgument = false
}
if shouldParseArgument {
let (unexpectedBeforeLeftParen, leftParen) = self.expect(.leftParen)
var (unexpectedBeforeLeftParen, leftParen) = self.expect(.leftParen)
if unexpectedBeforeLeftParen == nil && (attributeName.raw.trailingTriviaByteLength > 0 || leftParen.leadingTriviaByteLength > 0) {
let diagnostic = TokenDiagnostic(
self.swiftVersion < .v6 ? .extraneousLeadingWhitespaceWarning : .extraneousLeadingWhitespaceError,
byteOffset: 0
)
leftParen = leftParen.tokenView.withTokenDiagnostic(tokenDiagnostic: diagnostic, arena: self.arena)
Copy link
Member

@rintaro rintaro Feb 5, 2024

Choose a reason for hiding this comment

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

I think I understand why you did this , but the inconsistency of the diagnosing token between #/@ and ( is a little unfortunate. 😢

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it truly is. But I think everything else would be even more of a pain…

}
let argument = parseArguments(&self)
let (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen)
return .attribute(
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftParser/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ add_swift_syntax_library(SwiftParser
Recovery.swift
Specifiers.swift
Statements.swift
StringLiterals.swift
StringLiteralRepresentedLiteralValue.swift
StringLiterals.swift
SwiftVersion.swift
SyntaxUtils.swift
TokenConsumer.swift
TokenPrecedence.swift
Expand Down
19 changes: 9 additions & 10 deletions Sources/SwiftParser/Declarations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1928,19 +1928,18 @@ extension Parser {
) -> RawMacroExpansionDeclSyntax {

var (unexpectedBeforePound, pound) = self.eat(handle)
if pound.trailingTriviaByteLength != 0 {
// `#` and the macro name must not be separated by a newline.
unexpectedBeforePound = RawUnexpectedNodesSyntax(combining: unexpectedBeforePound, pound, arena: self.arena)
pound = RawTokenSyntax(missing: .pound, text: "#", leadingTriviaPieces: pound.leadingTriviaPieces, arena: self.arena)
if pound.trailingTriviaByteLength > 0 || currentToken.leadingTriviaByteLength > 0 {
// If there are whitespaces after '#' diagnose.
let diagnostic = TokenDiagnostic(
.extraneousTrailingWhitespaceError,
byteOffset: pound.leadingTriviaByteLength + pound.tokenText.count
)
pound = pound.tokenView.withTokenDiagnostic(tokenDiagnostic: diagnostic, arena: self.arena)
}
var unexpectedBeforeMacro: RawUnexpectedNodesSyntax?
var macro: RawTokenSyntax
let unexpectedBeforeMacro: RawUnexpectedNodesSyntax?
let macro: RawTokenSyntax
if !self.atStartOfLine {
(unexpectedBeforeMacro, macro) = self.expectIdentifier(allowKeywordsAsIdentifier: true)
if macro.leadingTriviaByteLength != 0 {
unexpectedBeforeMacro = RawUnexpectedNodesSyntax(combining: unexpectedBeforeMacro, macro, arena: self.arena)
pound = self.missingToken(.identifier, text: macro.tokenText)
}
} else {
unexpectedBeforeMacro = nil
macro = self.missingToken(.identifier)
Expand Down
18 changes: 8 additions & 10 deletions Sources/SwiftParser/Expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1283,20 +1283,18 @@ extension Parser {
flavor: ExprFlavor
) -> RawMacroExpansionExprSyntax {
var (unexpectedBeforePound, pound) = self.expect(.pound)
if pound.trailingTriviaByteLength != 0 {
if pound.trailingTriviaByteLength > 0 || currentToken.leadingTriviaByteLength > 0 {
// If there are whitespaces after '#' diagnose.
unexpectedBeforePound = RawUnexpectedNodesSyntax(combining: unexpectedBeforePound, pound, arena: self.arena)
pound = self.missingToken(.pound)
let diagnostic = TokenDiagnostic(
.extraneousTrailingWhitespaceError,
byteOffset: pound.leadingTriviaByteLength + pound.tokenText.count
)
pound = pound.tokenView.withTokenDiagnostic(tokenDiagnostic: diagnostic, arena: self.arena)
Comment on lines -1288 to +1292
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, why you decided to go with TokenDiagnostic instead of the previous "unexpected" route?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the unexpected route didn’t have any way of representing warnings, just errors. If there are unexpected or missing tokens in the syntax tree, we always consider the tree as having an error (which makes sense to me) and TokenDiagnostic is the only way we can currently communicate warnings.

}
var unexpectedBeforeMacroName: RawUnexpectedNodesSyntax?
var macroName: RawTokenSyntax
let unexpectedBeforeMacroName: RawUnexpectedNodesSyntax?
let macroName: RawTokenSyntax
if !self.atStartOfLine {
(unexpectedBeforeMacroName, macroName) = self.expectIdentifier(allowKeywordsAsIdentifier: true)
if macroName.leadingTriviaByteLength != 0 {
// If there're whitespaces after '#' diagnose.
unexpectedBeforeMacroName = RawUnexpectedNodesSyntax(combining: unexpectedBeforeMacroName, macroName, arena: self.arena)
pound = self.missingToken(.identifier, text: macroName.tokenText)
}
} else {
unexpectedBeforeMacroName = nil
macroName = self.missingToken(.identifier)
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftParser/Lexer/Lexeme.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ extension Lexer {
SyntaxText(baseAddress: start, count: byteLength)
}

var textRange: Range<SyntaxText.Index> {
@_spi(Testing)
public var textRange: Range<SyntaxText.Index> {
leadingTriviaByteLength..<leadingTriviaByteLength + textByteLength
}

Expand Down
7 changes: 7 additions & 0 deletions Sources/SwiftParser/Lookahead.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,30 @@ extension Parser {
/// i.e. how far it looked ahead.
var tokensConsumed: Int = 0

/// The Swift version as which this source file should be parsed.
let swiftVersion: SwiftVersion

/// The experimental features that have been enabled in the underlying
/// parser.
let experimentalFeatures: ExperimentalFeatures

private init(
lexemes: Lexer.LexemeSequence,
currentToken: Lexer.Lexeme,
swiftVersion: SwiftVersion,
experimentalFeatures: ExperimentalFeatures
) {
self.lexemes = lexemes
self.currentToken = currentToken
self.swiftVersion = swiftVersion
self.experimentalFeatures = experimentalFeatures
}

fileprivate init(cloning other: Parser) {
self.init(
lexemes: other.lexemes,
currentToken: other.currentToken,
swiftVersion: other.swiftVersion,
experimentalFeatures: other.experimentalFeatures
)
}
Expand All @@ -55,6 +61,7 @@ extension Parser {
return Lookahead(
lexemes: self.lexemes,
currentToken: self.currentToken,
swiftVersion: self.swiftVersion,
experimentalFeatures: self.experimentalFeatures
)
}
Expand Down
8 changes: 5 additions & 3 deletions Sources/SwiftParser/ParseSourceFile.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,21 @@ extension Parser {
@_spi(ExperimentalLanguageFeatures)
public static func parse(
source: UnsafeBufferPointer<UInt8>,
swiftVersion: SwiftVersion? = nil,
experimentalFeatures: ExperimentalFeatures
) -> SourceFileSyntax {
var parser = Parser(source, experimentalFeatures: experimentalFeatures)
var parser = Parser(source, swiftVersion: swiftVersion, experimentalFeatures: experimentalFeatures)
return SourceFileSyntax.parse(from: &parser)
}

/// Parse the source code in the given buffer as Swift source file. See
/// `Parser.init` for more details.
public static func parse(
source: UnsafeBufferPointer<UInt8>,
maximumNestingLevel: Int? = nil
maximumNestingLevel: Int? = nil,
swiftVersion: SwiftVersion? = nil
) -> SourceFileSyntax {
var parser = Parser(source, maximumNestingLevel: maximumNestingLevel)
var parser = Parser(source, maximumNestingLevel: maximumNestingLevel, swiftVersion: swiftVersion)
return SourceFileSyntax.parse(from: &parser)
}

Expand Down
28 changes: 23 additions & 5 deletions Sources/SwiftParser/Parser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ public struct Parser {
/// Parser should own a ``LookaheadTracker`` so that we can share one `furthestOffset` in a parse.
let lookaheadTrackerOwner: LookaheadTrackerOwner

/// The Swift version as which this source file should be parsed.
let swiftVersion: SwiftVersion

/// The experimental features that have been enabled.
let experimentalFeatures: ExperimentalFeatures

Expand All @@ -129,6 +132,9 @@ public struct Parser {
static let defaultMaximumNestingLevel = 256
#endif

/// The Swift version as which source files should be parsed if no Swift version is explicitly specified in the parser.
static let defaultSwiftVersion: SwiftVersion = .v6

var _emptyRawMultipleTrailingClosureElementListSyntax: RawMultipleTrailingClosureElementListSyntax?

/// Create an empty collection of the given type.
Expand Down Expand Up @@ -187,26 +193,28 @@ public struct Parser {
/// arena is created automatically, and `input` copied into the
/// arena. If non-`nil`, `input` must be within its registered
/// source buffer or allocator.
/// - swiftVersion: The version of Swift using which the file should be parsed.
/// Defaults to the latest version.
/// - experimentalFeatures: The experimental features enabled for the parser.
private init(
buffer input: UnsafeBufferPointer<UInt8>,
maximumNestingLevel: Int?,
parseTransition: IncrementalParseTransition?,
arena: ParsingSyntaxArena?,
swiftVersion: SwiftVersion?,
experimentalFeatures: ExperimentalFeatures
) {
var input = input
if let arena {
self.arena = arena
precondition(arena.contains(text: SyntaxText(baseAddress: input.baseAddress, count: input.count)))
} else {
self.arena = ParsingSyntaxArena(
parseTriviaFunction: TriviaParser.parseTrivia(_:position:)
)
self.arena = ParsingSyntaxArena(parseTriviaFunction: TriviaParser.parseTrivia)
input = self.arena.internSourceBuffer(input)
}

self.maximumNestingLevel = maximumNestingLevel ?? Self.defaultMaximumNestingLevel
self.swiftVersion = swiftVersion ?? Self.defaultSwiftVersion
self.experimentalFeatures = experimentalFeatures
self.lookaheadTrackerOwner = LookaheadTrackerOwner()

Expand All @@ -224,6 +232,7 @@ public struct Parser {
string input: String,
maximumNestingLevel: Int?,
parseTransition: IncrementalParseTransition?,
swiftVersion: SwiftVersion?,
experimentalFeatures: ExperimentalFeatures
) {
var input = input
Expand All @@ -234,6 +243,7 @@ public struct Parser {
maximumNestingLevel: maximumNestingLevel,
parseTransition: parseTransition,
arena: nil,
swiftVersion: swiftVersion,
experimentalFeatures: experimentalFeatures
)
}
Expand All @@ -243,13 +253,15 @@ public struct Parser {
public init(
_ input: String,
maximumNestingLevel: Int? = nil,
parseTransition: IncrementalParseTransition? = nil
parseTransition: IncrementalParseTransition? = nil,
swiftVersion: SwiftVersion? = nil
) {
// Chain to the private String initializer.
self.init(
string: input,
maximumNestingLevel: maximumNestingLevel,
parseTransition: parseTransition,
swiftVersion: swiftVersion,
experimentalFeatures: []
)
}
Expand Down Expand Up @@ -277,14 +289,16 @@ public struct Parser {
_ input: UnsafeBufferPointer<UInt8>,
maximumNestingLevel: Int? = nil,
parseTransition: IncrementalParseTransition? = nil,
arena: ParsingSyntaxArena? = nil
arena: ParsingSyntaxArena? = nil,
swiftVersion: SwiftVersion? = nil
) {
// Chain to the private buffer initializer.
self.init(
buffer: input,
maximumNestingLevel: maximumNestingLevel,
parseTransition: parseTransition,
arena: arena,
swiftVersion: swiftVersion,
experimentalFeatures: []
)
}
Expand All @@ -296,13 +310,15 @@ public struct Parser {
_ input: String,
maximumNestingLevel: Int? = nil,
parseTransition: IncrementalParseTransition? = nil,
swiftVersion: SwiftVersion? = nil,
experimentalFeatures: ExperimentalFeatures
) {
// Chain to the private String initializer.
self.init(
string: input,
maximumNestingLevel: maximumNestingLevel,
parseTransition: parseTransition,
swiftVersion: swiftVersion,
experimentalFeatures: experimentalFeatures
)
}
Expand All @@ -315,6 +331,7 @@ public struct Parser {
maximumNestingLevel: Int? = nil,
parseTransition: IncrementalParseTransition? = nil,
arena: ParsingSyntaxArena? = nil,
swiftVersion: SwiftVersion? = nil,
experimentalFeatures: ExperimentalFeatures
) {
// Chain to the private buffer initializer.
Expand All @@ -323,6 +340,7 @@ public struct Parser {
maximumNestingLevel: maximumNestingLevel,
parseTransition: parseTransition,
arena: arena,
swiftVersion: swiftVersion,
experimentalFeatures: experimentalFeatures
)
}
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftParser/StringLiterals.swift
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,11 @@ fileprivate class StringLiteralExpressionIndentationChecker {
// error is fixed
return nil
}
return token.tokenView.withTokenDiagnostic(
let tokenWithDiagnostic = token.tokenView.withTokenDiagnostic(
tokenDiagnostic: TokenDiagnostic(.insufficientIndentationInMultilineStringLiteral, byteOffset: 0),
arena: arena
)
return RawSyntax(tokenWithDiagnostic)
}

private func visitLayoutNode(node: RawSyntax) -> RawSyntax? {
Expand Down
20 changes: 20 additions & 0 deletions Sources/SwiftParser/SwiftVersion.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2024 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
//
//===----------------------------------------------------------------------===//

extension Parser {
/// A Swift language version.
public enum SwiftVersion: Comparable {
case v4
case v5
case v6
}
}
2 changes: 2 additions & 0 deletions Sources/SwiftParser/TokenConsumer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ protocol TokenConsumer {
/// The current token syntax being examined by the consumer
var currentToken: Lexer.Lexeme { get }

var swiftVersion: Parser.SwiftVersion { get }

/// The experimental features that have been enabled.
var experimentalFeatures: Parser.ExperimentalFeatures { get }

Expand Down
Loading