diff --git a/crates/cairo-lint-core/src/fix.rs b/crates/cairo-lint-core/src/fix.rs index 667df806..1518e1f8 100644 --- a/crates/cairo-lint-core/src/fix.rs +++ b/crates/cairo-lint-core/src/fix.rs @@ -451,10 +451,7 @@ impl Fixer { pub fn fix_manual_ok_or(&self, db: &dyn SyntaxGroup, node: SyntaxNode) -> String { let expr_match = ExprMatch::from_syntax_node(db, node.clone()); - let option_var_name = match expr_match.expr(db) { - Expr::Path(path_expr) => path_expr.as_syntax_node().get_text_without_trivia(db), - _ => panic!("Expected a variable or path in match expression"), - }; + let option_var_name = expr_match.expr(db).as_syntax_node().get_text_without_trivia(db); let arms = expr_match.arms(db).elements(db); if arms.len() != 2 { @@ -497,10 +494,7 @@ impl Fixer { pub fn fix_manual_is_some(&self, db: &dyn SyntaxGroup, node: SyntaxNode) -> String { let expr_match = ExprMatch::from_syntax_node(db, node.clone()); - let option_var_name = match expr_match.expr(db) { - Expr::Path(path_expr) => path_expr.as_syntax_node().get_text_without_trivia(db), - _ => panic!("Expected a variable or path in match expression"), - }; + let option_var_name = expr_match.expr(db).as_syntax_node().get_text_without_trivia(db); format!("{option_var_name}.is_some()") } @@ -509,10 +503,7 @@ impl Fixer { pub fn fix_manual_is_none(&self, db: &dyn SyntaxGroup, node: SyntaxNode) -> String { let expr_match = ExprMatch::from_syntax_node(db, node.clone()); - let option_var_name = match expr_match.expr(db) { - Expr::Path(path_expr) => path_expr.as_syntax_node().get_text_without_trivia(db), - _ => panic!("Expected a variable or path in match expression"), - }; + let option_var_name = expr_match.expr(db).as_syntax_node().get_text_without_trivia(db); format!("{option_var_name}.is_none()") } @@ -521,10 +512,7 @@ impl Fixer { pub fn fix_manual_expect(&self, db: &dyn SyntaxGroup, node: SyntaxNode) -> String { let expr_match = ExprMatch::from_syntax_node(db, node.clone()); - let option_var_name = match expr_match.expr(db) { - Expr::Path(path_expr) => path_expr.as_syntax_node().get_text_without_trivia(db), - _ => panic!("Expected a variable or path in match expression"), - }; + let option_var_name = expr_match.expr(db).as_syntax_node().get_text_without_trivia(db); let arms = expr_match.arms(db).elements(db); if arms.len() != 2 { diff --git a/crates/cairo-lint-core/tests/test_files/manual/manual_expect b/crates/cairo-lint-core/tests/test_files/manual/manual_expect index e36785d3..e4889a2c 100644 --- a/crates/cairo-lint-core/tests/test_files/manual/manual_expect +++ b/crates/cairo-lint-core/tests/test_files/manual/manual_expect @@ -160,4 +160,44 @@ fn main() { core::panic_with_felt252('err') }, }; -} \ No newline at end of file +} + +//! > ========================================================================== + +//! > test match expression is a function + +//! > cairo_code +fn foo(a: u256) -> Option { + Option::Some(a) +} +fn main() { + let a: u256 = 0; + let _a = match foo(a) { + Option::Some(value) => value, + Option::None => core::panic_with_felt252('err') + }; +} + +//! > diagnostics + +warning: Plugin diagnostic: Manual match for expect detected. Consider using `expect()` instead + --> lib.cairo:10:14 + | +10 | let _a = match foo(a) { + | ______________- +11 | | Option::Some(value) => value, +12 | | Option::None => core::panic_with_felt252('err') +13 | | }; + | |_____- + | + +//! > fixed +fn foo(a: u256) -> Option { + Option::Some(a) +} +fn main() { + let a: u256 = 0; + let _a = foo(a).expect('err'); +} + + diff --git a/crates/cairo-lint-core/tests/test_files/manual/manual_is_none b/crates/cairo-lint-core/tests/test_files/manual/manual_is_none index 80a3c676..0886d4de 100644 --- a/crates/cairo-lint-core/tests/test_files/manual/manual_is_none +++ b/crates/cairo-lint-core/tests/test_files/manual/manual_is_none @@ -88,3 +88,40 @@ fn main() { }; } +//! > ========================================================================== + +//! > test match expression is a function + +//! > cairo_code +fn foo(a: u256) -> Option { + Option::Some(a) +} +fn main() { + let a: u256 = 0; + let _a = match foo(a) { + Option::Some(_) => false, + Option::None => true + }; +} + +//! > diagnostics + +warning: Plugin diagnostic: Manual match for `is_none` detected. Consider using `is_none()` instead + --> lib.cairo:10:14 + | +10 | let _a = match foo(a) { + | ______________- +11 | | Option::Some(_) => false, +12 | | Option::None => true +13 | | }; + | |_____- + | + +//! > fixed +fn foo(a: u256) -> Option { + Option::Some(a) +} +fn main() { + let a: u256 = 0; + let _a = foo(a).is_none(); +} diff --git a/crates/cairo-lint-core/tests/test_files/manual/manual_is_some b/crates/cairo-lint-core/tests/test_files/manual/manual_is_some index 28eb72d1..00d14505 100644 --- a/crates/cairo-lint-core/tests/test_files/manual/manual_is_some +++ b/crates/cairo-lint-core/tests/test_files/manual/manual_is_some @@ -88,3 +88,41 @@ fn main() { }; } +//! > ========================================================================== + +//! > test match expression is a function + +//! > cairo_code +fn foo(a: u256) -> Option { + Option::Some(a) +} +fn main() { + let a: u256 = 0; + let _a = match foo(a) { + Option::Some(_) => true, + Option::None => false + }; +} + + +//! > diagnostics + +warning: Plugin diagnostic: Manual match for `is_some` detected. Consider using `is_some()` instead + --> lib.cairo:10:12 + | +10 | let _a = match foo(a) { + | ____________- +11 | | Option::Some(_) => true, +12 | | Option::None => false +13 | | }; + | |___- + | + +//! > fixed +fn foo(a: u256) -> Option { + Option::Some(a) +} +fn main() { + let a: u256 = 0; + let _a = foo(a).is_some(); +} diff --git a/crates/cairo-lint-core/tests/test_files/manual/manual_ok_or b/crates/cairo-lint-core/tests/test_files/manual/manual_ok_or index 5023a393..dee0e4cb 100644 --- a/crates/cairo-lint-core/tests/test_files/manual/manual_ok_or +++ b/crates/cairo-lint-core/tests/test_files/manual/manual_ok_or @@ -125,4 +125,44 @@ fn main() { }, Option::None => Result::Err('this is an err'), }; +} + +//! > ========================================================================== + +//! > test match expression not a variable + +//! > cairo_code +#[derive(Copy, Drop)] +enum Error { + Error, +} +fn main() { + let self: u256 = 0; + let _self_result: Result = match self.try_into() { + Option::Some(value) => Result::Ok(value), + Option::None => Result::Err(Error::Error) + }; +} + +//! > diagnostics + +warning: Plugin diagnostic: Manual match for Option detected. Consider using ok_or instead + --> lib.cairo:12:43 + | +12 | let _self_result: Result = match self.try_into() { + | ___________________________________________- +13 | | Option::Some(value) => Result::Ok(value), +14 | | Option::None => Result::Err(Error::Error) +15 | | }; + | |_____- + | + +//! > fixed +#[derive(Copy, Drop)] +enum Error { + Error, +} +fn main() { + let self: u256 = 0; + let _self_result: Result = self.try_into().ok_or(Error::Error); } \ No newline at end of file diff --git a/crates/cairo-lint-core/tests/tests.rs b/crates/cairo-lint-core/tests/tests.rs index c0e8c399..1a9c9726 100644 --- a/crates/cairo-lint-core/tests/tests.rs +++ b/crates/cairo-lint-core/tests/tests.rs @@ -196,7 +196,8 @@ test_file!( "test error str", "test error enum", "test with comment in None", - "test with comment in Some" + "test with comment in Some", + "test match expression not a variable" ); test_file!( @@ -208,9 +209,23 @@ test_file!( "with conditional logic" ); -test_file!(manual, manual_is_some, "test basic is some", "test with comment in Some", "test with comment in None"); +test_file!( + manual, + manual_is_some, + "test basic is some", + "test with comment in Some", + "test with comment in None", + "test match expression is a function" +); -test_file!(manual, manual_is_none, "test basic is none", "test with comment in Some", "test with comment in None"); +test_file!( + manual, + manual_is_none, + "test basic is none", + "test with comment in Some", + "test with comment in None", + "test match expression is a function" +); test_file!( manual, @@ -219,5 +234,6 @@ test_file!( "test panic_with_felt252", "test with enum error", "test with comment in Some", - "test with comment in None" + "test with comment in None", + "test match expression is a function" );