Skip to content

Conversation

tiendn
Copy link

@tiendn tiendn commented Aug 16, 2025

Motivation

More unittest

Solution

  • Add unit tests for InlineConfigItem::parse:
    • default "all" behavior (with and without parens)
    • specific lint IDs parsing and trimming
    • syntax errors (unknown directive, missing closing paren)
    • invalid IDs reporting
  • Add InvalidInlineConfigItem Display tests
  • Add DisabledRange::includes strict vs loose boundary tests

No functional changes; test-only.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

- Add unit tests for InlineConfigItem::parse:
  - default "all" behavior (with and without parens)
  - specific lint IDs parsing and trimming
  - syntax errors (unknown directive, missing closing paren)
  - invalid IDs reporting
- Add InvalidInlineConfigItem Display tests
- Add DisabledRange::includes strict vs loose boundary tests

No functional changes; test-only.
@0xrusowsky
Copy link
Contributor

0xrusowsky commented Aug 17, 2025

in this wip PR (soon to be finished) https://github.com/foundry-rs/foundry/pull/10907/files#diff-e43febe7f0bd2bdfe8724234b3559802bc35ebe0ac9cb9bab221c75e5b603e15 we abstract away everything related to inline-config, and upstream it to foundry-common.

because of that + the fact that we already test this feature in several lint testdata files such as https://github.com/foundry-rs/foundry/blob/fccc80782bf3da9b770c4e6cf83aaa6d0f6faa7d/crates/lint/testdata/Keccak256.sol i think that we can close this PR... with that being said, i'll defer the decision to others.

@0xrusowsky
Copy link
Contributor

@zerosnacks @grandizzy @DaniPopes to decide if we close or not

@DaniPopes
Copy link
Member

Closing as above

@DaniPopes DaniPopes closed this Aug 21, 2025
@github-project-automation github-project-automation bot moved this to Done in Foundry Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants