Fix `spans` for functions that return implicitly

[eureka-cpu]

After some discussion with @mohammadfawaz & others, this PR will rely on a refactor of how we handle Delimiters. One of Mohammad's comments below references how Rust handles implicit return spans, and to do this we will need the span of the curly brace. Currently we do not have spans for delimiters excluding angle brackets, which are oddly grouped with PunctKind for token creation.

TLDR: The focus is to make delimiter handling more robust, so that spans given to errors can point to, for instance, a curly brace token in the instance of mismatched return types when the return is implicit.

From should_fail/abi_method_signature_mismatch:

contract;

abi MyContract {
    /* snip... */
    fn baz() -> u64;
}

impl MyContract for Contract {
    /* snip... */
    fn baz() { // No return type here
    }
}

Using a dummy span would result in no pointer to the source code. And the current logic has it pointing to the function signature and not the return type. Refactoring the current Delimiters (Braces, Parens, SquareBrackets) to be more like it's sibling AngleBrackets will allow us to reference the span of an open or closed delimiter. In the case of ImplicitReturn this would allow us to pass on the span of that token like so

ImplicitReturn {
    // the span of a curly brace where an implicit return occurs 
    span: Span
}

resulting in consistent error handling when return types don't match, and the return type is implicit:

error
  --> /Users/eureka/Code/sway/test/src/e2e_vm_tests/test_programs/should_fail/abi_method_signature_mismatch/src/main.sw:21:5
   |
19 |
20 |
21 |     fn baz() { // No return type here
   |              ^ expected: u64
found:    ()
help:     The definition of this function must match the one in the ABI "MyContract" declaration.
22 |     }
23 | }
   |

In addition, this will also add to readability and give better/less hacky matching for LSP and the formatter or anything that uses the AST for that matter.

// What this might look like for the LSP
match item_fn.fn_signature.return_type {
    Implicit(implicit_return) => { /* yay better implicit return handling */ },
    TypedReturn((right_arrow_token, ty)) => {}
}
// and for the Formatter
// old:
import sway_ast::Delimiter;
let open_brace = Delimiter::Brace.as_open_char();
write!(buf, "{}", open_brace)?;
// new:
write!(buf, "{}", self.open_brace.as_char())?;

Ref #3634 #3734 Closes #3635 Closes #3594

[mohammadfawaz]

Looks good! But you have some E2E test failures that need to be addressed. Specifically, test.toml for:

    should_fail/trait_method_signature_mismatch ... failed
    should_fail/abi_method_signature_mismatch ... failed

should be updated.

[mohammadfawaz]

Ok so I will write my thoughts on the failing test here: for trait_method_signature_mismatch, we used to emit the following error: Now, we just emit this (not pointing to any location): We previously used to point to the function span itself because the return type is missing (i.e. () implicitly). Now that we’re using Span::dummy(), the error is different and doesn’t point to anything which is not good. I thought we didn’t need this span for anything but clearly we do. So I looked at what Rust does… it seems to point to the span of the following character as in: So I suggest we do the same instead of returning a dummy span.

[eureka-cpu]

Going to spend some time addressing this after #4097 & #4095

[eureka-cpu]

@IGI-111 @sdankel Going to spend some time getting this back into shape this coming week

[eureka-cpu]

Putting this on hold while I work on https://github.com/FuelLabs/fuelup/issues/457 for the next beta release.

[sdankel]

We should rethink this solution in light of https://github.com/FuelLabs/sway/issues/4981

[eureka-cpu]

We should rethink this solution in light of #4981

In this instance, we don't want the span to be none since it would mean that we couldn't point to where the error actually happens

Side note: this problem actually occurred due to essentially a similar implementation of #4981 where the return type could be None and we were replacing the span of what would be the return type span with the span of the entire function signature. In my main comment you can see what Rust does and how this PR aims to achieve that.

[eureka-cpu]

Closing due to how big of a PR this became, and I no longer have time to try and finish it. There is some good stuff in here, for anyone who may decide to tackle the issue of adding delimiters to the parser and how that could affect the resulting AST, error handling and so on.