Skip to content
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

fix: formatting for loop #6807

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Conversation

amiremohamadi
Copy link
Contributor

Description

Fix #6804

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@amiremohamadi amiremohamadi requested a review from a team as a code owner January 6, 2025 08:41
zees-dev
zees-dev previously approved these changes Jan 6, 2025
Copy link
Contributor

@zees-dev zees-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice; tested and seems to be working 👍

Copy link
Member

@JoshuaBatty JoshuaBatty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't solve the issue.

There is already white space added after the for keyword so this change just pads more white space.

Locking this while I look into it.

@zees-dev
Copy link
Contributor

zees-dev commented Jan 7, 2025

This doesn't solve the issue.

There is already white space added after the for keyword so this change just pads more white space.

Locking this while I look into it.

I manually tested switching to the PR branch to validate.
Weird, there maybe a deeper underlying issue then; thanks for looking deeper!

@JoshuaBatty
Copy link
Member

JoshuaBatty commented Jan 7, 2025

This looks like a parser error. If we take the example for from the linked issue and run it we get a parser error. @IGI-111 any idea why the parser would be failing here?

#[test]
fn contract_for_loop() {
    check(
        indoc! {r#"
        contract;

        abi MyContract {
            fn test_function() -> bool;
        }

        impl MyContract for Contract {
            fn test_function() -> bool {
                let mut my_vec: Vec<u64> = Vec::new();
                for iter in my_vec.iter() {

                }

                true
            }
        }
        "#},
        indoc! {r#"
        contract;

        abi MyContract {
            fn test_function() -> bool;
        }

        impl MyContract for Contract {
            fn test_function() -> bool {
                let mut my_vec: Vec<u64> = Vec::new();
                for iter in my_vec.iter() {

                }

                true
            }
        }
        "#},
    );
}
thread 'contract_for_loop' panicked at swayfmt/tests/mod.rs:9:86:
called `Result::unwrap()` on an `Err` value: ParseFileError(ParseFileError([Parse { error: ParseError { span: Span { src (ptr): 0x149a04180, source_id: None, start: 95, end: 101, as_str(): "my_vec" }, kind: ExpectedKeyword { word: "in" } } }]))

@amiremohamadi
Copy link
Contributor Author

@JoshuaBatty thanks for the review! The issue isn't with adding a space after the for keyword, but rather with AmbiguousSingleIdent. In this case, it treats

for iter in my_vec.iter() {
}

as if it was written like:

for iterin my_vec.iter() {
}

@IGI-111
Copy link
Contributor

IGI-111 commented Jan 7, 2025

Fixing the in keyword handling seems to do the trick, but @amiremohamadi can you add a test that validates the example from the issue that Josh is quoting here?

@amiremohamadi amiremohamadi requested a review from IGI-111 January 7, 2025 13:12
@kayagokalp kayagokalp added bug Something isn't working formatter compiler: parser Everything to do with the parser labels Jan 7, 2025
@kayagokalp
Copy link
Member

Seems like we have couple conflicts @amiremohamadi before we can continue running the CI

@amiremohamadi
Copy link
Contributor Author

@kayagokalp rebased my changes

@IGI-111 IGI-111 requested a review from JoshuaBatty January 8, 2025 08:19
@JoshuaBatty JoshuaBatty merged commit 96ee0b8 into FuelLabs:master Jan 8, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cla:signed compiler: parser Everything to do with the parser formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatter fails when formatting with for loop
6 participants