feat(core): add loop_match_pop_front lint

crates/cairo-lint-core/src/fix.rs

@@ -3,7 +3,9 @@ use cairo_lang_defs::plugin::PluginDiagnostic;
 use cairo_lang_filesystem::span::TextSpan;
 use cairo_lang_semantic::diagnostic::SemanticDiagnosticKind;
 use cairo_lang_semantic::SemanticDiagnostic;
-use cairo_lang_syntax::node::ast::{BlockOrIf, ElseClause, Expr, ExprBinary, ExprMatch, Pattern, Statement};
+use cairo_lang_syntax::node::ast::{
+ BlockOrIf, ElseClause, Expr, ExprBinary, ExprLoop, ExprMatch, OptionPatternEnumInnerPattern, Pattern, Statement,
+};
 use cairo_lang_syntax::node::db::SyntaxGroup;
 use cairo_lang_syntax::node::{SyntaxNode, TypedSyntaxNode};
 use cairo_lang_utils::Upcast;
@@ -160,6 +162,9 @@ impl Fixer {
  db,
  &ElseClause::from_syntax_node(db.upcast(), plugin_diag.stable_ptr.lookup(db.upcast())),
  ),
+ CairoLintKind::LoopMatchPopFront => {
+ self.fix_loop_match_pop_front(db, plugin_diag.stable_ptr.lookup(db.upcast()))
+ }
  _ => return None,
  };
 
@@ -177,6 +182,60 @@ impl Fixer {
  let result = generate_fixed_text_for_comparison(db, lhs.as_str(), rhs.as_str(), node.clone());
  result
  }
+ pub fn fix_loop_match_pop_front(&self, db: &dyn SyntaxGroup, node: SyntaxNode) -> String {
+ let expr_loop = ExprLoop::from_syntax_node(db, node.clone());
+ let body = expr_loop.body(db);
+ let Statement::Expr(expr) = &body.statements(db).elements(db)[0] else {
+ panic!("Wrong statement type. This is probably a bug in the lint detection. Please report it")
+ };
+ let Expr::Match(expr_match) = expr.expr(db) else {
+ panic!("Wrong expression type. This is probably a bug in the lint detection. Please report it")
+ };
+ let val = expr_match.expr(db);
+ let span_name = match val {
+ Expr::FunctionCall(func_call) => func_call.arguments(db).arguments(db).elements(db)[0]
+ .arg_clause(db)
+ .as_syntax_node()
+ .get_text_without_trivia(db),
+ Expr::Binary(dot_call) => dot_call.lhs(db).as_syntax_node().get_text_without_trivia(db),
+ _ => panic!("Wrong expressiin type. This is probably a bug in the lint detection. Please report it"),
+ };
+ let mut elt_name = "".to_owned();
+ let mut some_arm = "".to_owned();
+ let arms = expr_match.arms(db).elements(db);
+
+ let mut loop_span = node.span(db);
+ loop_span.end = node.span_start_without_trivia(db);
+ let indent = node.get_text(db).chars().take_while(|c| c.is_whitespace()).collect::<String>();
+ let trivia = node.clone().get_text_of_span(db, loop_span).trim().to_string();
+ let trivia = if trivia.is_empty() { trivia } else { format!("{indent}{trivia}\n") };
+ for arm in arms {
+ if let Pattern::Enum(enum_pattern) = &arm.patterns(db).elements(db)[0]
+ && let OptionPatternEnumInnerPattern::PatternEnumInnerPattern(var) = enum_pattern.pattern(db)
+ {
+ elt_name = var.pattern(db).as_syntax_node().get_text_without_trivia(db);
+ some_arm = if let Expr::Block(block_expr) = arm.expression(db) {
+ block_expr
+ .statements(db)
+ .elements(db)
+ .iter()
+ .map(|statement| {
+ statement
+ .as_syntax_node()
+ .get_text(db)
+ .lines()
+ .map(|line| line.strip_prefix(" ").unwrap_or(line).to_owned() + "\n")
+ .collect::<String>()
+ })
+ .collect::<String>()
+ } else {
+ arm.expression(db).as_syntax_node().get_text(db)
+ }
+ }
+ }
+
+ format!("{trivia}{indent}for {elt_name} in {span_name} {{\n{some_arm}\n{indent}}};\n")
+ }
 
  /// Removes unnecessary double parentheses from a syntax node.
  ///

crates/cairo-lint-core/src/lints/loops.rs

@@ -1,7 +1,11 @@
+use cairo_lang_defs::ids::NamedLanguageElementId;
 use cairo_lang_defs::plugin::PluginDiagnostic;
 use cairo_lang_diagnostics::Severity;
 use cairo_lang_semantic::db::SemanticGroup;
-use cairo_lang_semantic::{Arenas, Expr, ExprLoop, Statement};
+use cairo_lang_semantic::{
+ Arenas, Expr, ExprBlock, ExprId, ExprLoop, ExprMatch, Pattern, PatternEnumVariant, Statement,
+};
+use cairo_lang_syntax::node::{TypedStablePtr, TypedSyntaxNode};
 
 pub const LOOP_MATCH_POP_FRONT: &str =
  "you seem to be trying to use `loop` for iterating over a span. Consider using `for in`";
@@ -22,6 +26,9 @@ pub fn check_loop_match_pop_front(
  && let Expr::FunctionCall(func_call) = &arenas.exprs[expr_match.matched_expr]
  && func_call.function.name(db) == SPAN_MATCH_POP_FRONT
  {
+ if !check_single_match(db, expr_match, arenas) {
+ return;
+ }
  diagnostics.push(PluginDiagnostic {
  stable_ptr: loop_expr.stable_ptr.into(),
  message: LOOP_MATCH_POP_FRONT.to_owned(),
@@ -33,6 +40,9 @@ pub fn check_loop_match_pop_front(
  if let Statement::Expr(stmt_expr) = &arenas.statements[*statement]
  && let Expr::Match(expr_match) = &arenas.exprs[stmt_expr.expr]
  {
+ if !check_single_match(db, expr_match, arenas) {
+ continue;
+ }
  let Expr::FunctionCall(func_call) = &arenas.exprs[expr_match.matched_expr] else {
  continue;
  };
@@ -46,3 +56,85 @@ pub fn check_loop_match_pop_front(
  }
  }
 }
+
+const OPTION_TYPE: &str = "core::option::Option::<";
+const SOME_VARIANT: &str = "Some";
+const NONE_VARIANT: &str = "None";
+
+pub fn check_single_match(db: &dyn SemanticGroup, match_expr: &ExprMatch, arenas: &Arenas) -> bool {
+ let arms = &match_expr.arms;
+
+ // Check that we're in a setup with 2 arms that return unit
+ if arms.len() == 2 && match_expr.ty.is_unit(db) {
+ let first_arm = &arms[0];
+ let second_arm = &arms[1];
+ let is_first_arm_correct = if let Some(pattern) = first_arm.patterns.first() {
+ match &arenas.patterns[*pattern] {
+ // If the first arm is `_ => smth` it's incorrect
+ Pattern::Otherwise(_) => false,
+ // Check if the variant is of type option and if it's `None` checks that it only contains `{ break; }`
+ // without comments`
+ Pattern::EnumVariant(enum_pat) => check_enum_pattern(db, enum_pat, arenas, first_arm.expression),
+ _ => false,
+ }
+ } else {
+ false
+ };
+ let is_second_arm_correct = if let Some(pattern) = second_arm.patterns.first() {
+ match &arenas.patterns[*pattern] {
+ // If the 2nd arm is `_ => smth`, checks that smth is `{ break; }`
+ Pattern::Otherwise(_) => {
+ if let Expr::Block(expr_block) = &arenas.exprs[second_arm.expression] {
+ check_block_is_break(db, expr_block, arenas)
+ } else {
+ return false;
+ }
+ }
+ // Check if the variant is of type option and if it's `None` checks that it only contains `{ break; }`
+ // without comments`
+ Pattern::EnumVariant(enum_pat) => check_enum_pattern(db, enum_pat, arenas, second_arm.expression),
+ _ => false,
+ }
+ } else {
+ false
+ };
+ is_first_arm_correct && is_second_arm_correct
+ } else {
+ false
+ }
+}
+fn check_enum_pattern(
+ db: &dyn SemanticGroup,
+ enum_pat: &PatternEnumVariant,
+ arenas: &Arenas,
+ arm_expression: ExprId,
+) -> bool {
+ // Checks that the variant is from the option type.
+ if !enum_pat.ty.format(db.upcast()).starts_with(OPTION_TYPE) {
+ return false;
+ }
+ // Check if the variant is the None variant
+ if enum_pat.variant.id.name(db.upcast()) == NONE_VARIANT
+ // Get the expression of the None variant and checks if it's a block expression. 
+ && let Expr::Block(expr_block) = &arenas.exprs[arm_expression]
+ // If it's a block expression checks that it only contains `break;`
+ && check_block_is_break(db, expr_block, arenas)
+ {
+ true
+ } else {
+ enum_pat.variant.id.name(db.upcast()) == SOME_VARIANT
+ }
+}
+/// Checks that the block only contains `break;` without comments
+fn check_block_is_break(db: &dyn SemanticGroup, expr_block: &ExprBlock, arenas: &Arenas) -> bool {
+ if expr_block.statements.len() == 1
+ && let Statement::Break(break_stmt) = &arenas.statements[expr_block.statements[0]]
+ {
+ let break_node = break_stmt.stable_ptr.lookup(db.upcast()).as_syntax_node();
+ // Checks that the trimmed text == the text without trivia which would mean that there is no comment
+ if break_node.get_text(db.upcast()).trim() == break_node.get_text_without_trivia(db.upcast()) {
+ return true;
+ }
+ }
+ false
+}

crates/cairo-lint-core/src/lints/single_match.rs

@@ -1,6 +1,5 @@
 use cairo_lang_defs::plugin::PluginDiagnostic;
 use cairo_lang_diagnostics::Severity;
-use cairo_lang_semantic::corelib::unit_ty;
 use cairo_lang_semantic::db::SemanticGroup;
 use cairo_lang_semantic::{Arenas, ExprMatch, Pattern};
 use cairo_lang_syntax::node::ast::{Expr as AstExpr, ExprBlock, ExprListParenthesized, Statement};
@@ -53,7 +52,7 @@ pub fn check_single_match(
  let mut is_complete = false;
  let mut is_destructuring = false;
 
- if arms.len() == 2 && match_expr.ty == unit_ty(db) {
+ if arms.len() == 2 && match_expr.ty.is_unit(db) {
  let first_arm = &arms[0];
  let second_arm = &arms[1];
  let mut enum_len = None;

crates/cairo-lint-core/src/plugin.rs

@@ -11,7 +11,6 @@ use crate::lints::{
  bool_comparison, breaks, collapsible_if_else, double_comparison, double_parens, duplicate_underscore_args, loops,
  single_match,
 };
-use crate::plugin::duplicate_underscore_args::check_duplicate_underscore_args;
 
 pub fn cairo_lint_plugin_suite() -> PluginSuite {
  let mut suite = PluginSuite::default();
@@ -27,11 +26,12 @@ pub enum CairoLintKind {
  MatchForEquality,
  DoubleComparison,
  DoubleParens,
- Unknown,
  BreakUnit,
  BoolComparison,
  CollapsibleIfElse,
  DuplicateUnderscoreArgs,
+ LoopMatchPopFront,
+ Unknown,
 }
 
 pub fn diagnostic_kind_from_message(message: &str) -> CairoLintKind {
@@ -46,36 +46,14 @@ pub fn diagnostic_kind_from_message(message: &str) -> CairoLintKind {
  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,
  _ => CairoLintKind::Unknown,
  }
 }
 
 impl AnalyzerPlugin for CairoLint {
  fn diagnostics(&self, db: &dyn SemanticGroup, module_id: ModuleId) -> Vec<PluginDiagnostic> {
  let mut diags = Vec::new();
- let Ok(free_functions_ids) = db.module_free_functions_ids(module_id) else {
- return diags;
- };
- for free_func_id in free_functions_ids.iter() {
- check_duplicate_underscore_args(
- db.function_with_body_signature(FunctionWithBodyId::Free(*free_func_id)).unwrap().params,
- &mut diags,
- );
- let Ok(function_body) = db.function_body(FunctionWithBodyId::Free(*free_func_id)) else {
- return diags;
- };
- for (_expression_id, expression) in &function_body.arenas.exprs {
- match &expression {
- Expr::Match(expr_match) => {
- single_match::check_single_match(db, expr_match, &mut diags, &function_body.arenas)
- }
- Expr::Loop(expr_loop) => {
- loops::check_loop_match_pop_front(db, expr_loop, &mut diags, &function_body.arenas)
- }
- _ => (),
- };
- }
- }
  let syntax_db = db.upcast();
  let Ok(items) = db.module_items(module_id) else {
  return diags;
@@ -86,8 +64,50 @@ impl AnalyzerPlugin for CairoLint {
  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);
+ duplicate_underscore_args::check_duplicate_underscore_args(
+ db.function_with_body_signature(func_id).unwrap().params,
+ &mut diags,
+ );
+ let Ok(function_body) = db.function_body(func_id) else {
+ continue;
+ };
+ for (_expression_id, expression) in &function_body.arenas.exprs {
+ match &expression {
+ Expr::Match(expr_match) => {
+ single_match::check_single_match(db, expr_match, &mut diags, &function_body.arenas)
+ }
+ Expr::Loop(expr_loop) => {
+ loops::check_loop_match_pop_front(db, expr_loop, &mut diags, &function_body.arenas)
+ }
+ _ => (),
+ };
+ }
  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 Ok(function_body) = db.function_body(FunctionWithBodyId::Impl(*fn_id)) else {
+ continue;
+ };
+ for (_expression_id, expression) in &function_body.arenas.exprs {
+ match &expression {
+ Expr::Match(expr_match) => {
+ single_match::check_single_match(db, expr_match, &mut diags, &function_body.arenas)
+ }
+ Expr::Loop(expr_loop) => {
+ loops::check_loop_match_pop_front(db, expr_loop, &mut diags, &function_body.arenas)
+ }
+ _ => (),
+ };
+ }
+ }
+ impl_id.stable_ptr(db.upcast()).lookup(syntax_db).as_syntax_node()
+ }
  _ => continue,
  }
  .descendants(syntax_db);