core icon indicating copy to clipboard operation
core copied to clipboard

[Feature] Ability to add documentation inside of sol! macro to satisfy `missing_docs` lint rule

Open zerosnacks opened this issue 1 year ago • 6 comments

Component

sol! macro

Describe the feature you would like

When using the following common lint rule:

[lints]
rust.missing_docs = "warn"

It will warn users that this sol! block is missing documentation with the error: missing documentation for a struct field requested on the command line with "-W missing-docs"

sol! {
    #[sol(rpc, bytecode="6080...0033")]
    contract Counter {
        uint256 public number;

        function setNumber(uint256 newNumber) public {
            number = newNumber;
        }

        function increment() public {
            number++;
        }
    }
}

It is however currently not possible to add documentation inside of the sol! macro and therefore it requires adding a #[allow(missing_docs)] for now.

Additional context

Added a minimal repro here: https://github.com/zerosnacks/alloy-core-repro-588

Unclear if this should be marked as a feature request or a bug

zerosnacks avatar Mar 28 '24 10:03 zerosnacks

Came here to open this exact issue!

Fwiw, this invocation gives the same warning:

sol! {
    /// Emitted when `value` tokens are moved from one account (`from`) to
    /// another (`to`).
    ///
    /// Note that `value` may be zero.
    event Transfer(address indexed from, address indexed to, uint256 value);
    /// Emitted when the allowance of a `spender` for an `owner` is set by a
    /// call to `approve`. `value` is the new allowance.
    event Approval(address indexed owner, address indexed spender, uint256 value);
}

alexfertel avatar Apr 08 '24 18:04 alexfertel

Is it my understanding that you would prefer something like the following, instead of allowing the lint with #[allow(missing_docs)]?

sol! {
    /// ...
    event Approval(
        /// ...
        address indexed owner,
        /// ...
        address indexed spender,
        /// ...
        uint256 value
    );
}

DaniPopes avatar Apr 08 '24 18:04 DaniPopes

Is it my understanding that you would prefer something like the following

For me this gives a warning as well.

alexfertel avatar Apr 08 '24 19:04 alexfertel

Yes, it's not implemented, I am asking if that's what you would like

DaniPopes avatar Apr 08 '24 19:04 DaniPopes

Yeah, I think that works 👍

alexfertel avatar Apr 08 '24 19:04 alexfertel

Is it my understanding that you would prefer something like the following, instead of allowing the lint with #[allow(missing_docs)]

Ah @DaniPopes, so I was revisiting this and reading the code, and I realized that I probably misunderstood your question twice.

I think I'd prefer if we annotated the fields of the generated event struct with #[allow(missing_docs)], which is what you suggested?

I initially thought you meant:

sol! {
    /// ...
    event Approval(
        #[allow(missing_docs)]
        address indexed owner,
        #[allow(missing_docs)]
        address indexed spender,
        #[allow(missing_docs)]
        uint256 value
    );
}

It's a one-line change, so I'll just open a PR. I'm not entirely sure this addresses the original issue though, let me know if that's the case.

alexfertel avatar May 05 '24 14:05 alexfertel