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

bug: Inconsistent scarb fmt Behavior for Closures #6966

Closed
0xNeshi opened this issue Jan 3, 2025 · 2 comments · Fixed by #7065
Closed

bug: Inconsistent scarb fmt Behavior for Closures #6966

0xNeshi opened this issue Jan 3, 2025 · 2 comments · Fixed by #7065
Assignees
Labels
bug Something isn't working

Comments

@0xNeshi
Copy link

0xNeshi commented Jan 3, 2025

Bug Report

Cairo version:

scarb 2.9.2+nightly-2025-01-01 (bb807b9e4 2025-01-01)
cairo: 2.9.2 (9d2d903d3)
sierra: 1.6.0

Current behavior:

Running scarb fmt to format code that contains a closure results in inconsistent formatting.

As can be seen in the Related code section, an example with 3 different closures is given, where:

  • _closure_returns_sum_as_usize and _closure_returns_bytearray have the same initial (pre-format) line length.
  • _closure_returns_sum_as_usize and _closure_returns_sum_as_usize_but_with_a_much_longer_name have the same function signatures.

Running scarb fmt results in the following:

  • _closure_returns_sum_as_usize is left as-is.
  • _closure_returns_bytearray is wrapped after the left: usize argument.
  • _closure_returns_sum_as_usize_but_with_a_much_longer_name is wrapped after the left: usize argument.

Expected behavior:

Running scarb fmt should ideally have the same behavior as cargo fmt has (see Related code below).

Steps to reproduce:

To reproduce the issue with scarb fmt:

  1. Open terminal and run scarb new closures_example.
  2. Navigate to the project.
  3. Open src/lib.cairo.
  4. Copy paste Related code in the "Before formatting" section.
  5. Run scarb fmt.

To verify how cargo fmt behaves:

  1. Open terminal and run cargo new rust_closures_example.
  2. Navigate to the project.
  3. Open src/main.cairo.
  4. Copy paste Related code in the "Before formatting" section.
  5. Run cargo fmt.

Related code:

Before formatting:

fn main() {
    let _closure_returns_sum_as_usize = |left: usize, right: usize| -> usize {
        left + right
    };

    let _closure_return_bytearray = |left: usize, right: usize| -> ByteArray {
        format!("{left}{right}")
    };

    let _closure_returns_sum_as_usize_but_with_a_much_longer_name = |left: usize, right: usize| -> usize {
        left + right
    };
}

After formatting with scarb fmt:

fn main() {
    let _closure_returns_sum_as_usize = |left: usize, right: usize| -> usize {
        left + right
    };

    let _closure_return_bytearray = |left: usize,
    right: usize| -> ByteArray {
        format!("{left}{right}")
    };

    let _closure_returns_sum_as_usize_but_with_a_much_longer_name = |left: usize,
    right: usize| -> usize {
        left + right
    };
}

If the same code is pasted into a Rust project, and formatted with cargo fmt:

fn main() {
    let _closure_returns_sum_as_usize = |left: usize, right: usize| -> usize { left + right };

    let _closure_return_bytearray =
        |left: usize, right: usize| -> ByteArray { format!("{left}{right}") };

    let _closure_returns_sum_as_usize_but_with_a_much_longer_name =
        |left: usize, right: usize| -> usize { left + right };
}

Other information:

@0xNeshi 0xNeshi added the bug Something isn't working label Jan 3, 2025
@dean-starkware
Copy link
Collaborator

@orizi orizi linked a pull request Jan 14, 2025 that will close this issue
@orizi
Copy link
Collaborator

orizi commented Jan 14, 2025

formatting is better now on main.
not same as rust - but no reason for it to be exactly the same as rust.

@orizi orizi closed this as completed Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants