erg-lang / erg

A statically typed language compatible with Python
http://erg-lang.org
Apache License 2.0
2.61k stars 53 forks source link

Tracking issue: implement the linter #501

Open mtshiba opened 3 months ago

mtshiba commented 3 months ago

TODOs:

verma-kartik commented 2 months ago

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 commented 2 months ago

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.

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

  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!