-
Notifications
You must be signed in to change notification settings - Fork 440
Fix newline parsing at trailing trivia #1661
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
Changes from 2 commits
9b2ea0d
2ab4021
79c1b1a
d852fb9
1b18fdb
8e6ce86
fb134e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,6 +150,20 @@ extension Lexer.Cursor { | |
case .inRegexLiteral: return false | ||
} | ||
} | ||
|
||
/// Returns whether the lexer is currently parsing the multiline string. | ||
var isParsingMultilineString: Bool { | ||
switch self { | ||
case .normal, .preferRegexOverBinaryOperator: return false | ||
case .afterRawStringDelimiter: return false | ||
case .inStringLiteral(kind: let stringLiteralKind, delimiterLength: _): return stringLiteralKind == .multiLine | ||
case .afterStringLiteral: return false | ||
case .afterClosingStringQuote: return false | ||
case .inStringInterpolationStart: return false | ||
case .inStringInterpolation: return false | ||
case .inRegexLiteral: return false | ||
} | ||
} | ||
} | ||
|
||
/// A data structure that holds the state stack entries in the lexer. It is | ||
|
@@ -242,6 +256,9 @@ extension Lexer { | |
|
||
/// If we have already lexed a token, the kind of the previously lexed token | ||
var previousTokenKind: RawTokenKind? | ||
|
||
/// If we have already lexed a token, the `NewlinePresence` of the previously lexed token | ||
var previousTokenNewlinePresence: NewlinePresence? | ||
|
||
/// If the `previousTokenKind` is `.keyword`, the keyword kind. Otherwise | ||
/// `nil`. | ||
|
@@ -433,23 +450,30 @@ extension Lexer.Cursor { | |
if let stateTransition = result.stateTransition { | ||
self.stateStack.perform(stateTransition: stateTransition, stateAllocator: stateAllocator) | ||
} | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, I missed that! |
||
var flags = result.flags | ||
if newlineInLeadingTrivia == .present { | ||
flags.insert(.isAtStartOfLine) | ||
} | ||
if let previousTokenNewlinePresence, previousTokenNewlinePresence == .present, | ||
!currentState.isParsingMultilineString { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In an effort to minimize side effects, I focused on maintaining the existing test cases, which led me to misunderstand the behavior of the lexer. As you mentioned, it is correct that the line1, line2, and whitespace tokens, excluding the |
||
flags.insert(.isAtStartOfLine) | ||
} | ||
|
||
// Trailing trivia. | ||
let trailingTriviaStart = self | ||
if let trailingTriviaMode = result.trailingTriviaLexingMode ?? currentState.trailingTriviaLexingMode(cursor: self) { | ||
let triviaResult = self.lexTrivia(mode: trailingTriviaMode) | ||
self.previousTokenNewlinePresence = triviaResult.newlinePresence | ||
diagnostic = TokenDiagnostic(combining: diagnostic, triviaResult.error?.tokenDiagnostic(tokenStart: cursor)) | ||
} else { | ||
self.previousTokenNewlinePresence = nil | ||
} | ||
|
||
if self.currentState.shouldPopStateWhenReachingNewlineInTrailingTrivia && self.is(at: "\r", "\n") { | ||
self.stateStack.perform(stateTransition: .pop, stateAllocator: stateAllocator) | ||
} | ||
|
||
var flags = result.flags | ||
if newlineInLeadingTrivia == .present { | ||
flags.insert(.isAtStartOfLine) | ||
} | ||
|
||
diagnostic = TokenDiagnostic(combining: diagnostic, result.error?.tokenDiagnostic(tokenStart: cursor)) | ||
|
||
let lexeme = Lexer.Lexeme( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a naming suggestion: I’d like to make it absolutely clear that this is about whether the previous token’s trailing trivia has a newline and avoid any confusion of whether the previous token had a newline inside its leading trivia. So I’d go with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion! I will try to make the modifications along with the changes mentioned above. Thank you.