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

Feat: add comparison_to_empty #130

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 94 additions & 111 deletions Cargo.lock

Large diffs are not rendered by default.

Empty file added FETCH_HEAD
Empty file.
33 changes: 33 additions & 0 deletions crates/cairo-lint-core/src/lints/comparison_to_empty.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use cairo_lang_defs::plugin::PluginDiagnostic;
use cairo_lang_diagnostics::Severity;
use cairo_lang_syntax::node::ast::{BinaryOperator, Expr};
use cairo_lang_syntax::node::db::SyntaxGroup;
use cairo_lang_syntax::node::{TypedStablePtr, TypedSyntaxNode};

pub const COMPARISON_TO_EMPTY: &str = "Consider using `.is_empty()` instead of comparing to an empty array.";

pub fn check_comparison_to_empty(db: &dyn SyntaxGroup, expr: &Expr, diagnostics: &mut Vec<PluginDiagnostic>) {
if let Expr::Binary(binary_expr) = expr {
if let BinaryOperator::Eq(_) = binary_expr.op(db) {
let lhs = binary_expr.lhs(db);
let rhs = binary_expr.rhs(db);

if is_empty_array(db, &lhs) || is_empty_array(db, &rhs) {
diagnostics.push(PluginDiagnostic {
stable_ptr: expr.stable_ptr().untyped(),
message: COMPARISON_TO_EMPTY.to_string(),
severity: Severity::Warning,
});
}
}
}
}

fn is_empty_array(db: &dyn SyntaxGroup, expr: &Expr) -> bool {
if let Expr::FunctionCall(func_call) = expr {
let func_path = func_call.path(db);
func_path.as_syntax_node().get_text_without_trivia(db) == "ArrayTrait::new"
} else {
false
}
}
1 change: 1 addition & 0 deletions crates/cairo-lint-core/src/lints/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub mod bitwise_for_parity_check;
pub mod bool_comparison;
pub mod breaks;
pub mod comparison_to_empty;
pub mod double_comparison;
pub mod double_parens;
pub mod duplicate_underscore_args;
Expand Down
6 changes: 4 additions & 2 deletions crates/cairo-lint-core/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use cairo_lang_syntax::node::{TypedStablePtr, TypedSyntaxNode};
use crate::lints::ifs::*;
use crate::lints::manual::*;
use crate::lints::{
bitwise_for_parity_check, bool_comparison, breaks, double_comparison, double_parens, duplicate_underscore_args,
eq_op, erasing_op, loop_for_while, loops, panic, single_match,
bitwise_for_parity_check, bool_comparison, breaks, comparison_to_empty, double_comparison, double_parens,
duplicate_underscore_args, eq_op, erasing_op, loop_for_while, loops, panic, single_match,
};

pub fn cairo_lint_plugin_suite() -> PluginSuite {
Expand Down Expand Up @@ -43,6 +43,7 @@ pub enum CairoLintKind {
ManualIsSome,
ManualIsNone,
ManualExpect,
ComparisonToEmpty,
}

pub fn diagnostic_kind_from_message(message: &str) -> CairoLintKind {
Expand All @@ -67,6 +68,7 @@ pub fn diagnostic_kind_from_message(message: &str) -> CairoLintKind {
manual_is_some::MANUAL_IS_SOME => CairoLintKind::ManualIsSome,
manual_is_none::MANUAL_IS_NONE => CairoLintKind::ManualIsNone,
manual_expect::MANUAL_EXPECT => CairoLintKind::ManualExpect,
comparison_to_empty::COMPARISON_TO_EMPTY => CairoLintKind::ComparisonToEmpty,
_ => CairoLintKind::Unknown,
}
}
Expand Down
216 changes: 216 additions & 0 deletions crates/cairo-lint-core/src/plugin.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
use cairo_lang_defs::ids::{FunctionWithBodyId, ModuleId, ModuleItemId};
use cairo_lang_defs::plugin::PluginDiagnostic;
use cairo_lang_semantic::db::SemanticGroup;
use cairo_lang_semantic::plugin::{AnalyzerPlugin, PluginSuite};
use cairo_lang_semantic::Expr;
use cairo_lang_syntax::node::ast::{ElseClause, Expr as AstExpr, ExprBinary, ExprIf, ExprLoop, ExprMatch};
use cairo_lang_syntax::node::kind::SyntaxKind;
use cairo_lang_syntax::node::{TypedStablePtr, TypedSyntaxNode};

use crate::lints::ifs::*;
use crate::lints::manual::*;
use crate::lints::{
bitwise_for_parity_check, bool_comparison, bool_comparison, breaks, breaks, comparison_to_empty,
comparison_to_empty, double_comparison, double_comparison, double_parens, double_parens, duplicate_underscore_args,
duplicate_underscore_args, eq_op, eq_op, erasing_op, erasing_op, loop_for_while, loop_for_while, loops, loops,
panic, panic, single_match, single_match,
};

pub fn cairo_lint_plugin_suite() -> PluginSuite {
let mut suite = PluginSuite::default();
suite.add_analyzer_plugin::<CairoLint>();
suite
}
#[derive(Debug, Default)]
pub struct CairoLint;

#[derive(Debug, PartialEq)]
pub enum CairoLintKind {
DestructMatch,
MatchForEquality,
DoubleComparison,
DoubleParens,
EquatableIfLet,
BreakUnit,
BoolComparison,
CollapsibleIfElse,
DuplicateUnderscoreArgs,
LoopMatchPopFront,
BitwiseForParityCheck,
LoopForWhile,
Unknown,
Panic,
ErasingOperation,
ManualOkOr,
ManualIsSome,
ManualIsNone,
ManualExpect,
ComparisonToEmpty,
}

pub fn diagnostic_kind_from_message(message: &str) -> CairoLintKind {
match message {
single_match::DESTRUCT_MATCH => CairoLintKind::DestructMatch,
single_match::MATCH_FOR_EQUALITY => CairoLintKind::MatchForEquality,
double_parens::DOUBLE_PARENS => CairoLintKind::DoubleParens,
double_comparison::SIMPLIFIABLE_COMPARISON => CairoLintKind::DoubleComparison,
double_comparison::REDUNDANT_COMPARISON => CairoLintKind::DoubleComparison,
double_comparison::CONTRADICTORY_COMPARISON => CairoLintKind::DoubleComparison,
breaks::BREAK_UNIT => CairoLintKind::BreakUnit,
equatable_if_let::EQUATABLE_IF_LET => CairoLintKind::EquatableIfLet,
bool_comparison::BOOL_COMPARISON => CairoLintKind::BoolComparison,
collapsible_if_else::COLLAPSIBLE_IF_ELSE => CairoLintKind::CollapsibleIfElse,
duplicate_underscore_args::DUPLICATE_UNDERSCORE_ARGS => CairoLintKind::DuplicateUnderscoreArgs,
loops::LOOP_MATCH_POP_FRONT => CairoLintKind::LoopMatchPopFront,
panic::PANIC_IN_CODE => CairoLintKind::Panic,
loop_for_while::LOOP_FOR_WHILE => CairoLintKind::LoopForWhile,
erasing_op::ERASING_OPERATION => CairoLintKind::ErasingOperation,
manual_ok_or::MANUAL_OK_OR => CairoLintKind::ManualOkOr,
bitwise_for_parity_check::BITWISE_FOR_PARITY => CairoLintKind::BitwiseForParityCheck,
manual_is_some::MANUAL_IS_SOME => CairoLintKind::ManualIsSome,
manual_is_none::MANUAL_IS_NONE => CairoLintKind::ManualIsNone,
manual_expect::MANUAL_EXPECT => CairoLintKind::ManualExpect,
comparison_to_empty::COMPARISON_TO_EMPTY => CairoLintKind::ComparisonToEmpty,
_ => CairoLintKind::Unknown,
}
}

impl AnalyzerPlugin for CairoLint {
fn diagnostics(&self, db: &dyn SemanticGroup, module_id: ModuleId) -> Vec<PluginDiagnostic> {
let mut diags = Vec::new();
let syntax_db = db.upcast();
let Ok(items) = db.module_items(module_id) else {
return diags;
};
for item in &*items {
let function_nodes = match item {
ModuleItemId::Constant(constant_id) => {
constant_id.stable_ptr(db.upcast()).lookup(syntax_db).as_syntax_node()
}
ModuleItemId::FreeFunction(free_function_id) => {
let func_id = FunctionWithBodyId::Free(*free_function_id);
check_function(db, func_id, &mut diags);
free_function_id.stable_ptr(db.upcast()).lookup(syntax_db).as_syntax_node()
}
ModuleItemId::Impl(impl_id) => {
let impl_functions = db.impl_functions(*impl_id);
let Ok(functions) = impl_functions else {
continue;
};
for (_fn_name, fn_id) in functions.iter() {
let func_id = FunctionWithBodyId::Impl(*fn_id);
check_function(db, func_id, &mut diags);
}
impl_id.stable_ptr(db.upcast()).lookup(syntax_db).as_syntax_node()
}
_ => continue,
}
.descendants(syntax_db);

for node in function_nodes {
match node.kind(syntax_db) {
SyntaxKind::ExprParenthesized => double_parens::check_double_parens(
db.upcast(),
&AstExpr::from_syntax_node(db.upcast(), node),
&mut diags,
),
SyntaxKind::StatementBreak => breaks::check_break(db.upcast(), node, &mut diags),
SyntaxKind::ExprIf => {
equatable_if_let::check_equatable_if_let(
db.upcast(),
&ExprIf::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
manual_is_some::check_manual_if_is_some(
db.upcast(),
&ExprIf::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
manual_is_none::check_manual_if_is_none(
db.upcast(),
&ExprIf::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
manual_expect::check_manual_if_expect(
db.upcast(),
&ExprIf::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
manual_ok_or::check_manual_if_ok_or(
db.upcast(),
&ExprIf::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
}
SyntaxKind::ExprBinary => {
let expr_binary = ExprBinary::from_syntax_node(db.upcast(), node);
bool_comparison::check_bool_comparison(db.upcast(), &expr_binary, &mut diags);
double_comparison::check_double_comparison(db.upcast(), &expr_binary, &mut diags);
eq_op::check_eq_op(db.upcast(), &expr_binary, &mut diags);
bitwise_for_parity_check::check_bitwise_for_parity(db.upcast(), &expr_binary, &mut diags);
erasing_op::check_erasing_operation(db.upcast(), expr_binary, &mut diags);
}
SyntaxKind::ElseClause => {
collapsible_if_else::check_collapsible_if_else(
db.upcast(),
&ElseClause::from_syntax_node(db.upcast(), node),
&mut diags,
);
}
SyntaxKind::ExprLoop => {
loop_for_while::check_loop_for_while(
db.upcast(),
&ExprLoop::from_syntax_node(db.upcast(), node),
&mut diags,
);
}
SyntaxKind::ExprMatch => {
manual_ok_or::check_manual_ok_or(
db.upcast(),
&ExprMatch::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
manual_is_some::check_manual_is_some(
db.upcast(),
&ExprMatch::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
manual_is_none::check_manual_is_none(
db.upcast(),
&ExprMatch::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
manual_expect::check_manual_expect(
db.upcast(),
&ExprMatch::from_syntax_node(db.upcast(), node.clone()),
&mut diags,
);
}
_ => continue,
}
}
}
diags
}
}
fn check_function(db: &dyn SemanticGroup, func_id: FunctionWithBodyId, diagnostics: &mut Vec<PluginDiagnostic>) {
duplicate_underscore_args::check_duplicate_underscore_args(
db.function_with_body_signature(func_id).unwrap().params,
diagnostics,
);
let Ok(function_body) = db.function_body(func_id) else {
return;
};
for (_expression_id, expression) in &function_body.arenas.exprs {
match &expression {
Expr::Match(expr_match) => {
single_match::check_single_match(db, expr_match, diagnostics, &function_body.arenas)
}
Expr::Loop(expr_loop) => {
loops::check_loop_match_pop_front(db, expr_loop, diagnostics, &function_body.arenas)
}
Expr::FunctionCall(expr_func) => panic::check_panic_usage(db, expr_func, diagnostics),
_ => (),
};
}
}
Loading
Loading