Skip to content

Commit

Permalink
feat(lint): Add double_parens (#24)
Browse files Browse the repository at this point in the history
* feat(lint): implement double parentheses removal with extensive test coverage

* fix: improve return datatype in tests

:wq

* feat: add more tests for double_parens lint

* fix: format applied

* fix: rebase applied, changes made and it is working

* fix: cargo clippy happy again and format applied

* fix: rebase applied and fix documented

* feat: add refactor improvement for lint
  • Loading branch information
coxmars authored Aug 19, 2024
1 parent 6ea84a1 commit b9a1161
Show file tree
Hide file tree
Showing 7 changed files with 320 additions and 3 deletions.
38 changes: 37 additions & 1 deletion crates/cairo-lint-core/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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::{ExprMatch, Pattern};
use cairo_lang_syntax::node::ast::{Expr, ExprMatch, Pattern};
use cairo_lang_syntax::node::db::SyntaxGroup;
use cairo_lang_syntax::node::kind::SyntaxKind;
use cairo_lang_syntax::node::{SyntaxNode, TypedStablePtr, TypedSyntaxNode};
Expand Down Expand Up @@ -142,12 +142,48 @@ impl Fixer {
plugin_diag: &PluginDiagnostic,
) -> Option<(SyntaxNode, String)> {
let new_text = match diagnostic_kind_from_message(&plugin_diag.message) {
CairoLintKind::DoubleParens => {
self.fix_double_parens(db.upcast(), plugin_diag.stable_ptr.lookup(db.upcast()))
}
CairoLintKind::DestructMatch => self.fix_destruct_match(db, plugin_diag.stable_ptr.lookup(db.upcast())),
_ => "".to_owned(),
};

Some((semantic_diag.stable_location.syntax_node(db.upcast()), new_text))
}

/// Removes unnecessary double parentheses from a syntax node.
///
/// Simplifies an expression by stripping extra layers of parentheses while preserving
/// the original formatting and indentation.
///
/// # Arguments
///
/// * `db` - Reference to the `SyntaxGroup` for syntax tree access.
/// * `node` - The `SyntaxNode` containing the expression.
///
/// # Returns
///
/// A `String` with the simplified expression.
///
/// # Example
///
/// Input: `((x + y))`
/// Output: `x + y`
pub fn fix_double_parens(&self, db: &dyn SyntaxGroup, node: SyntaxNode) -> String {
let mut expr = Expr::from_syntax_node(db, node.clone());

while let Expr::Parenthesized(inner_expr) = expr {
expr = inner_expr.expr(db);
}

format!(
"{}{}",
node.get_text(db).chars().take_while(|c| c.is_whitespace()).collect::<String>(),
expr.as_syntax_node().get_text_without_trivia(db),
)
}

/// Attempts to fix an unused import by removing it.
///
/// This function handles both single imports and imports within a use tree.
Expand Down
23 changes: 23 additions & 0 deletions crates/cairo-lint-core/src/lints/double_parens.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use cairo_lang_defs::plugin::PluginDiagnostic;
use cairo_lang_diagnostics::Severity;
use cairo_lang_syntax::node::ast::Expr;
use cairo_lang_syntax::node::db::SyntaxGroup;
use cairo_lang_syntax::node::{TypedStablePtr, TypedSyntaxNode};

pub const DOUBLE_PARENS: &str = "unnecessary double parentheses found. Consider removing them.";

pub fn check_double_parens(db: &dyn SyntaxGroup, expr: &Expr, diagnostics: &mut Vec<PluginDiagnostic>) {
let is_double_parens = if let Expr::Parenthesized(parenthesized_expr) = expr {
matches!(parenthesized_expr.expr(db), Expr::Parenthesized(_) | Expr::Tuple(_))
} else {
false
};

if is_double_parens {
diagnostics.push(PluginDiagnostic {
stable_ptr: expr.stable_ptr().untyped(),
message: DOUBLE_PARENS.to_string(),
severity: Severity::Warning,
});
}
}
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 +1,2 @@
pub mod double_parens;
pub mod single_match;
1 change: 1 addition & 0 deletions crates/cairo-lint-core/src/lints/single_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub fn is_expr_unit(expr: Expr, db: &dyn SyntaxGroup) -> bool {
_ => false,
}
}

pub fn check_single_match(
db: &dyn SemanticGroup,
match_expr: &ExprMatch,
Expand Down
11 changes: 9 additions & 2 deletions crates/cairo-lint-core/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use cairo_lang_defs::ids::{ModuleId, ModuleItemId};
use cairo_lang_defs::plugin::PluginDiagnostic;
use cairo_lang_semantic::db::SemanticGroup;
use cairo_lang_semantic::plugin::{AnalyzerPlugin, PluginSuite};
use cairo_lang_syntax::node::ast::ExprMatch;
use cairo_lang_syntax::node::ast::{Expr, ExprMatch};
use cairo_lang_syntax::node::kind::SyntaxKind;
use cairo_lang_syntax::node::TypedSyntaxNode;

use crate::lints::single_match;
use crate::lints::{double_parens, single_match};

pub fn cairo_lint_plugin_suite() -> PluginSuite {
let mut suite = PluginSuite::default();
Expand All @@ -20,13 +20,15 @@ pub struct CairoLint;
pub enum CairoLintKind {
DestructMatch,
MatchForEquality,
DoubleParens,
Unknown,
}

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,
_ => CairoLintKind::Unknown,
}
}
Expand All @@ -51,6 +53,11 @@ impl AnalyzerPlugin for CairoLint {
&mut diags,
&module_id,
),
SyntaxKind::ExprParenthesized => double_parens::check_double_parens(
db.upcast(),
&Expr::from_syntax_node(db.upcast(), descendant),
&mut diags,
),
SyntaxKind::ItemExternFunction => (),
_ => (),
}
Expand Down
234 changes: 234 additions & 0 deletions crates/cairo-lint-core/tests/test_files/double_parens/double_parens
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
//! > assert expressions

//! > cairo_code
fn main() {
assert_eq!(((4)), 4);
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:2:16
assert_eq!(((4)), 4);
^***^

//! > fixed
fn main() {
assert_eq!(4, 4);
}

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

//! > double parens in let statement

//! > cairo_code
fn main() {
let x = ((10 * 2));
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:2:13
let x = ((10 * 2));
^********^

warning[E0001]: Unused variable. Consider ignoring by prefixing with `_`.
--> lib.cairo:2:9
let x = ((10 * 2));
^

//! > fixed
fn main() {
let _x = 10 * 2;
}

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

//! > double parens in match arm

//! > cairo_code
fn main() -> felt252 {
let x = 5;
match x {
1 => ((10)),
5 => ((20)),
_ => ((30)),
}
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:4:14
1 => ((10)),
^****^

warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:5:14
5 => ((20)),
^****^

warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:6:14
_ => ((30)),
^****^

//! > fixed
fn main() -> felt252 {
let x = 5;
match x {
1 => 10,
5 => 20,
_ => 30,
}
}

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

//! > double parens in struct field access

//! > cairo_code
struct MyStruct {
x: felt252,
y: felt252,
}

fn main() -> felt252 {
let my_struct = MyStruct { x: 10, y: 20 };
return ((my_struct.y));
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:8:12
return ((my_struct.y));
^*************^

//! > fixed
struct MyStruct {
x: felt252,
y: felt252,
}

fn main() -> felt252 {
let my_struct = MyStruct { x: 10, y: 20 };
return my_struct.y;
}

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

//! > double parens with function call

//! > cairo_code
fn foo(x: felt252) -> felt252 {
x * 2
}

fn main() -> felt252 {
((foo(10)))
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:6:5
((foo(10)))
^*********^

//! > fixed
fn foo(x: felt252) -> felt252 {
x * 2
}

fn main() -> felt252 {
foo(10)}

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

//! > double parens with return

//! > cairo_code
fn main() -> felt252 {
return ((5 + 7));
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:2:12
return ((5 + 7));
^*******^

//! > fixed
fn main() -> felt252 {
return 5 + 7;
}

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

//! > necessary parentheses in arithmetic expression

//! > cairo_code
fn main() -> u32 {
2 * (3 + 5)
}

//! > diagnostics

//! > fixed
fn main() -> u32 {
2 * (3 + 5)
}

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

//! > simple double parens

//! > cairo_code
fn main() -> u32 {
((0))
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:2:5
((0))
^***^

//! > fixed
fn main() -> u32 {
0}

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

//! > tuple double parens

//! > cairo_code
fn main() -> (felt252, felt252) {
((1, 2))
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:2:5
((1, 2))
^******^

//! > fixed
fn main() -> (felt252, felt252) {
(1, 2)}

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

//! > unnecessary parentheses in arithmetic expression

//! > cairo_code
fn main() -> u32 {
((3 + 5))
}

//! > diagnostics
warning: Plugin diagnostic: unnecessary double parentheses found. Consider removing them.
--> lib.cairo:2:5
((3 + 5))
^*******^

//! > fixed
fn main() -> u32 {
3 + 5}
15 changes: 15 additions & 0 deletions crates/cairo-lint-core/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,18 @@ test_file!(
"unused import aliased",
"unused import trait"
);

test_file!(
double_parens,
double_parens,
"simple double parens",
"unnecessary parentheses in arithmetic expression",
"necessary parentheses in arithmetic expression",
"tuple double parens",
"assert expressions",
"double parens with function call",
"double parens with return",
"double parens in let statement",
"double parens in struct field access",
"double parens in match arm"
);

0 comments on commit b9a1161

Please sign in to comment.