-
-
Notifications
You must be signed in to change notification settings - Fork 647
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
Support static
for locals
#10555
Support static
for locals
#10555
Conversation
The display issue is this: When parsing blocks in the display file, the parser goes into Hail Mary mode and ignores any sort of "deep" error, trying to parse the block no matter what. It will only stop if its "next element" function comes across something that is not the start of a valid block element, at which point it closes the block. What changed here is that now class Main {
@:debug.display
static function main() {
test({
}
static function test(o:{a:Float, b:String}) {}
} Requesting completion at
That This shouldn't be a new issue, at the very least it should be reproducible with |
For the time being I will just change the tests. This is a very specific problem that requires a certain constellation to reproduce, which is going to be a rare occurrence in practice. We should still look into how to handle these kinds of parser resuming problems, but that's a separate topic altogether. |
If it's any consolation, I would argue this is not really a regression, because it's no different from this: class Main {
static function main() {}
@:debug.display
function wip() {
test({
}
function test(o:{a:Float, b:String}) {}
} This makes All that being said, I wonder whether it wouldn't be better for "Hail Mary mode" to assume that any keyword that can begin a field is the start of a field. Worst case you get completion for things that are intended to be not-yet-declared local functions / vars. Seems a better tradeoff then loosing all the following fields that have no access modifier. |
Good point about plain old I'm not sure about the tradeoff you mention though. I would prefer no completion over any sort of completion that causes broken code if "accepted", because that is way more irritating. |
For now we'll just error in case of a naming conflict, and go for a simple |
There's still some work to be done here.
There's some particular problem with completion due to the parser now picking up
static
at expression-level. In the failing tests this causes something that's supposed to be a class field to be parsed as an expression, which then leads to wrong completion. I already tried looking into it but got a headache.