Skip to content

feat(forge lint): disable lints with inline comment #10776

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

Merged
merged 19 commits into from
Jun 26, 2025

Conversation

0xrusowsky
Copy link
Contributor

@0xrusowsky 0xrusowsky commented Jun 13, 2025

ref:

Motivation

easily allow disabling lints with inline-config via comments

Implementation

having standardization in mind, i decided to follow the same naming convention as forge-fmt (and borrowed some code from https://github.com/foundry-rs/foundry/blob/dani/fmt-solar/crates/fmt-2/src/inline_config.rs)

because of that, these are the available config options:

  • forge-lint: disable-next-item
  • forge-lint: disable-line
  • forge-lint: disable-next-line
  • forge-lint: disable-start
  • forge-lint: disable-end

unlike when formatting though, devs can also inform the target lints by wrapping their ids in parentheses:

  • forge-lint: disable-next-line(asm-keccak256)
  • forge-lint: disable-next-line(asm-keccak256, mixed-case-variable)

if no lint ids are provided (or "all" is used), all lints will be disabled:

  • forge-lint: disable-next-line(all)
  • forge-lint: disable-next-line

@zerosnacks
Copy link
Member

having standardization in mind, i decided to follow the same naming convention as forge-fmt

Good call 👍

@grandizzy
Copy link
Collaborator

having standardization in mind, i decided to follow the same naming convention as forge-fmt

Good call 👍

let's keep the same name style as for forge-fmt, that is forge-lint?

0xrusowsky and others added 4 commits June 16, 2025 16:39

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@0xrusowsky 0xrusowsky marked this pull request as ready for review June 16, 2025 16:38
Comment on lines 102 to 119
InlineConfigItem::DisableNextItem(lint) => {
let lints = match validate_ids(&mut invalid_ids, lints, lint, sp) {
Some(lints) => lints,
None => continue,
};

let comment_end = sp.hi().to_usize();

if let Some(next_item) = NextItemFinder::new(comment_end).find(ast) {
for lint in lints {
disabled_ranges.entry(lint).or_default().push(DisabledRange {
start: next_item.lo().to_usize(),
end: next_item.hi().to_usize(),
loose: false,
});
}
};
}
Copy link
Contributor Author

@0xrusowsky 0xrusowsky Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaniPopes this is the only custom logic regarding InlineConfigItem "identification" (the others mimic your fmt-2 code).

your fmt code seemed to be targeting the next word for the fmt, but in my case i wanted to target the next AST item (fn, contract, etc).

lmk if this makes sense or if i should follow a different approach


match item {
InlineConfigItem::DisableNextItem(lint) => {
let lints = match validate_ids(&mut invalid_ids, lints, lint, sp) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move this to InlineConfigItem::parse? This is specific to the inline config parser not the disabled ranges. And this way we don't need to have 2 separate error handling in parse_inline_config

@0xrusowsky 0xrusowsky requested a review from DaniPopes June 24, 2025 08:07
@jenpaff jenpaff moved this to Ready For Review in Foundry Jun 24, 2025
DaniPopes
DaniPopes previously approved these changes Jun 26, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@0xrusowsky 0xrusowsky merged commit 242069c into master Jun 26, 2025
42 of 44 checks passed
@0xrusowsky 0xrusowsky deleted the feat/forge-lint-inline-disable branch June 26, 2025 14:51
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in Foundry Jun 26, 2025
@grandizzy grandizzy moved this from Done to Completed in Foundry Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed
4 participants