Comment by Barry Tolnas (JIRA)

bar() -\> Int

^

Good call. @rintaro, do you think this would make a good Starter Bug?

I agree that this is a common mistake.

class C: Base {
  bar() {
    self.something()
  }
}

Although this is ambiguous between a misplaced function call expression with trailing closure and a method declaration with missing keyword, I think parsing and recovering as the latter makes more sense.

We do know when we're inside a type declaration, extension, or non-script file, which would help make that call.

Comment by Sergej Jaskiewicz (JIRA)

I'm trying to implement this diagnostic, and, since this is my first contribution to the Swift compiler, I could use some advice.

I can see that I need to add some logic to the `Parser::parseDecl` function. I've already learned how to distinguish member declarations and top-level declarations in non-script files from all the other declarations, and now what I'm struggling with is the following.

What I want to do is to parse the declaration as a function (via `Parser::parseDeclFunc`), if it fails, parse it as a property declaration (via `Parser::parseDeclVar`), and if that fails, then fallback to the current "expected declaration" error.

It's not clear how I can do that, since both `parseDeclFunc` and `parseDeclVar` functions can consume tokens, so I would have to restore the parser state if I wanted to go that way. I assume that's not how the parser was designed, so I need to do something different.

Or am I overcomplicating things and as soon as I meet an identifier or operator token I only need to try to parse a function declaration?

Thanks in advance.

Yeah, I think it's okay if this logic is not very smart. If you see an identifier in parseDecl, you could maybe check if the immediately following token is : or =, and otherwise assume it's a function.

Comment by Sergej Jaskiewicz (JIRA)

Totally makes sense, thank you!

Comment by Sergej Jaskiewicz (JIRA)

So I've implemented the diagnostic and ran some tests, and it turns out that this diagnostic now shadows more helpful ones in some cases.

For example, consider this non-script file:

let x = 42
x + x

Before this diagnostic was implemented, the error was "expressions are not allowed at the top level", and now there's a bunch of useless messages since the parser first assumes that x is a top-level function declaration with a missing keyword, next it sees the operator and tries to parse it as a function declaration, and finally it meets x again, parses it as a function declaration again, recalls that there already was a variable with that name, so it produces "invalid redeclaration of 'x()'".

It seems more reasonable to me to keep this diagnostic in type/extension scope, but not produce it for the top-level code. What do you think?

P. S. OTOH, one can always come up with a messed up code example for which errors and warnings are nothing but useless nonsense

Comment by Sergej Jaskiewicz (JIRA)

In types it sometimes behaves weird as well.

Before:

struct InitializerWithLabels {
  init c d: Int {}
  // expected-error @-1 {{expected '(' for initializer parameters}}
  // expected-error @-2 {{expected declaration}}
  // expected-error @-3 {{consecutive declarations on a line must be separated by ';'}}
  // expected-note @-5 {{in declaration of 'InitializerWithLabels'}}
}

After:

struct InitializerWithLabels {
  init c d: Int {}
  // expected-error @-1 {{expected '(' for initializer parameters}}
  // expected-error @-2 {{expected 'func' keyword in function declaration}}
  // expected-error @-3 {{consecutive declarations on a line must be separated by ';'}}
  // expected-error @-4 {{found an unexpected second identifier in function declaration; is there an accidental break?}}
  // expected-note  @-5 {{join the identifiers together}} {{8-11=cd}}
  // expected-note  @-6 {{join the identifiers together with camel-case}} {{8-11=cD}}
  // expected-error @-7 {{expected '(' in argument list of function declaration}}
  // expected-error @-8 {{expected '->' after function parameter tuple}}
}

Some quick-and-dirty things we could do is limit the recovery to when you're already inside a type or extension, and only do it at the start of a line.

Comment by Sergej Jaskiewicz (JIRA)

#24613

Comment by Sergej Jaskiewicz (JIRA)

Merged in master