Add parser recovery for unfinished interface implementation #10416
Conversation
86913cd to
dd887b2
Compare
Yeah, this one is tricky. I think I would prefer the old behavior because out of the two following scenarios:
The second one feels more likely to happen. But neither is really "correct". |
|
If returning the old behaviour, subsequent members have to be I'm thinking about other approaches. |
|
OK, subsequent members are parsed properly now and belong to the type, and the indentation warning in this particular case is reported by the parser instead of
The warning range is larger with this approach, but a similar range is used in some other places in the compiler already and is probably not that bad. We don't have the first token range info at this point anymore anyway. Technically this is a breaking change, since it was possible to implement interfaces without further indenting members, but I much prefer how it's handled now with an explicit error and properly parsing all type members around. |
|
@cartermp @vzarytovskii The test failures seem unrelated, have you seen this before? |
That is a weird one, looks like some agent hiccup, I'll take a look. |
cartermp
left a comment
There was a problem hiding this comment.
Thanks, this is approved pending tests passing. I like the adjustment!
39ef8c0 to
7a5ca77
Compare
|
This is ready. |
…0416) * Add parser recovery for unfinished interface implementation * Report indentation problem via parser * Update public area
Adds recovery for unfinished
interface ... withthat contains no members but is already givenwithkeyword.Example 1: before (interface impl is not parsed at all, subsequent declarations are corrupted):
Example 1: after (everything is parsed, type check results are available for the next declaration):
The main problem with the example above is when
withis present without any members, it simply fails to parse, skips other declarations, and no error about missing members is reported at all (for a user or tooling to fix it).Example 2: before (subsequent member is considered to be interface part):
Example 2: after (subsequent member belongs to the type declaration):
The current behaviour in the second example may actually be an intended behaviour. I've initially thought that from my experience it usually did more harm than good, since subsequence members do have the correct offset for the type declaration, but now I'm thinking it shouldn't have been changed. This part can easily be reverted by using
<in theLexFilterchange instead. @dsyme @cartermp What do you think about this change?