noir-lang / noir

Noir is a domain specific language for zero knowledge proofs
https://noir-lang.org
Apache License 2.0
823 stars 178 forks source link

nargo fmt: inconsistent formatting with arrays #3945

Open LHerskind opened 6 months ago

LHerskind commented 6 months ago

Aim

Run fmt on

dep::std::hash::pedersen_hash_with_separator([
    contract_address.to_field(),
    storage_slot
], GENERATOR_INDEX__PUBLIC_LEAF_INDEX)

Expected Behavior

dep::std::hash::pedersen_hash_with_separator([
    contract_address.to_field(),
    storage_slot
], GENERATOR_INDEX__PUBLIC_LEAF_INDEX)

Or

dep::std::hash::pedersen_hash_with_separator(
    [
        contract_address.to_field(),
        storage_slot
    ], 
    GENERATOR_INDEX__PUBLIC_LEAF_INDEX
)

Bug

The formatted code looks like

dep::std::hash::pedersen_hash_with_separator(
    [
    contract_address.to_field(),
    storage_slot
],
    GENERATOR_INDEX__PUBLIC_LEAF_INDEX
)

To Reproduce

  1. Run nargo fmt

Installation Method

None

Nargo Version

No response

Additional Context

Saw the issue as part of https://github.com/AztecProtocol/aztec-packages/pull/3803/files/c6f4b7b9b9efe9a118e57aec0f28219523812563

Would you like to submit a PR for this Issue?

No

Support Needs

No response

benesjan commented 6 months ago

I tried to use the master branch of noir (version 219423eb87fa12bd8cca2a6fd2ce4c06e308783c) instead of the aztec one to see if the issues pertain and unfortunately they do.

benesjan commented 6 months ago

2 more issue I've stumbled upon.

1) When I put 2 spaces after "fn" the formatter leaves it as it is:

fn main(input: PrivateKernelInputsInner) -> distinct pub KernelCircuitPublicInputs { input.native_private_kernel_circuit_inner() }

2) It doesn't format spaces in dependencies:

use dep:: protocol_types::{ address::AztecAddress, abis::function_selector::FunctionSelector, };

benesjan commented 6 months ago

The formatter ignores spaces in struct functions:

image
kevaundray commented 6 months ago

@f01dab1e can you take this on?

ghost commented 6 months ago

I'll have a look on Monday.