Generate missing string closer quote and delimiter #690
Conversation
| @@ -907,7 +907,9 @@ extension Parser { | |||
| at: openDelimiter != nil ? .leadingRaw : .leading, | |||
| text: text | |||
| ) ?? RawTokenSyntax(missing: .stringQuote, arena: arena) | |||
| text = text.dropFirst(openQuote.tokenText.count) | |||
| if !openQuote.isMissing { | |||
There was a problem hiding this comment.
I fixed logic here.
If opener quote missing token is generated, don't drop text.
| @@ -917,16 +919,37 @@ extension Parser { | |||
| /// Parse close quote. | |||
| let closeQuote = self.parseStringLiteralQuote( | |||
| at: openDelimiter != nil ? .trailingRaw : .trailing, | |||
| text: text[closeStart...] | |||
| text: text | |||
There was a problem hiding this comment.
subscript is unnecessary here
| if position == .leadingRaw && text[index] == UInt8(ascii: "#") { | ||
| if position == .leadingRaw, | ||
| index < text.endIndex, | ||
| text[index] == UInt8(ascii: "#") |
There was a problem hiding this comment.
This subscript may access out of bounds.
I added checking of range.
|
|
||
| AssertParse( | ||
| ##""" | ||
| #"#^DQ^##^DD^# |
There was a problem hiding this comment.
Though I didn't report this input, it was also failed before.
There was a problem hiding this comment.
I think you can just use a single marker in these tests because the two you have resolve to the same location currently.
| #"#^DQ^##^DD^# | |
| #"#^DIAG^# |
If you name it DIAG, you also don’t need to specify the maker name in DiagnosticSpec.
There was a problem hiding this comment.
I changed to share single #^DIAG^#.
| return RawTokenSyntax( | ||
| kind: .rawStringDelimiter, | ||
| text: openDelimiter.tokenText, | ||
| leadingTriviaPieces: [], | ||
| trailingTriviaPieces: [], | ||
| presence: .missing, | ||
| arena: arena | ||
| ) |
There was a problem hiding this comment.
You could use
| return RawTokenSyntax( | |
| kind: .rawStringDelimiter, | |
| text: openDelimiter.tokenText, | |
| leadingTriviaPieces: [], | |
| trailingTriviaPieces: [], | |
| presence: .missing, | |
| arena: arena | |
| ) | |
| return RawTokenSyntax( | |
| missing: .rawStringDelimiter, | |
| text: openDelimiter.tokenText, | |
| arena: arena | |
| ) |
If you add text: SyntaxText? = nil, to RawTokenText.init(missing:arena:) and apply the following change in the initializer.
- text: kind.defaultText ?? "",
+ text: text ?? kind.defaultText ?? "",I think I’ve got at least two branches locally that do this as well…
There was a problem hiding this comment.
Actually I looked for RawTokenText(missing:text:arena:) first time 😄.
I added text: and changed to use it.
| } | ||
|
|
||
| return nil | ||
| }() |
There was a problem hiding this comment.
I’m personally not a huge fan of declaring a closure that you immediately call. I would prefer a paradigm like
let closeDelimiter: RawTokenSyntax?
if let parsedDelimiter = self.parseStringLiteralDelimiter(…) {
closeDelimiter = parsedDelimiter
} else if let openDelimiter = openDelimiter {
closeDelimiter = RawTokenSyntax(… missing ...)
} else {
closeDelimiter = nil
}
That matches the style we use in the rest of the parser.
There was a problem hiding this comment.
Sorry I didn't check style well.
I changed it not to use closure.
|
|
||
| AssertParse( | ||
| ##""" | ||
| #"#^DQ^##^DD^# |
There was a problem hiding this comment.
I think you can just use a single marker in these tests because the two you have resolve to the same location currently.
| #"#^DQ^##^DD^# | |
| #"#^DIAG^# |
If you name it DIAG, you also don’t need to specify the maker name in DiagnosticSpec.
|
@ahoppen Thanks for review. I changed styles to follow comments. |
|
@swift-ci Please test |
This PR fix #668 #669.
If closer quote and delimiter is missing but opener ones are present,
generate missing token to fix round trip bug and emit missing diagnostics.