bugfix: Don't throw when broken string interpolation #3936
Conversation
46c0038 to
a941cbe
Compare
|
|
||
| // the rest of tokens shouldn't be obtained via this method | ||
| case _ => unreachable(debug(curr)) | ||
| case _ => getInvalid(curr, curr.strVal) |
There was a problem hiding this comment.
so what type of token was attempted here? just want to be sure we're not masking a problem elsewhere.so what type of token was attempted here? just want to be sure we're not masking a problem elsewhere.
There was a problem hiding this comment.
STRINGPART since we have invalid token on single $ we then have a normal string contents
There was a problem hiding this comment.
keeping getInvalid is a good idea, just in case. do you think we should adjust getInvalid to include the LegacyToken that was parsed? for debugging purposes?
There was a problem hiding this comment.
I split the cases again and added a proper message which includes that info whenever unexpected token was found
tests/shared/src/test/scala/scala/meta/tests/tokenizers/TokenizerSuite.scala
Outdated
Show resolved
Hide resolved
| |Interpolation.Start(''') [2..5) | ||
| |Interpolation.Part(|Available languages (default = ) [5..37) | ||
| |Interpolation.SpliceStart [37..38) | ||
| |Invalid(Not one of: `$'_, `$$', `$'ident, `$'this, `$'BlockExpr) [38..38) |
There was a problem hiding this comment.
I changed it a bit and I think it's currently the best option. Splice starts, but is broken, so we continue with normal string part and an invalid token.
f75e64d to
eae341d
Compare
...eta/tokenizers/shared/src/main/scala/scala/meta/internal/tokenizers/ScalametaTokenizer.scala
Outdated
Show resolved
Hide resolved
|
|
||
| // the rest of tokens shouldn't be obtained via this method | ||
| case _ => unreachable(debug(curr)) | ||
| case _ => getInvalid(curr, curr.strVal) |
There was a problem hiding this comment.
keeping getInvalid is a good idea, just in case. do you think we should adjust getInvalid to include the LegacyToken that was parsed? for debugging purposes?
|
@tgodzik i merged but perhaps you'd like to remove your branch if it's not automatically removed. |
Related to scalameta/metals#6699
It seems the code is perfectly reachable after all, so maybe it's better to default to invalid.
The other option I was thinking about (or an additional) is to finish interpolation splice right away with and End, but that might be too complex for a case that might not pop up that often.