"::" token provides poor error when in place of ":" token

[ludamad]

Aim

Take this example of correctly using ":" to specify an object's fields:

struct Point {
    x: Field,
    y: Field,
}

impl Point {
    fn new(x: Field, y: Field) -> Self {
        Point { x: x, y: y }
    } 
}

This works. However, imagine you fat finger (in the real case, this was deep in a nested structure):
Point { x: x, y:: y }
You get the wrong category of error:

  ┌─ /Users/adomurad/sources/aztec3-packages/yarn-project/noir-contracts/src/contracts/noir-aztec3/src/types/point.nr:8:9
  │
8 │         Point { x: x, y:: y }
  │         ----- not found in this scope

Expected behavior

Expected error is more like when the y field is omitted:

error: missing field: y
  ┌─ /Users/adomurad/sources/aztec3-packages/yarn-project/noir-contracts/src/contracts/noir-aztec3/src/types/point.nr:1:8
  │
1 │ struct Point {
  │        ----- Point defined here
  ·
8 │         Point { x: x }
  │         -----

The ideal error, but I appreciate if this is too naunced for this case, is something like Cannot bind constructor fields with '::', did you mean a single ':'"?. The biggest gain is just having the right category of error. For whatever reason, find_variable was called on Point instead of the usual error path if Point wasn't defined.

Bug

See above

To reproduce

See above

Installation method

Compiled from source

Nargo version

nargo 0.3.2 (git version hash: 7528f59, is dirty: false)

@noir-lang/noir_wasm version

N/A

@noir-lang/barretenberg version

N/A

@noir-lang/aztec_backend version

N/A

Additional context

No response

Submission Checklist

• Once I hit submit, I will assign this issue to the Project Board with the appropriate tags.

[jfecher]

The first error issued is a bit clearer, at least hinting that it is a parsing error:

error: Expected an expression but found :
   ┌─ /.../src/main.nr:22:18
   │
22 │         Point { x: x, y:: y }
   │                  -

This is actually a somewhat difficult case to issue a better error for. Since we hit an unexpected token, the error we'd normally expect to issue would be:

error: Expected a : but found ::
   ┌─ /.../src/main.nr:22:24
   │
22 │         Point { x: x, y:: y }
   │                        -

And indeed, the parser does generate this error internally before it is thrown away and we attempt to parse a block expression instead. The block expression expects nested expression separated by ;, hence the actual "Expected an expression" error. If we knew we needed to parse a constructor we'd keep the first error and wouldn't try to parse a block afterward. Unfortunately, there are cases like if a { ... } where the a { ... } has roughly the same syntax as a constructor, so we cannot always commit to only parsing a constructor when we see an identifier followed by curly braces.

I'll look at seeing if I can improve this error in any way. It may be related to parser recovery after an error as well. Since normally would need to be two separate expressions separated by a ; if they are not a constructor. We seem to be falling back to this state currently so perhaps we could change the fallback slightly.

[kevaundray]

@jfecher what is the current status of this issue?

[jfecher]

Still open with the same error message. Looks like the parser may be failing in parsing a constructor so it tries to parse an identifier followed by a block, hence the odd error message.

[michaeljklein]

Another example that appears to be related (missing ',' between baz and qux):

contract C {
    use dep::foo::{
            bar,
            baz
            qux
    };
}

Gives the error:

error: Expected a Ident but found {.
  ┌─ /Users/michaelklein/Coding/rust/noir/test_programs/compile_failure/missing_comma_error/src/main.nr:2:19
  │
2 │     use dep::foo::{
  │                   -
  │

Aborting due to 1 previous error