From ffd1235303c4161374b9cbe63719557b7737bbbd Mon Sep 17 00:00:00 2001 From: LucasLvy Date: Tue, 6 Aug 2024 13:54:13 +0200 Subject: [PATCH] fix(core): if let correctly works now --- crates/unretardify-core/src/fix.rs | 71 ++++++++++--------- crates/unretardify-core/src/plugin.rs | 30 ++++++-- .../unretardify-core/tests/test_files/if_let | 38 ++++++++-- .../tests/test_files/unused_variables | 18 ++--- crates/unretardify-core/tests/tests.rs | 2 +- 5 files changed, 105 insertions(+), 54 deletions(-) diff --git a/crates/unretardify-core/src/fix.rs b/crates/unretardify-core/src/fix.rs index 0b2d5162..b3970fab 100644 --- a/crates/unretardify-core/src/fix.rs +++ b/crates/unretardify-core/src/fix.rs @@ -3,7 +3,8 @@ use cairo_lang_filesystem::span::TextSpan; use cairo_lang_semantic::diagnostic::SemanticDiagnosticKind; use cairo_lang_semantic::SemanticDiagnostic; use cairo_lang_syntax::node::ast::{ExprMatch, Pattern}; -use cairo_lang_syntax::node::TypedSyntaxNode; +use cairo_lang_syntax::node::db::SyntaxGroup; +use cairo_lang_syntax::node::{SyntaxNode, TypedSyntaxNode}; use cairo_lang_utils::Upcast; use crate::db::AnalysisDatabase; @@ -20,47 +21,47 @@ pub fn fix_semantic_diagnostic(db: &AnalysisDatabase, diag: &SemanticDiagnostic) SemanticDiagnosticKind::UnusedVariable => { format!("_{}", diag.stable_location.syntax_node(db.upcast()).get_text(db.upcast())) } - SemanticDiagnosticKind::PluginDiagnostic(ref diag) => fix_plugin_diagnostic(db, diag), + SemanticDiagnosticKind::PluginDiagnostic(ref diag) => Fixer.fix_plugin_diagnostic(db, diag), _ => "".to_owned(), } } -pub fn fix_plugin_diagnostic(db: &AnalysisDatabase, diag: &PluginDiagnostic) -> String { - match diagnostic_kind_from_message(&diag.message) { - RetardationKind::IfLet => { - // - let node = diag.stable_ptr.lookup(db.upcast()); - let match_expr = ExprMatch::from_syntax_node(db.upcast(), node); - let arms = match_expr.arms(db.upcast()).elements(db.upcast()); - let first_arm = &arms[0]; - let second_arm = &arms[1]; - match ( - &first_arm.patterns(db.upcast()).elements(db.upcast())[0], - &second_arm.patterns(db.upcast()).elements(db.upcast())[0], - ) { + +#[derive(Default)] +pub struct Fixer; +impl Fixer { + pub fn fix_if_let(&self, db: &dyn SyntaxGroup, node: SyntaxNode) -> String { + let match_expr = ExprMatch::from_syntax_node(db, node.clone()); + let arms = match_expr.arms(db).elements(db); + let first_arm = &arms[0]; + let second_arm = &arms[1]; + let (pattern, first_expr) = + match (&first_arm.patterns(db).elements(db)[0], &second_arm.patterns(db).elements(db)[0]) { (Pattern::Underscore(_), Pattern::Enum(pat)) => { - // - pat.as_syntax_node().get_text(db.upcast()) + (&pat.as_syntax_node().get_text_without_trivia(db), second_arm) } (Pattern::Enum(pat), Pattern::Underscore(_)) => { - // - format!( - "{}if let {} {{ {} }} else {{ {} }}", - match_expr - .as_syntax_node() - .get_text(db.upcast()) - .chars() - .take_while(|c| c.is_whitespace()) - .collect::(), - pat.as_syntax_node().get_text_without_trivia(db.upcast()), - first_arm.expression(db.upcast()).as_syntax_node().get_text_without_trivia(db.upcast()), - second_arm.expression(db.upcast()).as_syntax_node().get_text_without_trivia(db.upcast()) - ) + (&pat.as_syntax_node().get_text_without_trivia(db), first_arm) + } + (Pattern::Underscore(_), Pattern::Struct(pat)) => { + (&pat.as_syntax_node().get_text_without_trivia(db), second_arm) } - (Pattern::Underscore(_), Pattern::Struct(pat)) => pat.as_syntax_node().get_text(db.upcast()), - (Pattern::Struct(pat), Pattern::Underscore(_)) => pat.as_syntax_node().get_text(db.upcast()), - (_, _) => Default::default(), - } + (Pattern::Struct(pat), Pattern::Underscore(_)) => { + (&pat.as_syntax_node().get_text_without_trivia(db), first_arm) + } + (_, _) => panic!("Incorrect diagnostic"), + }; + format!( + "{}if let {} = {} {{ {} }}", + node.get_text(db).chars().take_while(|c| c.is_whitespace()).collect::(), + pattern, + match_expr.expr(db).as_syntax_node().get_text(db), + first_expr.expression(db).as_syntax_node().get_text_without_trivia(db), + ) + } + pub fn fix_plugin_diagnostic(&self, db: &AnalysisDatabase, diag: &PluginDiagnostic) -> String { + match diagnostic_kind_from_message(&diag.message) { + RetardationKind::IfLet => self.fix_if_let(db, diag.stable_ptr.lookup(db.upcast())), + _ => "".to_owned(), } - _ => "".to_owned(), } } diff --git a/crates/unretardify-core/src/plugin.rs b/crates/unretardify-core/src/plugin.rs index 00130287..518fd2d6 100644 --- a/crates/unretardify-core/src/plugin.rs +++ b/crates/unretardify-core/src/plugin.rs @@ -2,9 +2,10 @@ use std::ops::Deref; use cairo_lang_defs::ids::{ModuleId, ModuleItemId}; use cairo_lang_defs::plugin::PluginDiagnostic; +use cairo_lang_diagnostics::Severity; use cairo_lang_semantic::db::SemanticGroup; use cairo_lang_semantic::plugin::{AnalyzerPlugin, PluginSuite}; -use cairo_lang_syntax::node::ast::{ExprMatch, Pattern}; +use cairo_lang_syntax::node::ast::{Expr, ExprMatch, Pattern, Statement}; use cairo_lang_syntax::node::db::SyntaxGroup; use cairo_lang_syntax::node::kind::SyntaxKind; use cairo_lang_syntax::node::{TypedStablePtr, TypedSyntaxNode}; @@ -46,7 +47,28 @@ impl IsRetarded { let patterns = arm.patterns(db).elements(db); match patterns[0].clone() { Pattern::Underscore(_) => { - is_single_armed = true; + let tuple_expr = match arm.expression(db) { + Expr::Block(block_expr) => { + let statements = block_expr.statements(db).elements(db); + if statements.len() == 1 { + match &statements[0] { + Statement::Expr(statement_expr) => { + if let Expr::Tuple(tuple_expr) = statement_expr.expr(db) { + Some(tuple_expr) + } else { + None + } + } + _ => None, + } + } else { + None + } + } + Expr::Tuple(tuple_expr) => Some(tuple_expr), + _ => None, + }; + is_single_armed = tuple_expr.is_some_and(|list| list.expressions(db).elements(db).is_empty()); } Pattern::Enum(pat) => { @@ -63,12 +85,12 @@ impl IsRetarded { (true, false) => diagnostics.push(PluginDiagnostic { stable_ptr: match_expr.stable_ptr().untyped(), message: Self::IF.to_string(), - severity: cairo_lang_diagnostics::Severity::Warning, + severity: Severity::Warning, }), (true, true) => diagnostics.push(PluginDiagnostic { stable_ptr: match_expr.stable_ptr().untyped(), message: Self::IF_LET.to_string(), - severity: cairo_lang_diagnostics::Severity::Warning, + severity: Severity::Warning, }), (_, _) => (), } diff --git a/crates/unretardify-core/tests/test_files/if_let b/crates/unretardify-core/tests/test_files/if_let index 50a0fcaa..792f4480 100644 --- a/crates/unretardify-core/tests/test_files/if_let +++ b/crates/unretardify-core/tests/test_files/if_let @@ -4,8 +4,8 @@ fn main() { let variable = Option::Some(1_felt252); match variable { - Option::Some(a) => a + 2, - _ => 0, + Option::Some(a) => println!("{a}"), + _ => (), }; } @@ -13,13 +13,41 @@ fn main() { warning: Plugin diagnostic: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> lib.cairo:3:2 \ match variable { -| Option::Some(a) => a + 2, -| _ => 0, +| Option::Some(a) => println!("{a}"), +| _ => (), | } |___^ //! > fixed fn main() { let variable = Option::Some(1_felt252); - if let Option::Some(a) { a + 2 } else { 0 }; + if let Option::Some(a) = variable { println!("{a}") }; +} + +//! > ========================================================================== + +//! > simple if let with scope + +//! > cairo_code +fn main() { + let variable = Option::Some(1_felt252); + match variable { + Option::Some(a) => println!("{a}"), + _ => { () }, + }; +} + +//! > diagnostics +warning: Plugin diagnostic: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` + --> lib.cairo:3:2 +\ match variable { +| Option::Some(a) => println!("{a}"), +| _ => { () }, +| } +|___^ + +//! > fixed +fn main() { + let variable = Option::Some(1_felt252); + if let Option::Some(a) = variable { println!("{a}") }; } diff --git a/crates/unretardify-core/tests/test_files/unused_variables b/crates/unretardify-core/tests/test_files/unused_variables index e6a25826..bd801c6c 100644 --- a/crates/unretardify-core/tests/test_files/unused_variables +++ b/crates/unretardify-core/tests/test_files/unused_variables @@ -1,9 +1,8 @@ -//! > two unused variable +//! > one unused variable //! > cairo_code fn main() { let a: Option = Option::Some(1); - let b = 1; } //! > diagnostics @@ -12,24 +11,19 @@ warning[E0001]: Unused variable. Consider ignoring by prefixing with `_`. let a: Option = Option::Some(1); ^ -warning[E0001]: Unused variable. Consider ignoring by prefixing with `_`. - --> lib.cairo:3:9 - let b = 1; - ^ - //! > fixed fn main() { let _a: Option = Option::Some(1); - let _b = 1; } //! > ========================================================================== -//! > one unused variable +//! > two unused variable //! > cairo_code fn main() { let a: Option = Option::Some(1); + let b = 1; } //! > diagnostics @@ -38,7 +32,13 @@ warning[E0001]: Unused variable. Consider ignoring by prefixing with `_`. let a: Option = Option::Some(1); ^ +warning[E0001]: Unused variable. Consider ignoring by prefixing with `_`. + --> lib.cairo:3:9 + let b = 1; + ^ + //! > fixed fn main() { let _a: Option = Option::Some(1); + let _b = 1; } diff --git a/crates/unretardify-core/tests/tests.rs b/crates/unretardify-core/tests/tests.rs index 86ed43c5..7d3936df 100644 --- a/crates/unretardify-core/tests/tests.rs +++ b/crates/unretardify-core/tests/tests.rs @@ -16,4 +16,4 @@ use unretardify_test_utils::{get_diags, test_file, Tests}; test_file!(unused_variables, "one unused variable", "two unused variable"); -test_file!(if_let, "simple if let"); +test_file!(if_let, "simple if let", "simple if let with scope");