Enforce same line bracket opens in Solidity

[0x4007]

A custom lint-staged script would need to run to enforce this bracket style on commits. I was unable to find a tool out-of-the-box for this but it is possible that they exist.

@rndquu we are having styling issues, we should decide whether to allow

function setRndnuu() private {
    // code
}

or

function setRndnuu() private
{

}

this can be seen here as well as many other parts of the code

I guess the linter decides how to format code

I looked into this and we would have to make a custom tool to enforce this with lint-staged I believe.

Originally posted by @pavlovcik in #832 (comment)

[rndquu]

Let's start with utilizing our existing linter. Perhaps there is a rule which enforces line brackets to a particular format. If there is no such rule (or some other linter with such rule) then we could increase time estimate to < 1 Day to create our own custom solution.

[gitcoindev]

Let's start with utilizing our existing linter. Perhaps there is a rule which enforces line brackets to a particular format. If there is no such rule (or some other linter with such rule) then we could increase time estimate to < 1 Day to create our own custom solution.

Good point. I started looking into this as well, the existing linter should be able to enforce this. I will check and should have a solution today.

[gitcoindev]

I already found the root cause, we should use prettier-plugin-solidity in lint-staged settings. I will test and submit a pr soon.

[gitcoindev]

/start

[ubiquibot]

Deadline	Mon, 20 Nov 2023 13:37:26 UTC
Registered Wallet	0x7e92476D69Ff1377a8b45176b1829C4A5566653a

Tips:

• Use /wallet 0x0000...0000 if you want to update your registered payment wallet address @user.
• Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
• Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the bounty.

[gitcoindev]

I added the 'yarn format' to packages/contracts , made sure prettier-plugin-solidity is explicitly used and all in all the current formatting of development branch contracts matches Prettier way of formatting.

The full explanation is available at https://github.com/prettier-solidity/prettier-plugin-solidity/blob/main/STYLEGUIDE.md#function-declaration

I will add a mention to the style guide in contracts README.md , pasting here below the content for full clarity.

Function Declaration

• For short function declarations, it is recommended for the opening brace of the function body to be kept on the same line as the function declaration.

  The closing brace should be at the same indentation level as the function declaration.

  The opening brace should be preceded by a single space.

function increment(uint x) public pure returns (uint) {
    return x + 1;
}

function increment(uint x) public pure onlyowner returns (uint) {
    return x + 1;
}

• The visibility modifier for a function should come before any custom modifiers.

function kill() public onlyowner {
    selfdestruct(owner);
}

• For long function declarations, it is recommended to drop each argument onto it's own line at the same indentation level as the function body. The closing parenthesis and opening bracket should be placed on their own line as well at the same indentation level as the function declaration.

function thisFunctionHasLotsOfArguments(
    address a,
    address b,
    address c,
    address d,
    address e,
    address f
)
    public
{
    doSomething();
}

• If a long function declaration has modifiers, then each modifier should be dropped to its own line.

function thisFunctionNameIsReallyLong(address x, address y, address z)
    public
    onlyowner
    priced
    returns (address)
{
    doSomething();
}

function thisFunctionNameIsReallyLong(
    address x,
    address y,
    address z,
)
    public
    onlyowner
    priced
    returns (address)
{
    doSomething();
}

• Multiline output parameters and return statements should follow the same style recommended for wrapping long lines found in the Maximum Line Length section.

function thisFunctionNameIsReallyLong(
    address a,
    address b,
    address c
)
    public
    returns (
        address someAddressName,
        uint256 LongArgument,
        uint256 Argument
    )
{
    doSomething()

    return (
        veryLongReturnArg1,
        veryLongReturnArg2,
        veryLongReturnArg3
    );
}

• For constructor functions on inherited contracts whose bases require arguments, it is recommended to drop the base constructors onto new lines in the same manner as modifiers if the function declaration is long or hard to read.

pragma solidity >=0.4.0 <0.7.0;

// Base contracts just to make this compile
contract B {
    constructor(uint) public {
    }
}


contract C {
    constructor(uint, uint) public {
    }
}


contract D {
    constructor(uint) public {
    }
}


contract A is B, C, D {
    uint x;

    constructor(uint param1, uint param2, uint param3, uint param4, uint param5)
        B(param1)
        C(param2, param3)
        D(param4)
        public
    {
        // do something with param5
        x = param5;
    }
}

These guidelines for function declarations are intended to improve readability. Authors should use their best judgement as this guide does not try to cover all possible permutations for function declarations.

Therefore, there are certain cases where the opening brace of a solidity function is formatted in the same line and some on the new line, depending on the rules above.

[gitcoindev]

Pull request opened #840

[molecula451]

the idea (my idea) is to enforce the tool to have the same pattern, this is a per best solidity practices, what do you think are the nuances on this due to long range functions with params? @gitcoindev

unless rndqnuu has different in how he wants to format.

best practice for solidity is

function getPavlovcik() private {

}

struct PavlovcikInfo {
   address info;
}

struct OpenPavlovick {
   mapping(address => PavlovcikInfo) data; 
}

[rndquu]

I guess we can use any approach towards the brackets formatting unless it is consistent across the whole project

[gitcoindev]

I guess we can use any approach towards the brackets formatting unless it is consistent across the whole project

I checked a few big projects

• OpenZeppelin uses prettier
• Uniswap uses prettier

They are also fine with mixed brackets formatting, see example here: https://github.com/Uniswap/v4-core/blob/main/src/test/MockHooks.sol vs https://github.com/Uniswap/v4-core/blob/main/src/test/MockContract.sol

I would stick then to whatever prettier output formats, too.

[gitcoindev]

@molecula451 another point to use prettier output without changes is: https://prettier.io/docs/en/why-prettier

And from the main website https://prettier.io/

Why?

• Your code is formatted on save
• No need to discuss style in code review
• Saves you time and energy

And Ubiquity would use the same formatting tool as big code-bases, which makes it easier to onboard new partners / projects.

[0x4007]

getPavlovcik()

"Hello, World!"

[ubiquibot]

Permit generation skipped because this issue has been closed by an external contributor.

If you've enjoyed your experience in the DevPool, we'd appreciate your support. Follow Ubiquity on GitHub and star this repo. Your endorsement means the world to us and helps us grow!

We are excited to announce that the DevPool and UbiquiBot are now available to partners! Our ideal collaborators are globally distributed crypto-native organizations, who actively work on open source on GitHub, and excel in research & development. If you can introduce us to the repository maintainers in these types of companies, we have a special bonus in store for you!

[ubiquibot]

Task Creator Reward

pavlovcik: [ CLAIM 26.7 WXDAI ]

[ubiquibot]

Conversation Rewards

rndquu: [ CLAIM 23.4 WXDAI ]

gitcoindev: [ CLAIM 272.1 WXDAI ]

molecula451: [ CLAIM 24.9 WXDAI ]

pavlovcik: [ CLAIM 0.6 WXDAI ]