Reject bad string interpolations #17074
Conversation
|
|
||
| // Test with too few interpolated values. | ||
| func mamaBear() { | ||
| _ = "\()" // expected-error{{expected expression in list of expressions}} |
There was a problem hiding this comment.
Any way that we can also make this error less cryptic as part of this change? It’s essentially nonsense right now, at least to me.
There was a problem hiding this comment.
It was surprisingly nontrivial, but I've just pushed changes to do it. The new version emits the same "interpolations should be a single expression" error message for empty interpolations, but provides a different fix-it telling the user to delete the empty interpolation.
| @@ -1927,6 +1927,37 @@ parseStringSegments(SmallVectorImpl<Lexer::StringSegment> &Segments, | |||
| Status |= E; | |||
| if (E.isNonNull()) { | |||
| Exprs.push_back(E.get()); | |||
|
|
|||
| if (auto tuple = dyn_cast<TupleExpr>(Exprs.back())) { | |||
There was a problem hiding this comment.
Is the expression ever not a TupleExpr? Maybe use cast<> here instead?
There was a problem hiding this comment.
The expression actually should be a ParenExpr. I don’t know of a reason it should be a TupleExpr without one of the nested conditions being true, but if it ever was, as far as I know it would be valid.
|
Fingers crossed @swift-ci please test source compatibility |
|
Source Compatibility Suite Passed Kill It With 🔥🔥🔥 |
|
|
||
| // Test with an argument label | ||
| func funkyBear() { | ||
| _ = "\(describing: str)" // expected-error{{interpolations cannot start with a keyword argument}} {{10-22=}} |
There was a problem hiding this comment.
A suggestion: Try another String initializer or two.
String interpolations with multiple comma-separate expressions or argument labels were being incorrectly accepted.
Fixes a test at Parse/recovery.swift:799 which the previous commit broke.
A previous version of this test used FileCheck instead of -verify, and the run line wasn’t properly corrected to use -verify.
Ensures that we don’t get different results from an initializer that doesn’t exist or doesn’t take a String.
We now diagnose the error and remove the label before it has an opportunity to crash.
|
With the crasher in and fixed, I think this one's ready to go. |
|
@swift-ci please smoke test |
|
⛵️ |
This reverts commit fc23f34.
Revert "Reject bad string interpolations (#17074)"
String interpolations with multiple comma-separated expressions or argument labels were being incorrectly accepted. This change makes both of them into errors, with appropriate fix-its.
The comma-separated-expressions error could be a warning if we wanted to preserve source compatibility, but as far as I can tell there are no examples of it in the source compatibility suite. The argument-label one can cause an error/crash in serialization if not diagnosed.
Resolves SR-7937 and SR-7958.