Skip to content

Commit

Permalink
feat(linter): add bool_comparison rule
Browse files Browse the repository at this point in the history
fix #504
  • Loading branch information
mtshiba committed Apr 11, 2024
1 parent 2f0c86a commit c05432f
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 6 deletions.
9 changes: 9 additions & 0 deletions crates/erg_common/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,15 @@ macro_rules! impl_try_from_trait_for_enum {
}
}
}
impl<'x> TryFrom<&'x $Enum> for &'x $Variant {
type Error = &'x $Enum;
fn try_from(from: &'x $Enum) -> Result<Self, Self::Error> {
match from {
Expr::$Variant(to) => Ok(to),
_ => Err(from),
}
}
}
)*
}
}
Expand Down
48 changes: 43 additions & 5 deletions crates/erg_linter/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ use erg_common::traits::{BlockKind, ExitStatus, Locational, New, Runnable, Strea
use erg_compiler::artifact::{Buildable, ErrorArtifact};
use erg_compiler::build_package::PackageBuilder;
use erg_compiler::error::{CompileError, CompileErrors, CompileWarnings};
use erg_compiler::hir::{Accessor, Def, Dict, Expr, List, Set, Signature, Tuple};
use erg_compiler::hir::{Accessor, Def, Dict, Expr, List, Literal, Set, Signature, Tuple};
use erg_compiler::module::SharedCompilerResource;
use erg_compiler::ty::ValueObj;

use erg_parser::token::TokenKind;
use erg_parser::ParserRunner;

use crate::warn::too_many_params;
use crate::warn::*;

#[derive(Debug)]
pub struct Linter {
Expand Down Expand Up @@ -147,13 +149,14 @@ impl Linter {
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())
}

fn lint_too_many_params(&mut self, chunk: &Expr) {
match chunk {
fn lint_too_many_params(&mut self, expr: &Expr) {
match expr {
Expr::Def(Def {
sig: Signature::Subr(subr),
body,
Expand All @@ -169,7 +172,42 @@ impl Linter {
self.lint_too_many_params(chunk);
}
}
_ => self.check_recursively(&Self::lint_too_many_params, chunk),
_ => self.check_recursively(&Self::lint_too_many_params, expr),
}
}

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),
}
}

Expand Down
50 changes: 49 additions & 1 deletion crates/erg_linter/warn.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use erg_common::error::{ErrorCore, ErrorKind, Location};
use erg_common::error::{ErrorCore, ErrorKind, Location, SubMessage};
use erg_common::io::Input;
use erg_common::traits::NoTypeDisplay;
use erg_compiler::error::CompileWarning;
use erg_compiler::hir::Expr;

pub(crate) fn too_many_params(input: Input, caused_by: String, loc: Location) -> CompileWarning {
CompileWarning::new(
Expand All @@ -15,3 +17,49 @@ pub(crate) fn too_many_params(input: Input, caused_by: String, loc: Location) ->
caused_by,
)
}

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,
)
}

pub(crate) fn false_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: not {}", expr.to_string_notype())),
)],
"equality checks against False are redundant".to_string(),
0,
ErrorKind::Warning,
loc,
),
input,
caused_by,
)
}

0 comments on commit c05432f

Please sign in to comment.