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

Tracking issue: implement the linter #501

Open
4 of 5 tasks
mtshiba opened this issue Mar 24, 2024 · 2 comments
Open
4 of 5 tasks

Tracking issue: implement the linter #501

mtshiba opened this issue Mar 24, 2024 · 2 comments
Labels
enhancement New feature or request lint tracking Solving this problem is a long process

Comments

@mtshiba
Copy link
Member

mtshiba commented Mar 24, 2024

TODOs:

@mtshiba mtshiba added enhancement New feature or request lint labels Mar 24, 2024
@mtshiba mtshiba changed the title Implement linter Tracking issue: implement the linter Mar 24, 2024
@mtshiba mtshiba added the tracking Solving this problem is a long process label Mar 24, 2024
@verma-kartik
Copy link

Hey @mtshiba. The child issues seems good for me to tinker around Erg code. I would like to work on this.

If you have any pointers to point me in the right direction, that would be great!

@mtshiba
Copy link
Member Author

mtshiba commented Apr 11, 2024

OK, here are the steps for adding rules to the linter.

  1. Define a function to create a warning in erg_linter/warn.rs

For example:

pub(crate) fn true_comparison(expr: &Expr, input: Input, caused_by: String, loc: Location) -> CompileWarning {
    CompileWarning::new(
        ErrorCore::new(
            vec![
                SubMessage::ambiguous_new(loc, vec![], Some(format!("just write: {}", expr.to_string_notype()))),
            ],
            "equality checks against True are redundant".to_string(),
            0,
            ErrorKind::Warning,
            loc,
        ),
        input,
        caused_by,
    )
}

The function can take any arguments, but at least the following arguments are required.

  • input: file where the warning occurred
  • caused_by: context in which the warning occurred
  • loc: location where the warning occurred

The return type is CompileWarning. This is a structure that wraps ErrorCore with additional information. The following information is given to ErrorCore.

  • sub_messages: sub messages contain hints, etc.
  • main_message: main message
  • errno: error number, 0 for now
  • kind: error type, ErrorKind::Warning
  • loc: location where the warning occurred
  1. Add lint_* methods to Linter in erg_linter/lint.rs

For example, let's add a rule to warn about redundant expressions such as x == True. This rule can be implemented with a method like this.

impl Linter {
    ...
    fn lint_bool_comparison(&mut self, expr: &Expr) {
        match expr {
            Expr::BinOp(binop) => {
                let lhs = <&Literal>::try_from(binop.lhs.as_ref()).map(|lit| &lit.value);
                let rhs = <&Literal>::try_from(binop.rhs.as_ref()).map(|lit| &lit.value);
                let lhs_or_rhs = lhs.or(rhs);
                let func = match (binop.op.kind, lhs_or_rhs) {
                    (TokenKind::DblEq, Ok(ValueObj::Bool(true))) => true_comparison,
                    (TokenKind::DblEq, Ok(ValueObj::Bool(false))) => false_comparison,
                    (TokenKind::NotEq, Ok(ValueObj::Bool(true))) => false_comparison,
                    (TokenKind::NotEq, Ok(ValueObj::Bool(false))) => true_comparison,
                    _ => {
                        self.lint_bool_comparison(&binop.lhs);
                        self.lint_bool_comparison(&binop.rhs);
                        return;
                    }
                };
                let expr = if lhs.is_ok_and(|val| val.is_bool()) {
                    binop.rhs.as_ref()
                } else if rhs.is_ok_and(|val| val.is_bool()) {
                    binop.lhs.as_ref()
                } else {
                    self.lint_bool_comparison(&binop.lhs);
                    self.lint_bool_comparison(&binop.rhs);
                    return;
                };
                let warn = func(expr, self.input(), self.caused_by(), binop.loc());
                self.warns.push(warn);
                self.lint_bool_comparison(&binop.lhs);
                self.lint_bool_comparison(&binop.rhs);
            }
            _ => self.check_recursively(&Self::lint_bool_comparison, expr),
        }
    }
    ...
}

I used some conversion functions and helper methods. Please check docs.rs to find out what APIs are available. Or you can implement methods plainly without using helper methods.

Creates a warning and adds it to Linter.warns when expr meets the condition.

You can apply this rule recursively to the expressions in expr with check_recursively.

  1. Add method checks inside Linter.lint's main loop
    pub fn lint(&mut self, src: String) -> Result<CompileWarnings, ErrorArtifact> {
        log!(info "Start linting");
        let art = self.builder.build(src, "exec")?;
        self.warns.extend(art.warns);
        for chunk in art.object.module.iter() {
            self.lint_too_many_params(chunk);
+           self.lint_bool_comparison(chunk);
        }
        log!(info "Finished linting");
        Ok(self.warns.take())
    }

That's all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lint tracking Solving this problem is a long process
Projects
None yet
Development

No branches or pull requests

2 participants