Skip to content
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

Rule: impossible-not #603

Closed
anderseknert opened this issue Mar 15, 2024 · 1 comment · Fixed by #698
Closed

Rule: impossible-not #603

anderseknert opened this issue Mar 15, 2024 · 1 comment · Fixed by #698

Comments

@anderseknert
Copy link
Member

This is a mistake I see devs do every now and then — it just never struck me as something we could try and prevent.

deny contains something if {
    # ...
}

test_deny if {
    not deny with # ...
}

We should flag when not is used on anything but boolean rules / vars. This might not be possible to determine in all cases based on the AST alone, but for the cases where we can, we should.

@anderseknert
Copy link
Member Author

Thinking more about this, and it's certainly an aggregate rule as we'll need to collect info from all packages to try and determine the type of refs. It's totally fine to just include simple rules here as that'll catch the most common mistake which is not being used with multi-value rules — trivial to determine even from the AST.

However, I'm also starting to realize that we'll want to consider an architectural improvement with regards to aggregate rules. The data collected for this rule would pretty much be identical to the data we need to determine unresolved-imports or unused-rules. We should look into an option where several rules are allowed to share data from a single aggregation rather than each having to do their own. Now, separation should still be the default, but not the only option.

@anderseknert anderseknert changed the title Rule: non-boolean-not Rule: impossible-not Apr 20, 2024
@anderseknert anderseknert self-assigned this May 6, 2024
anderseknert added a commit that referenced this issue May 7, 2024
See docs in PR (and issue) for further details.

Fixes #603

Signed-off-by: Anders Eknert <[email protected]>
anderseknert added a commit that referenced this issue May 8, 2024
See docs in PR (and issue) for further details.

Fixes #603

Signed-off-by: Anders Eknert <[email protected]>
anderseknert added a commit that referenced this issue May 8, 2024
See docs in PR (and issue) for further details.

Fixes #603

Signed-off-by: Anders Eknert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant