foundry-rs / foundry-rs/foundry

feat(lint/fmt): Adding NatSpec linting via lintspec

Open
#11,471 10 comments 5 reactions 0 assignees View on GitHub
Cmd-forge-doc Cmd-forge-lint T-feature
Dominant language
Rust
Stars
10.6k
Forks
2.6k
Avg merge
16h 38m
Merged PRs (30d)
511

Description

### Component

Forge

### Describe the feature you would like

I am the author of [`lintspec`](https://github.com/beeb/lintspec), a parser and linter for NatSpec comments in Solidity.

Following discussions with various users and notably [this issue](https://github.com/beeb/lintspec/issues/119), I would like to propose that its functionality is added to `forge lint` (or another subcommand if we think it's better).

The crate is both a binary and a library, which makes it suitable for integration with other tools. At the moment, the main Solidity parser is `slang_solidity`, but the alternative `solar` parser can be enabled through feature flags. ~~For now, enabling `solar` doesn't remove the dependency on `slang_solidity` but this could easily be changed if necessary.~~ The parsers are optional now.

As `lintspec` is [highly configurable](https://github.com/beeb/lintspec/blob/main/.lintspec.toml), the main question is how can we translate the various config items into a structure suitable for `forge lint` (if we think it's the best way to integrate it).

The bulk of the configuration allows users to set, for each source item type (function, external or internal variable, struct, enum, etc.) and for each NatSpec comment category (`@notice`, `@dev`, `@param` etc.), whether it's "required" or "forbidden". The third option ("ignored") would simply be to not activate any lint.

So we propose a set of lints in the form `natspec-[-]--`:
- `natspec-function-private-dev-required`
- `natspec-function-private-notice-forbidden`
- `natspec-constructor-param-required`
- `natpsec-struct-notice-required`
- etc.

An additional important configuration options -- `notice_or_dev` -- allows to accept `@dev` as a substitute for a required `@notice`, or `@notice` as a substitute for a required `@dev`.
We could image a lint that would activate this option globally (is that possible?) for consideration when running the lints above.

The last two options to consider, -- `inheritdoc` and `inheritdoc-override` -- enforce that all overridden, public and external items have `@inheritdoc`. This could be its own lint `natspec-inheritdoc`.

### Additional context

Happy to answer any questions you have!

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the forge lint entry point and lintspec's .lintspec.toml configuration described in the issue. Determine how the required, forbidden, notice_or_dev, and inheritdoc options could fit the lint system; done would mean an agreed integration design and corresponding NatSpec lint behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, solidity
Domain
cli, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.