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

LintPass trait API #32

Open
Niki4tap opened this issue Jan 6, 2023 · 0 comments
Open

LintPass trait API #32

Niki4tap opened this issue Jan 6, 2023 · 0 comments
Labels
A-stable-api Area: Stable API, How it should look and what should be included in it C-discussion

Comments

@Niki4tap
Copy link
Member

Niki4tap commented Jan 6, 2023

Current LintPass trait looks something like this:

pub trait LintPass<'ast> {
    fn check_item(
        &mut self,
        cx: &'ast AstContext<'ast>,
        item: ItemKind<'ast>);
    fn check_mod(
        &mut self,
        cx: &'ast AstContext<'ast>,
        item: &'ast ModItem<'ast>);
    fn check_extern_crate(
        &mut self,
        cx: &'ast AstContext<'ast>,
        item: &'ast ExternCrateItem<'ast>);
    fn check_use_decl(
        &mut self,
        cx: &'ast AstContext<'ast>,
        item: &'ast UseItem<'ast>);
    fn check_static_item(
        &mut self,
        cx: &'ast AstContext<'ast>,
        item: &'ast StaticItem<'ast>);
    fn check_const_item(
        &mut self,
        cx: &'ast AstContext<'ast>,
        item: &'ast ConstItem<'ast>);
}

The question is, should we extend its API with simpler functions like check_fn? If yes, for which items should we define them?

The difference in code is basically using a more generic function (check_item) and then checking if the item you are given is the one you want. With proposed helper functions:

impl<'ast> LintPass<'ast> for TestLintPass {
    fn check_fn(&mut self, cx: &'ast AstContext<'ast>, fn_item: &'ast FnItem<'ast>) {
        // do something with the function item
    }
}

Without:

impl<'ast> LintPass<'ast> for TestLintPass {
    fn check_item(&mut self, cx: &'ast AstContext<'ast>, item: &'ast ItemKind<'ast>) {
        if let ItemKind::Fn(fn_item) = item {
            // do something with the function item
        }
    }
}

As you can see, we already provide some of these functions (check_const_item, check_static_item), and rustc does too in its LateLintPass trait (check_fn, check_arm).

@xFrednet's comment from rust-marker/marker#75:

If I look at rustc's LateLintPass documentation, I find it a bit overwhelming and cluttered. Based on this, I think it could be good, to only provide very few check_* methods for ItemKind, ExprKind, StmtKind kind where the user then selects the nodes they're interested in.

@xFrednet xFrednet added A-stable-api Area: Stable API, How it should look and what should be included in it C-discussion labels Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stable-api Area: Stable API, How it should look and what should be included in it C-discussion
Projects
None yet
Development

No branches or pull requests

2 participants