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.

Sign up for GitHub

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

fix: manual_lint bug for function expression #125

Merged
merged 1 commit into from
Sep 27, 2024
Merged

fix: manual_lint bug for function expression #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 4 additions & 16 deletions crates/cairo-lint-core/src/fix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()")
}
Expand All @@ -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()")
}
Expand All @@ -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 {
Expand Down
42 changes: 41 additions & 1 deletion crates/cairo-lint-core/tests/test_files/manual/manual_expect
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,44 @@ fn main() {
core::panic_with_felt252('err')
},
};
}
}

//! > ==========================================================================

//! > test match expression is a function

//! > cairo_code
fn foo(a: u256) -> Option<u256> {
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<u256> {
Option::Some(a)
}
fn main() {
let a: u256 = 0;
let _a = foo(a).expect('err');
}


37 changes: 37 additions & 0 deletions crates/cairo-lint-core/tests/test_files/manual/manual_is_none
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,40 @@ fn main() {
};
}

//! > ==========================================================================

//! > test match expression is a function

//! > cairo_code
fn foo(a: u256) -> Option<u256> {
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<u256> {
Option::Some(a)
}
fn main() {
let a: u256 = 0;
let _a = foo(a).is_none();
}
38 changes: 38 additions & 0 deletions crates/cairo-lint-core/tests/test_files/manual/manual_is_some
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,41 @@ fn main() {
};
}

//! > ==========================================================================

//! > test match expression is a function

//! > cairo_code
fn foo(a: u256) -> Option<u256> {
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<u256> {
Option::Some(a)
}
fn main() {
let a: u256 = 0;
let _a = foo(a).is_some();
}
40 changes: 40 additions & 0 deletions crates/cairo-lint-core/tests/test_files/manual/manual_ok_or
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<u8, Error> = match self.try_into() {
Option::Some(value) => Result::Ok(value),
Option::None => Result::Err(Error::Error)
};
}

//! > diagnostics

warning: Plugin diagnostic: Manual match for Option<T> detected. Consider using ok_or instead
--> lib.cairo:12:43
|
12 | let _self_result: Result<u8, Error> = 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<u8, Error> = self.try_into().ok_or(Error::Error);
}
24 changes: 20 additions & 4 deletions crates/cairo-lint-core/tests/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand All @@ -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,
Expand All @@ -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"
);
Loading