From beca92b245953873d273f19a08c7a927e5a3ed78 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 14 Aug 2021 16:40:00 +0300 Subject: [PATCH 1/5] internal: make invert binary op more robust Previously, we only inverted comparison operators (< and the like) if the type implemented Ord. This doesn't make sense: if `<` works, then `>=` will work as well! Extra semantic checks greatly reduce robustness and predictability of the assist, it's better to keep things simple. --- .../src/handlers/apply_demorgan.rs | 73 +++---------------- .../src/handlers/convert_bool_then.rs | 2 +- .../ide_assists/src/handlers/early_return.rs | 2 +- crates/ide_assists/src/handlers/invert_if.rs | 2 +- crates/ide_assists/src/tests/generated.rs | 2 +- crates/ide_assists/src/utils.rs | 50 +++---------- 6 files changed, 22 insertions(+), 109 deletions(-) diff --git a/crates/ide_assists/src/handlers/apply_demorgan.rs b/crates/ide_assists/src/handlers/apply_demorgan.rs index e2bd6e4567e..cafc4297fde 100644 --- a/crates/ide_assists/src/handlers/apply_demorgan.rs +++ b/crates/ide_assists/src/handlers/apply_demorgan.rs @@ -19,7 +19,7 @@ // -> // ``` // fn main() { -// if !(x == 4 && !(y < 3.14)) {} +// if !(x == 4 && y >= 3.14) {} // } // ``` pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { @@ -99,7 +99,7 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<( if let Some(paren_expr) = paren_expr { for term in terms { let range = term.syntax().text_range(); - let not_term = invert_boolean_expression(&ctx.sema, term); + let not_term = invert_boolean_expression(term); edit.replace(range, not_term.syntax().text()); } @@ -114,21 +114,21 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<( } else { if let Some(lhs) = terms.pop_front() { let lhs_range = lhs.syntax().text_range(); - let not_lhs = invert_boolean_expression(&ctx.sema, lhs); + let not_lhs = invert_boolean_expression(lhs); edit.replace(lhs_range, format!("!({}", not_lhs.syntax().text())); } if let Some(rhs) = terms.pop_back() { let rhs_range = rhs.syntax().text_range(); - let not_rhs = invert_boolean_expression(&ctx.sema, rhs); + let not_rhs = invert_boolean_expression(rhs); edit.replace(rhs_range, format!("{})", not_rhs.syntax().text())); } for term in terms { let term_range = term.syntax().text_range(); - let not_term = invert_boolean_expression(&ctx.sema, term); + let not_term = invert_boolean_expression(term); edit.replace(term_range, not_term.syntax().text()); } } @@ -156,40 +156,12 @@ fn demorgan_handles_leq() { check_assist( apply_demorgan, r#" -//- minicore: ord, derive -#[derive(PartialEq, Eq, PartialOrd, Ord)] struct S; - -fn f() { - S < S &&$0 S <= S -} -"#, - r#" -#[derive(PartialEq, Eq, PartialOrd, Ord)] -struct S; - -fn f() { - !(S >= S || S > S) -} -"#, - ); - - check_assist( - apply_demorgan, - r#" -//- minicore: ord, derive -struct S; - -fn f() { - S < S &&$0 S <= S -} +fn f() { S < S &&$0 S <= S } "#, r#" struct S; - -fn f() { - !(!(S < S) || !(S <= S)) -} +fn f() { !(S >= S || S > S) } "#, ); } @@ -199,39 +171,12 @@ fn demorgan_handles_geq() { check_assist( apply_demorgan, r#" -//- minicore: ord, derive -#[derive(PartialEq, Eq, PartialOrd, Ord)] struct S; - -fn f() { - S > S &&$0 S >= S -} -"#, - r#" -#[derive(PartialEq, Eq, PartialOrd, Ord)] -struct S; - -fn f() { - !(S <= S || S < S) -} -"#, - ); - check_assist( - apply_demorgan, - r#" -//- minicore: ord, derive -struct S; - -fn f() { - S > S &&$0 S >= S -} +fn f() { S > S &&$0 S >= S } "#, r#" struct S; - -fn f() { - !(!(S > S) || !(S >= S)) -} +fn f() { !(S <= S || S < S) } "#, ); } diff --git a/crates/ide_assists/src/handlers/convert_bool_then.rs b/crates/ide_assists/src/handlers/convert_bool_then.rs index 3bb78fe0f29..5adb3f5a1b3 100644 --- a/crates/ide_assists/src/handlers/convert_bool_then.rs +++ b/crates/ide_assists/src/handlers/convert_bool_then.rs @@ -97,7 +97,7 @@ pub(crate) fn convert_if_to_bool_then(acc: &mut Assists, ctx: &AssistContext) -> e => e, }; - let cond = if invert_cond { invert_boolean_expression(&ctx.sema, cond) } else { cond }; + let cond = if invert_cond { invert_boolean_expression(cond) } else { cond }; let arg_list = make::arg_list(Some(make::expr_closure(None, closure_body))); let mcall = make::expr_method_call(cond, make::name_ref("then"), arg_list); builder.replace(target, mcall.to_string()); diff --git a/crates/ide_assists/src/handlers/early_return.rs b/crates/ide_assists/src/handlers/early_return.rs index b4745b84242..1b3fa898bb7 100644 --- a/crates/ide_assists/src/handlers/early_return.rs +++ b/crates/ide_assists/src/handlers/early_return.rs @@ -115,7 +115,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext) let new_expr = { let then_branch = make::block_expr(once(make::expr_stmt(early_expression).into()), None); - let cond = invert_boolean_expression(&ctx.sema, cond_expr); + let cond = invert_boolean_expression(cond_expr); make::expr_if(make::condition(cond, None), then_branch, None) .indent(if_indent_level) }; diff --git a/crates/ide_assists/src/handlers/invert_if.rs b/crates/ide_assists/src/handlers/invert_if.rs index f7f38dffbda..50845cd9e03 100644 --- a/crates/ide_assists/src/handlers/invert_if.rs +++ b/crates/ide_assists/src/handlers/invert_if.rs @@ -47,7 +47,7 @@ pub(crate) fn invert_if(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { }; acc.add(AssistId("invert_if", AssistKind::RefactorRewrite), "Invert if", if_range, |edit| { - let flip_cond = invert_boolean_expression(&ctx.sema, cond.clone()); + let flip_cond = invert_boolean_expression(cond.clone()); edit.replace_ast(cond, flip_cond); let else_node = else_block.syntax(); diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index c4df6aec9fc..853c41f78f4 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -151,7 +151,7 @@ fn main() { "#####, r#####" fn main() { - if !(x == 4 && !(y < 3.14)) {} + if !(x == 4 && y >= 3.14) {} } "#####, ) diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index a4e4a00f78d..256ddb8c9b2 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -5,12 +5,8 @@ use std::ops; -use hir::{Adt, HasSource, Semantics}; -use ide_db::{ - helpers::{FamousDefs, SnippetCap}, - path_transform::PathTransform, - RootDatabase, -}; +use hir::{Adt, HasSource}; +use ide_db::{helpers::SnippetCap, path_transform::PathTransform, RootDatabase}; use itertools::Itertools; use stdx::format_to; use syntax::{ @@ -207,31 +203,19 @@ pub(crate) fn vis_offset(node: &SyntaxNode) -> TextSize { .unwrap_or_else(|| node.text_range().start()) } -pub(crate) fn invert_boolean_expression( - sema: &Semantics, - expr: ast::Expr, -) -> ast::Expr { - invert_special_case(sema, &expr).unwrap_or_else(|| make::expr_prefix(T![!], expr)) +pub(crate) fn invert_boolean_expression(expr: ast::Expr) -> ast::Expr { + invert_special_case(&expr).unwrap_or_else(|| make::expr_prefix(T![!], expr)) } -fn invert_special_case(sema: &Semantics, expr: &ast::Expr) -> Option { +fn invert_special_case(expr: &ast::Expr) -> Option { match expr { ast::Expr::BinExpr(bin) => match bin.op_kind()? { ast::BinOp::NegatedEqualityTest => bin.replace_op(T![==]).map(|it| it.into()), ast::BinOp::EqualityTest => bin.replace_op(T![!=]).map(|it| it.into()), - // Swap `<` with `>=`, `<=` with `>`, ... if operands `impl Ord` - ast::BinOp::LesserTest if bin_impls_ord(sema, bin) => { - bin.replace_op(T![>=]).map(|it| it.into()) - } - ast::BinOp::LesserEqualTest if bin_impls_ord(sema, bin) => { - bin.replace_op(T![>]).map(|it| it.into()) - } - ast::BinOp::GreaterTest if bin_impls_ord(sema, bin) => { - bin.replace_op(T![<=]).map(|it| it.into()) - } - ast::BinOp::GreaterEqualTest if bin_impls_ord(sema, bin) => { - bin.replace_op(T![<]).map(|it| it.into()) - } + ast::BinOp::LesserTest => bin.replace_op(T![>=]).map(|it| it.into()), + ast::BinOp::LesserEqualTest => bin.replace_op(T![>]).map(|it| it.into()), + ast::BinOp::GreaterTest => bin.replace_op(T![<=]).map(|it| it.into()), + ast::BinOp::GreaterEqualTest => bin.replace_op(T![<]).map(|it| it.into()), // Parenthesize other expressions before prefixing `!` _ => Some(make::expr_prefix(T![!], make::expr_paren(expr.clone()))), }, @@ -267,22 +251,6 @@ fn invert_special_case(sema: &Semantics, expr: &ast::Expr) -> Opti } } -fn bin_impls_ord(sema: &Semantics, bin: &ast::BinExpr) -> bool { - match ( - bin.lhs().and_then(|lhs| sema.type_of_expr(&lhs)).map(hir::TypeInfo::adjusted), - bin.rhs().and_then(|rhs| sema.type_of_expr(&rhs)).map(hir::TypeInfo::adjusted), - ) { - (Some(lhs_ty), Some(rhs_ty)) if lhs_ty == rhs_ty => { - let krate = sema.scope(bin.syntax()).module().map(|it| it.krate()); - let ord_trait = FamousDefs(sema, krate).core_cmp_Ord(); - ord_trait.map_or(false, |ord_trait| { - lhs_ty.autoderef(sema.db).any(|ty| ty.impls_trait(sema.db, ord_trait, &[])) - }) - } - _ => false, - } -} - pub(crate) fn next_prev() -> impl Iterator { [Direction::Next, Direction::Prev].iter().copied() } From faa420fc32566bd9de81d5d14445dd25bb3694a3 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 14 Aug 2021 16:58:46 +0300 Subject: [PATCH 2/5] internal: prepare a dedicated module for all operators --- crates/syntax/src/ast.rs | 14 ++++++++--- crates/syntax/src/ast/expr_ext.rs | 37 ++++++++++-------------------- crates/syntax/src/ast/operators.rs | 17 ++++++++++++++ 3 files changed, 40 insertions(+), 28 deletions(-) create mode 100644 crates/syntax/src/ast/operators.rs diff --git a/crates/syntax/src/ast.rs b/crates/syntax/src/ast.rs index a8071b51d2f..fce09851d94 100644 --- a/crates/syntax/src/ast.rs +++ b/crates/syntax/src/ast.rs @@ -5,6 +5,7 @@ mod token_ext; mod node_ext; mod expr_ext; +mod operators; pub mod edit; pub mod edit_in_place; pub mod make; @@ -17,14 +18,21 @@ }; pub use self::{ - expr_ext::{ArrayExprKind, BinOp, Effect, ElseBranch, LiteralKind, PrefixOp, RangeOp}, + expr_ext::{ArrayExprKind, BinOp, Effect, ElseBranch, LiteralKind}, generated::{nodes::*, tokens::*}, node_ext::{ AttrKind, AttrsOwnerNode, FieldKind, Macro, NameLike, NameOrNameRef, PathSegmentKind, SelfParamKind, SlicePatComponents, StructKind, TypeBoundKind, VisibilityKind, }, - token_ext::*, - traits::*, + operators::{PrefixOp, RangeOp}, + token_ext::{ + CommentKind, CommentPlacement, CommentShape, HasFormatSpecifier, IsString, QuoteOffsets, + Radix, + }, + traits::{ + ArgListOwner, AttrsOwner, CommentIter, DocCommentsOwner, GenericParamsOwner, LoopBodyOwner, + ModuleItemOwner, NameOwner, TypeBoundsOwner, VisibilityOwner, + }, }; /// The main trait to go from untyped `SyntaxNode` to a typed ast. The diff --git a/crates/syntax/src/ast/expr_ext.rs b/crates/syntax/src/ast/expr_ext.rs index 2dfb0d1ad31..f482a45dbed 100644 --- a/crates/syntax/src/ast/expr_ext.rs +++ b/crates/syntax/src/ast/expr_ext.rs @@ -3,7 +3,11 @@ use rowan::WalkEvent; use crate::{ - ast::{self, support, AstChildren, AstNode}, + ast::{ + self, + operators::{PrefixOp, RangeOp}, + support, AstChildren, AstNode, + }, AstToken, SyntaxKind::*, SyntaxToken, T, @@ -193,24 +197,15 @@ pub fn blocks(&self) -> AstChildren { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum PrefixOp { - /// The `*` operator for dereferencing - Deref, - /// The `!` operator for logical inversion - Not, - /// The `-` operator for negation - Neg, -} - impl ast::PrefixExpr { pub fn op_kind(&self) -> Option { - match self.op_token()?.kind() { - T![*] => Some(PrefixOp::Deref), - T![!] => Some(PrefixOp::Not), - T![-] => Some(PrefixOp::Neg), - _ => None, - } + let res = match self.op_token()?.kind() { + T![*] => PrefixOp::Deref, + T![!] => PrefixOp::Not, + T![-] => PrefixOp::Neg, + _ => return None, + }; + Some(res) } pub fn op_token(&self) -> Option { @@ -398,14 +393,6 @@ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum RangeOp { - /// `..` - Exclusive, - /// `..=` - Inclusive, -} - impl ast::RangeExpr { fn op_details(&self) -> Option<(usize, SyntaxToken, RangeOp)> { self.syntax().children_with_tokens().enumerate().find_map(|(ix, child)| { diff --git a/crates/syntax/src/ast/operators.rs b/crates/syntax/src/ast/operators.rs new file mode 100644 index 00000000000..03fd7da8406 --- /dev/null +++ b/crates/syntax/src/ast/operators.rs @@ -0,0 +1,17 @@ +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum PrefixOp { + /// The `*` operator for dereferencing + Deref, + /// The `!` operator for logical inversion + Not, + /// The `-` operator for negation + Neg, +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum RangeOp { + /// `..` + Exclusive, + /// `..=` + Inclusive, +} From 6df00f8495e2c8aa5e7312a6e293dee169be137b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 14 Aug 2021 17:01:28 +0300 Subject: [PATCH 3/5] internal: make naming consistent --- crates/hir_def/src/expr.rs | 2 +- crates/ide/src/syntax_highlighting/highlight.rs | 2 +- crates/ide_assists/src/handlers/apply_demorgan.rs | 2 +- crates/ide_assists/src/handlers/pull_assignment_up.rs | 4 ++-- crates/ide_assists/src/utils.rs | 2 +- crates/ide_assists/src/utils/suggest_name.rs | 2 +- crates/syntax/src/ast.rs | 2 +- crates/syntax/src/ast/expr_ext.rs | 10 +++++----- crates/syntax/src/ast/operators.rs | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/hir_def/src/expr.rs b/crates/hir_def/src/expr.rs index e8e73f79fe2..f1e66d9f685 100644 --- a/crates/hir_def/src/expr.rs +++ b/crates/hir_def/src/expr.rs @@ -219,7 +219,7 @@ pub enum ArithOp { BitAnd, } -pub use syntax::ast::PrefixOp as UnaryOp; +pub use syntax::ast::UnaryOp; #[derive(Debug, Clone, Eq, PartialEq)] pub enum Array { ElementList(Vec), diff --git a/crates/ide/src/syntax_highlighting/highlight.rs b/crates/ide/src/syntax_highlighting/highlight.rs index 5a7683c5779..f82566d978e 100644 --- a/crates/ide/src/syntax_highlighting/highlight.rs +++ b/crates/ide/src/syntax_highlighting/highlight.rs @@ -126,7 +126,7 @@ pub(super) fn element( let ty = sema.type_of_expr(&expr)?.original; if ty.is_raw_ptr() { HlTag::Operator(HlOperator::Other) | HlMod::Unsafe - } else if let Some(ast::PrefixOp::Deref) = prefix_expr.op_kind() { + } else if let Some(ast::UnaryOp::Deref) = prefix_expr.op_kind() { HlOperator::Other.into() } else { HlPunct::Other.into() diff --git a/crates/ide_assists/src/handlers/apply_demorgan.rs b/crates/ide_assists/src/handlers/apply_demorgan.rs index cafc4297fde..b8822595d42 100644 --- a/crates/ide_assists/src/handlers/apply_demorgan.rs +++ b/crates/ide_assists/src/handlers/apply_demorgan.rs @@ -85,7 +85,7 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<( .and_then(|paren_expr| paren_expr.syntax().parent()) .and_then(ast::PrefixExpr::cast) .and_then(|prefix_expr| { - if prefix_expr.op_kind().unwrap() == ast::PrefixOp::Not { + if prefix_expr.op_kind().unwrap() == ast::UnaryOp::Not { Some(prefix_expr) } else { None diff --git a/crates/ide_assists/src/handlers/pull_assignment_up.rs b/crates/ide_assists/src/handlers/pull_assignment_up.rs index f07b8a6c0d5..4d0041e424d 100644 --- a/crates/ide_assists/src/handlers/pull_assignment_up.rs +++ b/crates/ide_assists/src/handlers/pull_assignment_up.rs @@ -173,8 +173,8 @@ fn is_equivalent( } } (ast::Expr::PrefixExpr(prefix0), ast::Expr::PrefixExpr(prefix1)) - if prefix0.op_kind() == Some(ast::PrefixOp::Deref) - && prefix1.op_kind() == Some(ast::PrefixOp::Deref) => + if prefix0.op_kind() == Some(ast::UnaryOp::Deref) + && prefix1.op_kind() == Some(ast::UnaryOp::Deref) => { cov_mark::hit!(test_pull_assignment_up_deref); if let (Some(prefix0), Some(prefix1)) = (prefix0.expr(), prefix1.expr()) { diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 256ddb8c9b2..476525d1be8 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -233,7 +233,7 @@ fn invert_special_case(expr: &ast::Expr) -> Option { }; Some(make::expr_method_call(receiver, make::name_ref(method), arg_list)) } - ast::Expr::PrefixExpr(pe) if pe.op_kind()? == ast::PrefixOp::Not => { + ast::Expr::PrefixExpr(pe) if pe.op_kind()? == ast::UnaryOp::Not => { if let ast::Expr::ParenExpr(parexpr) = pe.expr()? { parexpr.expr() } else { diff --git a/crates/ide_assists/src/utils/suggest_name.rs b/crates/ide_assists/src/utils/suggest_name.rs index c1513f97dad..17db6d3c19d 100644 --- a/crates/ide_assists/src/utils/suggest_name.rs +++ b/crates/ide_assists/src/utils/suggest_name.rs @@ -110,7 +110,7 @@ pub(crate) fn for_variable(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) } ast::Expr::ParenExpr(inner) => next_expr = inner.expr(), ast::Expr::TryExpr(inner) => next_expr = inner.expr(), - ast::Expr::PrefixExpr(prefix) if prefix.op_kind() == Some(ast::PrefixOp::Deref) => { + ast::Expr::PrefixExpr(prefix) if prefix.op_kind() == Some(ast::UnaryOp::Deref) => { next_expr = prefix.expr() } _ => break, diff --git a/crates/syntax/src/ast.rs b/crates/syntax/src/ast.rs index fce09851d94..2dcbcccc106 100644 --- a/crates/syntax/src/ast.rs +++ b/crates/syntax/src/ast.rs @@ -24,7 +24,7 @@ AttrKind, AttrsOwnerNode, FieldKind, Macro, NameLike, NameOrNameRef, PathSegmentKind, SelfParamKind, SlicePatComponents, StructKind, TypeBoundKind, VisibilityKind, }, - operators::{PrefixOp, RangeOp}, + operators::{RangeOp, UnaryOp}, token_ext::{ CommentKind, CommentPlacement, CommentShape, HasFormatSpecifier, IsString, QuoteOffsets, Radix, diff --git a/crates/syntax/src/ast/expr_ext.rs b/crates/syntax/src/ast/expr_ext.rs index f482a45dbed..3ebb85fc830 100644 --- a/crates/syntax/src/ast/expr_ext.rs +++ b/crates/syntax/src/ast/expr_ext.rs @@ -5,7 +5,7 @@ use crate::{ ast::{ self, - operators::{PrefixOp, RangeOp}, + operators::{RangeOp, UnaryOp}, support, AstChildren, AstNode, }, AstToken, @@ -198,11 +198,11 @@ pub fn blocks(&self) -> AstChildren { } impl ast::PrefixExpr { - pub fn op_kind(&self) -> Option { + pub fn op_kind(&self) -> Option { let res = match self.op_token()?.kind() { - T![*] => PrefixOp::Deref, - T![!] => PrefixOp::Not, - T![-] => PrefixOp::Neg, + T![*] => UnaryOp::Deref, + T![!] => UnaryOp::Not, + T![-] => UnaryOp::Neg, _ => return None, }; Some(res) diff --git a/crates/syntax/src/ast/operators.rs b/crates/syntax/src/ast/operators.rs index 03fd7da8406..e8eaf743ba2 100644 --- a/crates/syntax/src/ast/operators.rs +++ b/crates/syntax/src/ast/operators.rs @@ -1,5 +1,5 @@ #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum PrefixOp { +pub enum UnaryOp { /// The `*` operator for dereferencing Deref, /// The `!` operator for logical inversion From fe4f059450dd3f69dc5c0b6801d4c8a414673cab Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 14 Aug 2021 17:07:51 +0300 Subject: [PATCH 4/5] internal: prepare to merge hir::BinaryOp and ast::BinOp --- crates/hir_def/src/body/lower.rs | 49 +-------------- crates/hir_def/src/expr.rs | 44 +------------- crates/syntax/src/ast.rs | 6 +- crates/syntax/src/ast/operators.rs | 95 ++++++++++++++++++++++++++++-- 4 files changed, 97 insertions(+), 97 deletions(-) diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs index 4f912136739..1c8a33a33aa 100644 --- a/crates/hir_def/src/body/lower.rs +++ b/crates/hir_def/src/body/lower.rs @@ -27,9 +27,8 @@ builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, db::DefDatabase, expr::{ - dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Label, - LabelId, Literal, LogicOp, MatchArm, MatchGuard, Ordering, Pat, PatId, RecordFieldPat, - RecordLitField, Statement, + dummy_expr_id, Array, BinaryOp, BindingAnnotation, Expr, ExprId, Label, LabelId, Literal, + MatchArm, MatchGuard, Pat, PatId, RecordFieldPat, RecordLitField, Statement, }, intern::Interned, item_scope::BuiltinShadowMode, @@ -954,50 +953,6 @@ fn check_cfg(&mut self, owner: &dyn ast::AttrsOwner) -> Option<()> { } } -impl From for BinaryOp { - fn from(ast_op: ast::BinOp) -> Self { - match ast_op { - ast::BinOp::BooleanOr => BinaryOp::LogicOp(LogicOp::Or), - ast::BinOp::BooleanAnd => BinaryOp::LogicOp(LogicOp::And), - ast::BinOp::EqualityTest => BinaryOp::CmpOp(CmpOp::Eq { negated: false }), - ast::BinOp::NegatedEqualityTest => BinaryOp::CmpOp(CmpOp::Eq { negated: true }), - ast::BinOp::LesserEqualTest => { - BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: false }) - } - ast::BinOp::GreaterEqualTest => { - BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: false }) - } - ast::BinOp::LesserTest => { - BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: true }) - } - ast::BinOp::GreaterTest => { - BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: true }) - } - ast::BinOp::Addition => BinaryOp::ArithOp(ArithOp::Add), - ast::BinOp::Multiplication => BinaryOp::ArithOp(ArithOp::Mul), - ast::BinOp::Subtraction => BinaryOp::ArithOp(ArithOp::Sub), - ast::BinOp::Division => BinaryOp::ArithOp(ArithOp::Div), - ast::BinOp::Remainder => BinaryOp::ArithOp(ArithOp::Rem), - ast::BinOp::LeftShift => BinaryOp::ArithOp(ArithOp::Shl), - ast::BinOp::RightShift => BinaryOp::ArithOp(ArithOp::Shr), - ast::BinOp::BitwiseXor => BinaryOp::ArithOp(ArithOp::BitXor), - ast::BinOp::BitwiseOr => BinaryOp::ArithOp(ArithOp::BitOr), - ast::BinOp::BitwiseAnd => BinaryOp::ArithOp(ArithOp::BitAnd), - ast::BinOp::Assignment => BinaryOp::Assignment { op: None }, - ast::BinOp::AddAssign => BinaryOp::Assignment { op: Some(ArithOp::Add) }, - ast::BinOp::DivAssign => BinaryOp::Assignment { op: Some(ArithOp::Div) }, - ast::BinOp::MulAssign => BinaryOp::Assignment { op: Some(ArithOp::Mul) }, - ast::BinOp::RemAssign => BinaryOp::Assignment { op: Some(ArithOp::Rem) }, - ast::BinOp::ShlAssign => BinaryOp::Assignment { op: Some(ArithOp::Shl) }, - ast::BinOp::ShrAssign => BinaryOp::Assignment { op: Some(ArithOp::Shr) }, - ast::BinOp::SubAssign => BinaryOp::Assignment { op: Some(ArithOp::Sub) }, - ast::BinOp::BitOrAssign => BinaryOp::Assignment { op: Some(ArithOp::BitOr) }, - ast::BinOp::BitAndAssign => BinaryOp::Assignment { op: Some(ArithOp::BitAnd) }, - ast::BinOp::BitXorAssign => BinaryOp::Assignment { op: Some(ArithOp::BitXor) }, - } - } -} - impl From for Literal { fn from(ast_lit_kind: ast::LiteralKind) -> Self { match ast_lit_kind { diff --git a/crates/hir_def/src/expr.rs b/crates/hir_def/src/expr.rs index f1e66d9f685..b508d875e81 100644 --- a/crates/hir_def/src/expr.rs +++ b/crates/hir_def/src/expr.rs @@ -14,7 +14,6 @@ use hir_expand::name::Name; use la_arena::{Idx, RawIdx}; -use syntax::ast::RangeOp; use crate::{ builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, @@ -24,6 +23,8 @@ BlockId, }; +pub use syntax::ast::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering, RangeOp, UnaryOp}; + pub type ExprId = Idx; pub(crate) fn dummy_expr_id() -> ExprId { ExprId::from_raw(RawIdx::from(!0)) @@ -179,47 +180,6 @@ pub enum Expr { Literal(Literal), } -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum BinaryOp { - LogicOp(LogicOp), - ArithOp(ArithOp), - CmpOp(CmpOp), - Assignment { op: Option }, -} - -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum LogicOp { - And, - Or, -} - -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum CmpOp { - Eq { negated: bool }, - Ord { ordering: Ordering, strict: bool }, -} - -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum Ordering { - Less, - Greater, -} - -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum ArithOp { - Add, - Mul, - Sub, - Div, - Rem, - Shl, - Shr, - BitXor, - BitOr, - BitAnd, -} - -pub use syntax::ast::UnaryOp; #[derive(Debug, Clone, Eq, PartialEq)] pub enum Array { ElementList(Vec), diff --git a/crates/syntax/src/ast.rs b/crates/syntax/src/ast.rs index 2dcbcccc106..799ba0ced9d 100644 --- a/crates/syntax/src/ast.rs +++ b/crates/syntax/src/ast.rs @@ -24,10 +24,10 @@ AttrKind, AttrsOwnerNode, FieldKind, Macro, NameLike, NameOrNameRef, PathSegmentKind, SelfParamKind, SlicePatComponents, StructKind, TypeBoundKind, VisibilityKind, }, - operators::{RangeOp, UnaryOp}, + operators::{ArithOp, BinaryOp, CmpOp, LogicOp, RangeOp, UnaryOp, Ordering}, token_ext::{ - CommentKind, CommentPlacement, CommentShape, HasFormatSpecifier, IsString, QuoteOffsets, - Radix, + CommentKind, CommentPlacement, CommentShape, FormatSpecifier, HasFormatSpecifier, IsString, + QuoteOffsets, Radix, }, traits::{ ArgListOwner, AttrsOwner, CommentIter, DocCommentsOwner, GenericParamsOwner, LoopBodyOwner, diff --git a/crates/syntax/src/ast/operators.rs b/crates/syntax/src/ast/operators.rs index e8eaf743ba2..84cb7c0b012 100644 --- a/crates/syntax/src/ast/operators.rs +++ b/crates/syntax/src/ast/operators.rs @@ -1,3 +1,11 @@ +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum RangeOp { + /// `..` + Exclusive, + /// `..=` + Inclusive, +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum UnaryOp { /// The `*` operator for dereferencing @@ -9,9 +17,86 @@ pub enum UnaryOp { } #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum RangeOp { - /// `..` - Exclusive, - /// `..=` - Inclusive, +pub enum BinaryOp { + LogicOp(LogicOp), + ArithOp(ArithOp), + CmpOp(CmpOp), + Assignment { op: Option }, +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum LogicOp { + And, + Or, +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum CmpOp { + Eq { negated: bool }, + Ord { ordering: Ordering, strict: bool }, +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum Ordering { + Less, + Greater, +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum ArithOp { + Add, + Mul, + Sub, + Div, + Rem, + Shl, + Shr, + BitXor, + BitOr, + BitAnd, +} + +use crate::ast; +impl From for BinaryOp { + fn from(ast_op: ast::BinOp) -> Self { + match ast_op { + ast::BinOp::BooleanOr => BinaryOp::LogicOp(LogicOp::Or), + ast::BinOp::BooleanAnd => BinaryOp::LogicOp(LogicOp::And), + ast::BinOp::EqualityTest => BinaryOp::CmpOp(CmpOp::Eq { negated: false }), + ast::BinOp::NegatedEqualityTest => BinaryOp::CmpOp(CmpOp::Eq { negated: true }), + ast::BinOp::LesserEqualTest => { + BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: false }) + } + ast::BinOp::GreaterEqualTest => { + BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: false }) + } + ast::BinOp::LesserTest => { + BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: true }) + } + ast::BinOp::GreaterTest => { + BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: true }) + } + ast::BinOp::Addition => BinaryOp::ArithOp(ArithOp::Add), + ast::BinOp::Multiplication => BinaryOp::ArithOp(ArithOp::Mul), + ast::BinOp::Subtraction => BinaryOp::ArithOp(ArithOp::Sub), + ast::BinOp::Division => BinaryOp::ArithOp(ArithOp::Div), + ast::BinOp::Remainder => BinaryOp::ArithOp(ArithOp::Rem), + ast::BinOp::LeftShift => BinaryOp::ArithOp(ArithOp::Shl), + ast::BinOp::RightShift => BinaryOp::ArithOp(ArithOp::Shr), + ast::BinOp::BitwiseXor => BinaryOp::ArithOp(ArithOp::BitXor), + ast::BinOp::BitwiseOr => BinaryOp::ArithOp(ArithOp::BitOr), + ast::BinOp::BitwiseAnd => BinaryOp::ArithOp(ArithOp::BitAnd), + ast::BinOp::Assignment => BinaryOp::Assignment { op: None }, + ast::BinOp::AddAssign => BinaryOp::Assignment { op: Some(ArithOp::Add) }, + ast::BinOp::DivAssign => BinaryOp::Assignment { op: Some(ArithOp::Div) }, + ast::BinOp::MulAssign => BinaryOp::Assignment { op: Some(ArithOp::Mul) }, + ast::BinOp::RemAssign => BinaryOp::Assignment { op: Some(ArithOp::Rem) }, + ast::BinOp::ShlAssign => BinaryOp::Assignment { op: Some(ArithOp::Shl) }, + ast::BinOp::ShrAssign => BinaryOp::Assignment { op: Some(ArithOp::Shr) }, + ast::BinOp::SubAssign => BinaryOp::Assignment { op: Some(ArithOp::Sub) }, + ast::BinOp::BitOrAssign => BinaryOp::Assignment { op: Some(ArithOp::BitOr) }, + ast::BinOp::BitAndAssign => BinaryOp::Assignment { op: Some(ArithOp::BitAnd) }, + ast::BinOp::BitXorAssign => BinaryOp::Assignment { op: Some(ArithOp::BitXor) }, + } + } } From 90357a9090a88b2f7bc5db5d05cb576680ce2960 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 14 Aug 2021 18:08:31 +0300 Subject: [PATCH 5/5] internal: merge hir::BinaryOp and ast::BinOp --- crates/hir_def/src/body/lower.rs | 6 +- .../src/handlers/apply_demorgan.rs | 17 +- .../src/handlers/extract_function.rs | 2 +- .../ide_assists/src/handlers/flip_binexpr.rs | 21 +- .../src/handlers/pull_assignment_up.rs | 4 +- crates/ide_assists/src/utils.rs | 17 +- .../src/utils/gen_trait_fn_body.rs | 25 ++- crates/ide_db/src/search.rs | 2 +- crates/syntax/src/ast.rs | 4 +- crates/syntax/src/ast/expr_ext.rs | 186 ++++-------------- crates/syntax/src/ast/make.rs | 3 +- crates/syntax/src/ast/operators.rs | 106 ++++++---- 12 files changed, 160 insertions(+), 233 deletions(-) diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs index 1c8a33a33aa..d8dac66556b 100644 --- a/crates/hir_def/src/body/lower.rs +++ b/crates/hir_def/src/body/lower.rs @@ -27,8 +27,8 @@ builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint}, db::DefDatabase, expr::{ - dummy_expr_id, Array, BinaryOp, BindingAnnotation, Expr, ExprId, Label, LabelId, Literal, - MatchArm, MatchGuard, Pat, PatId, RecordFieldPat, RecordLitField, Statement, + dummy_expr_id, Array, BindingAnnotation, Expr, ExprId, Label, LabelId, Literal, MatchArm, + MatchGuard, Pat, PatId, RecordFieldPat, RecordLitField, Statement, }, intern::Interned, item_scope::BuiltinShadowMode, @@ -508,7 +508,7 @@ fn maybe_collect_expr(&mut self, expr: ast::Expr) -> Option { ast::Expr::BinExpr(e) => { let lhs = self.collect_expr_opt(e.lhs()); let rhs = self.collect_expr_opt(e.rhs()); - let op = e.op_kind().map(BinaryOp::from); + let op = e.op_kind(); self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr) } ast::Expr::TupleExpr(e) => { diff --git a/crates/ide_assists/src/handlers/apply_demorgan.rs b/crates/ide_assists/src/handlers/apply_demorgan.rs index b8822595d42..9c888e2d4e3 100644 --- a/crates/ide_assists/src/handlers/apply_demorgan.rs +++ b/crates/ide_assists/src/handlers/apply_demorgan.rs @@ -26,7 +26,13 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<( let expr = ctx.find_node_at_offset::()?; let op = expr.op_kind()?; let op_range = expr.op_token()?.text_range(); - let opposite_op = opposite_logic_op(op)?; + + let opposite_op = match op { + ast::BinaryOp::LogicOp(ast::LogicOp::And) => "||", + ast::BinaryOp::LogicOp(ast::LogicOp::Or) => "&&", + _ => return None, + }; + let cursor_in_range = op_range.contains_range(ctx.frange.range); if !cursor_in_range { return None; @@ -136,15 +142,6 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<( ) } -// Return the opposite text for a given logical operator, if it makes sense -fn opposite_logic_op(kind: ast::BinOp) -> Option<&'static str> { - match kind { - ast::BinOp::BooleanOr => Some("&&"), - ast::BinOp::BooleanAnd => Some("||"), - _ => None, - } -} - #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 692c3f7f071..5e96cec511c 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -892,7 +892,7 @@ fn expr_require_exclusive_access(ctx: &AssistContext, expr: &ast::Expr) -> Optio let parent = expr.syntax().parent()?; if let Some(bin_expr) = ast::BinExpr::cast(parent.clone()) { - if bin_expr.op_kind()?.is_assignment() { + if matches!(bin_expr.op_kind()?, ast::BinaryOp::Assignment { .. }) { return Some(bin_expr.lhs()?.syntax() == expr.syntax()); } return Some(false); diff --git a/crates/ide_assists/src/handlers/flip_binexpr.rs b/crates/ide_assists/src/handlers/flip_binexpr.rs index 209e5d43c24..0117b8a84b8 100644 --- a/crates/ide_assists/src/handlers/flip_binexpr.rs +++ b/crates/ide_assists/src/handlers/flip_binexpr.rs @@ -1,4 +1,4 @@ -use syntax::ast::{AstNode, BinExpr, BinOp}; +use syntax::ast::{self, AstNode, BinExpr}; use crate::{AssistContext, AssistId, AssistKind, Assists}; @@ -56,14 +56,19 @@ enum FlipAction { DontFlip, } -impl From for FlipAction { - fn from(op_kind: BinOp) -> Self { +impl From for FlipAction { + fn from(op_kind: ast::BinaryOp) -> Self { match op_kind { - kind if kind.is_assignment() => FlipAction::DontFlip, - BinOp::GreaterTest => FlipAction::FlipAndReplaceOp("<"), - BinOp::GreaterEqualTest => FlipAction::FlipAndReplaceOp("<="), - BinOp::LesserTest => FlipAction::FlipAndReplaceOp(">"), - BinOp::LesserEqualTest => FlipAction::FlipAndReplaceOp(">="), + ast::BinaryOp::Assignment { .. } => FlipAction::DontFlip, + ast::BinaryOp::CmpOp(ast::CmpOp::Ord { ordering, strict }) => { + let rev_op = match (ordering, strict) { + (ast::Ordering::Less, true) => ">", + (ast::Ordering::Less, false) => ">=", + (ast::Ordering::Greater, true) => "<", + (ast::Ordering::Greater, false) => "<=", + }; + FlipAction::FlipAndReplaceOp(rev_op) + } _ => FlipAction::Flip, } } diff --git a/crates/ide_assists/src/handlers/pull_assignment_up.rs b/crates/ide_assists/src/handlers/pull_assignment_up.rs index 4d0041e424d..8946ecfac7c 100644 --- a/crates/ide_assists/src/handlers/pull_assignment_up.rs +++ b/crates/ide_assists/src/handlers/pull_assignment_up.rs @@ -39,7 +39,7 @@ pub(crate) fn pull_assignment_up(acc: &mut Assists, ctx: &AssistContext) -> Opti let assign_expr = ctx.find_node_at_offset::()?; let op_kind = assign_expr.op_kind()?; - if op_kind != ast::BinOp::Assignment { + if op_kind != (ast::BinaryOp::Assignment { op: None }) { cov_mark::hit!(test_cant_pull_non_assignments); return None; } @@ -143,7 +143,7 @@ fn collect_block(&mut self, block: &ast::BlockExpr) -> Option<()> { } fn collect_expr(&mut self, expr: &ast::BinExpr) -> Option<()> { - if expr.op_kind()? == ast::BinOp::Assignment + if expr.op_kind()? == (ast::BinaryOp::Assignment { op: None }) && is_equivalent(self.sema, &expr.lhs()?, &self.common_lhs) { self.assignments.push((expr.clone(), expr.rhs()?)); diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 476525d1be8..ee81e5048a3 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -210,12 +210,17 @@ pub(crate) fn invert_boolean_expression(expr: ast::Expr) -> ast::Expr { fn invert_special_case(expr: &ast::Expr) -> Option { match expr { ast::Expr::BinExpr(bin) => match bin.op_kind()? { - ast::BinOp::NegatedEqualityTest => bin.replace_op(T![==]).map(|it| it.into()), - ast::BinOp::EqualityTest => bin.replace_op(T![!=]).map(|it| it.into()), - ast::BinOp::LesserTest => bin.replace_op(T![>=]).map(|it| it.into()), - ast::BinOp::LesserEqualTest => bin.replace_op(T![>]).map(|it| it.into()), - ast::BinOp::GreaterTest => bin.replace_op(T![<=]).map(|it| it.into()), - ast::BinOp::GreaterEqualTest => bin.replace_op(T![<]).map(|it| it.into()), + ast::BinaryOp::CmpOp(op) => { + let rev_op = match op { + ast::CmpOp::Eq { negated: false } => T![!=], + ast::CmpOp::Eq { negated: true } => T![==], + ast::CmpOp::Ord { ordering: ast::Ordering::Less, strict: true } => T![>=], + ast::CmpOp::Ord { ordering: ast::Ordering::Less, strict: false } => T![>], + ast::CmpOp::Ord { ordering: ast::Ordering::Greater, strict: true } => T![<=], + ast::CmpOp::Ord { ordering: ast::Ordering::Greater, strict: false } => T![<], + }; + bin.replace_op(rev_op).map(ast::Expr::from) + } // Parenthesize other expressions before prefixing `!` _ => Some(make::expr_prefix(T![!], make::expr_paren(expr.clone()))), }, diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 111fa0b23fc..5a8914b3316 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -1,7 +1,7 @@ //! This module contains functions to generate default trait impl function bodies where possible. use syntax::{ - ast::{self, edit::AstNodeEdit, make, AstNode, NameOwner}, + ast::{self, edit::AstNodeEdit, make, AstNode, BinaryOp, CmpOp, LogicOp, NameOwner}, ted, }; @@ -325,7 +325,7 @@ fn gen_hash_call(target: ast::Expr) -> ast::Stmt { fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { fn gen_eq_chain(expr: Option, cmp: ast::Expr) -> Option { match expr { - Some(expr) => Some(make::expr_op(ast::BinOp::BooleanAnd, expr, cmp)), + Some(expr) => Some(make::expr_bin_op(expr, BinaryOp::LogicOp(LogicOp::And), cmp)), None => Some(cmp), } } @@ -362,7 +362,8 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let lhs = make::expr_call(make_discriminant()?, make::arg_list(Some(lhs_name.clone()))); let rhs_name = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_call(make_discriminant()?, make::arg_list(Some(rhs_name.clone()))); - let eq_check = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + let eq_check = + make::expr_bin_op(lhs, BinaryOp::CmpOp(CmpOp::Eq { negated: false }), rhs); let mut case_count = 0; let mut arms = vec![]; @@ -386,7 +387,11 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let lhs = make::expr_path(make::ext::ident_path(l_name)); let rhs = make::expr_path(make::ext::ident_path(r_name)); - let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + let cmp = make::expr_bin_op( + lhs, + BinaryOp::CmpOp(CmpOp::Eq { negated: false }), + rhs, + ); expr = gen_eq_chain(expr, cmp); } @@ -415,7 +420,11 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let lhs = make::expr_path(make::ext::ident_path(&l_name)); let rhs = make::expr_path(make::ext::ident_path(&r_name)); - let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + let cmp = make::expr_bin_op( + lhs, + BinaryOp::CmpOp(CmpOp::Eq { negated: false }), + rhs, + ); expr = gen_eq_chain(expr, cmp); } @@ -455,7 +464,8 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let lhs = make::expr_field(lhs, &field.name()?.to_string()); let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_field(rhs, &field.name()?.to_string()); - let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + let cmp = + make::expr_bin_op(lhs, BinaryOp::CmpOp(CmpOp::Eq { negated: false }), rhs); expr = gen_eq_chain(expr, cmp); } make::block_expr(None, expr).indent(ast::edit::IndentLevel(1)) @@ -469,7 +479,8 @@ fn gen_tuple_field(field_name: &String) -> ast::Pat { let lhs = make::expr_field(lhs, &idx); let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_field(rhs, &idx); - let cmp = make::expr_op(ast::BinOp::EqualityTest, lhs, rhs); + let cmp = + make::expr_bin_op(lhs, BinaryOp::CmpOp(CmpOp::Eq { negated: false }), rhs); expr = gen_eq_chain(expr, cmp); } make::block_expr(None, expr).indent(ast::edit::IndentLevel(1)) diff --git a/crates/ide_db/src/search.rs b/crates/ide_db/src/search.rs index 980abae93c4..627f5e97fa7 100644 --- a/crates/ide_db/src/search.rs +++ b/crates/ide_db/src/search.rs @@ -655,7 +655,7 @@ fn reference_access(def: &Definition, name_ref: &ast::NameRef) -> Option { - if expr.op_kind()?.is_assignment() { + if matches!(expr.op_kind()?, ast::BinaryOp::Assignment { .. }) { // If the variable or field ends on the LHS's end then it's a Write (covers fields and locals). // FIXME: This is not terribly accurate. if let Some(lhs) = expr.lhs() { diff --git a/crates/syntax/src/ast.rs b/crates/syntax/src/ast.rs index 799ba0ced9d..e26c5b7ad90 100644 --- a/crates/syntax/src/ast.rs +++ b/crates/syntax/src/ast.rs @@ -18,13 +18,13 @@ }; pub use self::{ - expr_ext::{ArrayExprKind, BinOp, Effect, ElseBranch, LiteralKind}, + expr_ext::{ArrayExprKind, Effect, ElseBranch, LiteralKind}, generated::{nodes::*, tokens::*}, node_ext::{ AttrKind, AttrsOwnerNode, FieldKind, Macro, NameLike, NameOrNameRef, PathSegmentKind, SelfParamKind, SlicePatComponents, StructKind, TypeBoundKind, VisibilityKind, }, - operators::{ArithOp, BinaryOp, CmpOp, LogicOp, RangeOp, UnaryOp, Ordering}, + operators::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering, RangeOp, UnaryOp}, token_ext::{ CommentKind, CommentPlacement, CommentShape, FormatSpecifier, HasFormatSpecifier, IsString, QuoteOffsets, Radix, diff --git a/crates/syntax/src/ast/expr_ext.rs b/crates/syntax/src/ast/expr_ext.rs index 3ebb85fc830..4598066bc93 100644 --- a/crates/syntax/src/ast/expr_ext.rs +++ b/crates/syntax/src/ast/expr_ext.rs @@ -5,7 +5,7 @@ use crate::{ ast::{ self, - operators::{RangeOp, UnaryOp}, + operators::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering, RangeOp, UnaryOp}, support, AstChildren, AstNode, }, AstToken, @@ -213,127 +213,51 @@ pub fn op_token(&self) -> Option { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum BinOp { - /// The `||` operator for boolean OR - BooleanOr, - /// The `&&` operator for boolean AND - BooleanAnd, - /// The `==` operator for equality testing - EqualityTest, - /// The `!=` operator for equality testing - NegatedEqualityTest, - /// The `<=` operator for lesser-equal testing - LesserEqualTest, - /// The `>=` operator for greater-equal testing - GreaterEqualTest, - /// The `<` operator for comparison - LesserTest, - /// The `>` operator for comparison - GreaterTest, - /// The `+` operator for addition - Addition, - /// The `*` operator for multiplication - Multiplication, - /// The `-` operator for subtraction - Subtraction, - /// The `/` operator for division - Division, - /// The `%` operator for remainder after division - Remainder, - /// The `<<` operator for left shift - LeftShift, - /// The `>>` operator for right shift - RightShift, - /// The `^` operator for bitwise XOR - BitwiseXor, - /// The `|` operator for bitwise OR - BitwiseOr, - /// The `&` operator for bitwise AND - BitwiseAnd, - /// The `=` operator for assignment - Assignment, - /// The `+=` operator for assignment after addition - AddAssign, - /// The `/=` operator for assignment after division - DivAssign, - /// The `*=` operator for assignment after multiplication - MulAssign, - /// The `%=` operator for assignment after remainders - RemAssign, - /// The `>>=` operator for assignment after shifting right - ShrAssign, - /// The `<<=` operator for assignment after shifting left - ShlAssign, - /// The `-=` operator for assignment after subtraction - SubAssign, - /// The `|=` operator for assignment after bitwise OR - BitOrAssign, - /// The `&=` operator for assignment after bitwise AND - BitAndAssign, - /// The `^=` operator for assignment after bitwise XOR - BitXorAssign, -} - -impl BinOp { - pub fn is_assignment(self) -> bool { - matches!( - self, - BinOp::Assignment - | BinOp::AddAssign - | BinOp::DivAssign - | BinOp::MulAssign - | BinOp::RemAssign - | BinOp::ShrAssign - | BinOp::ShlAssign - | BinOp::SubAssign - | BinOp::BitOrAssign - | BinOp::BitAndAssign - | BinOp::BitXorAssign - ) - } -} - impl ast::BinExpr { - pub fn op_details(&self) -> Option<(SyntaxToken, BinOp)> { + pub fn op_details(&self) -> Option<(SyntaxToken, BinaryOp)> { self.syntax().children_with_tokens().filter_map(|it| it.into_token()).find_map(|c| { + #[rustfmt::skip] let bin_op = match c.kind() { - T![||] => BinOp::BooleanOr, - T![&&] => BinOp::BooleanAnd, - T![==] => BinOp::EqualityTest, - T![!=] => BinOp::NegatedEqualityTest, - T![<=] => BinOp::LesserEqualTest, - T![>=] => BinOp::GreaterEqualTest, - T![<] => BinOp::LesserTest, - T![>] => BinOp::GreaterTest, - T![+] => BinOp::Addition, - T![*] => BinOp::Multiplication, - T![-] => BinOp::Subtraction, - T![/] => BinOp::Division, - T![%] => BinOp::Remainder, - T![<<] => BinOp::LeftShift, - T![>>] => BinOp::RightShift, - T![^] => BinOp::BitwiseXor, - T![|] => BinOp::BitwiseOr, - T![&] => BinOp::BitwiseAnd, - T![=] => BinOp::Assignment, - T![+=] => BinOp::AddAssign, - T![/=] => BinOp::DivAssign, - T![*=] => BinOp::MulAssign, - T![%=] => BinOp::RemAssign, - T![>>=] => BinOp::ShrAssign, - T![<<=] => BinOp::ShlAssign, - T![-=] => BinOp::SubAssign, - T![|=] => BinOp::BitOrAssign, - T![&=] => BinOp::BitAndAssign, - T![^=] => BinOp::BitXorAssign, + T![||] => BinaryOp::LogicOp(LogicOp::Or), + T![&&] => BinaryOp::LogicOp(LogicOp::And), + + T![==] => BinaryOp::CmpOp(CmpOp::Eq { negated: false }), + T![!=] => BinaryOp::CmpOp(CmpOp::Eq { negated: true }), + T![<=] => BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: false }), + T![>=] => BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: false }), + T![<] => BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: true }), + T![>] => BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: true }), + + T![+] => BinaryOp::ArithOp(ArithOp::Add), + T![*] => BinaryOp::ArithOp(ArithOp::Mul), + T![-] => BinaryOp::ArithOp(ArithOp::Sub), + T![/] => BinaryOp::ArithOp(ArithOp::Div), + T![%] => BinaryOp::ArithOp(ArithOp::Rem), + T![<<] => BinaryOp::ArithOp(ArithOp::Shl), + T![>>] => BinaryOp::ArithOp(ArithOp::Shr), + T![^] => BinaryOp::ArithOp(ArithOp::BitXor), + T![|] => BinaryOp::ArithOp(ArithOp::BitOr), + T![&] => BinaryOp::ArithOp(ArithOp::BitAnd), + + T![=] => BinaryOp::Assignment { op: None }, + T![+=] => BinaryOp::Assignment { op: Some(ArithOp::Add) }, + T![*=] => BinaryOp::Assignment { op: Some(ArithOp::Mul) }, + T![-=] => BinaryOp::Assignment { op: Some(ArithOp::Sub) }, + T![/=] => BinaryOp::Assignment { op: Some(ArithOp::Div) }, + T![%=] => BinaryOp::Assignment { op: Some(ArithOp::Rem) }, + T![<<=] => BinaryOp::Assignment { op: Some(ArithOp::Shl) }, + T![>>=] => BinaryOp::Assignment { op: Some(ArithOp::Shr) }, + T![^=] => BinaryOp::Assignment { op: Some(ArithOp::BitXor) }, + T![|=] => BinaryOp::Assignment { op: Some(ArithOp::BitOr) }, + T![&=] => BinaryOp::Assignment { op: Some(ArithOp::BitAnd) }, + _ => return None, }; Some((c, bin_op)) }) } - pub fn op_kind(&self) -> Option { + pub fn op_kind(&self) -> Option { self.op_details().map(|t| t.1) } @@ -357,42 +281,6 @@ pub fn sub_exprs(&self) -> (Option, Option) { } } -impl std::fmt::Display for BinOp { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - BinOp::BooleanOr => write!(f, "||"), - BinOp::BooleanAnd => write!(f, "&&"), - BinOp::EqualityTest => write!(f, "=="), - BinOp::NegatedEqualityTest => write!(f, "!="), - BinOp::LesserEqualTest => write!(f, "<="), - BinOp::GreaterEqualTest => write!(f, ">="), - BinOp::LesserTest => write!(f, "<"), - BinOp::GreaterTest => write!(f, ">"), - BinOp::Addition => write!(f, "+"), - BinOp::Multiplication => write!(f, "*"), - BinOp::Subtraction => write!(f, "-"), - BinOp::Division => write!(f, "/"), - BinOp::Remainder => write!(f, "%"), - BinOp::LeftShift => write!(f, "<<"), - BinOp::RightShift => write!(f, ">>"), - BinOp::BitwiseXor => write!(f, "^"), - BinOp::BitwiseOr => write!(f, "|"), - BinOp::BitwiseAnd => write!(f, "&"), - BinOp::Assignment => write!(f, "="), - BinOp::AddAssign => write!(f, "+="), - BinOp::DivAssign => write!(f, "/="), - BinOp::MulAssign => write!(f, "*="), - BinOp::RemAssign => write!(f, "%="), - BinOp::ShrAssign => write!(f, ">>="), - BinOp::ShlAssign => write!(f, "<<="), - BinOp::SubAssign => write!(f, "-"), - BinOp::BitOrAssign => write!(f, "|="), - BinOp::BitAndAssign => write!(f, "&="), - BinOp::BitXorAssign => write!(f, "^="), - } - } -} - impl ast::RangeExpr { fn op_details(&self) -> Option<(usize, SyntaxToken, RangeOp)> { self.syntax().children_with_tokens().enumerate().find_map(|(ix, child)| { diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index e6fab72ac05..5494dd1d3e8 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -276,7 +276,8 @@ pub fn expr_path(path: ast::Path) -> ast::Expr { pub fn expr_continue() -> ast::Expr { expr_from_text("continue") } -pub fn expr_op(op: ast::BinOp, lhs: ast::Expr, rhs: ast::Expr) -> ast::Expr { +// Consider `op: SyntaxKind` instead for nicer syntax at the call-site? +pub fn expr_bin_op(lhs: ast::Expr, op: ast::BinaryOp, rhs: ast::Expr) -> ast::Expr { expr_from_text(&format!("{} {} {}", lhs, op, rhs)) } pub fn expr_break(expr: Option) -> ast::Expr { diff --git a/crates/syntax/src/ast/operators.rs b/crates/syntax/src/ast/operators.rs index 84cb7c0b012..a687ba0b77a 100644 --- a/crates/syntax/src/ast/operators.rs +++ b/crates/syntax/src/ast/operators.rs @@ -1,3 +1,9 @@ +//! Defines a bunch of data-less enums for unary and binary operators. +//! +//! Types here don't know about AST, this allows re-using them for both AST and +//! HIR. +use std::fmt; + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum RangeOp { /// `..` @@ -8,11 +14,11 @@ pub enum RangeOp { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum UnaryOp { - /// The `*` operator for dereferencing + /// `*` Deref, - /// The `!` operator for logical inversion + /// `!` Not, - /// The `-` operator for negation + /// `-` Neg, } @@ -56,47 +62,61 @@ pub enum ArithOp { BitAnd, } -use crate::ast; -impl From for BinaryOp { - fn from(ast_op: ast::BinOp) -> Self { - match ast_op { - ast::BinOp::BooleanOr => BinaryOp::LogicOp(LogicOp::Or), - ast::BinOp::BooleanAnd => BinaryOp::LogicOp(LogicOp::And), - ast::BinOp::EqualityTest => BinaryOp::CmpOp(CmpOp::Eq { negated: false }), - ast::BinOp::NegatedEqualityTest => BinaryOp::CmpOp(CmpOp::Eq { negated: true }), - ast::BinOp::LesserEqualTest => { - BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: false }) +impl fmt::Display for LogicOp { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let res = match self { + LogicOp::And => "&&", + LogicOp::Or => "||", + }; + f.write_str(res) + } +} + +impl fmt::Display for ArithOp { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let res = match self { + ArithOp::Add => "+", + ArithOp::Mul => "*", + ArithOp::Sub => "-", + ArithOp::Div => "/", + ArithOp::Rem => "%", + ArithOp::Shl => "<<", + ArithOp::Shr => ">>", + ArithOp::BitXor => "^", + ArithOp::BitOr => "|", + ArithOp::BitAnd => "&", + }; + f.write_str(res) + } +} + +impl fmt::Display for CmpOp { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let res = match self { + CmpOp::Eq { negated: false } => "==", + CmpOp::Eq { negated: true } => "!=", + CmpOp::Ord { ordering: Ordering::Less, strict: false } => "<=", + CmpOp::Ord { ordering: Ordering::Less, strict: true } => "<", + CmpOp::Ord { ordering: Ordering::Greater, strict: false } => ">=", + CmpOp::Ord { ordering: Ordering::Greater, strict: true } => ">", + }; + f.write_str(res) + } +} + +impl fmt::Display for BinaryOp { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + BinaryOp::LogicOp(op) => fmt::Display::fmt(op, f), + BinaryOp::ArithOp(op) => fmt::Display::fmt(op, f), + BinaryOp::CmpOp(op) => fmt::Display::fmt(op, f), + BinaryOp::Assignment { op } => { + f.write_str("=")?; + if let Some(op) = op { + fmt::Display::fmt(op, f)?; + } + Ok(()) } - ast::BinOp::GreaterEqualTest => { - BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: false }) - } - ast::BinOp::LesserTest => { - BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: true }) - } - ast::BinOp::GreaterTest => { - BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: true }) - } - ast::BinOp::Addition => BinaryOp::ArithOp(ArithOp::Add), - ast::BinOp::Multiplication => BinaryOp::ArithOp(ArithOp::Mul), - ast::BinOp::Subtraction => BinaryOp::ArithOp(ArithOp::Sub), - ast::BinOp::Division => BinaryOp::ArithOp(ArithOp::Div), - ast::BinOp::Remainder => BinaryOp::ArithOp(ArithOp::Rem), - ast::BinOp::LeftShift => BinaryOp::ArithOp(ArithOp::Shl), - ast::BinOp::RightShift => BinaryOp::ArithOp(ArithOp::Shr), - ast::BinOp::BitwiseXor => BinaryOp::ArithOp(ArithOp::BitXor), - ast::BinOp::BitwiseOr => BinaryOp::ArithOp(ArithOp::BitOr), - ast::BinOp::BitwiseAnd => BinaryOp::ArithOp(ArithOp::BitAnd), - ast::BinOp::Assignment => BinaryOp::Assignment { op: None }, - ast::BinOp::AddAssign => BinaryOp::Assignment { op: Some(ArithOp::Add) }, - ast::BinOp::DivAssign => BinaryOp::Assignment { op: Some(ArithOp::Div) }, - ast::BinOp::MulAssign => BinaryOp::Assignment { op: Some(ArithOp::Mul) }, - ast::BinOp::RemAssign => BinaryOp::Assignment { op: Some(ArithOp::Rem) }, - ast::BinOp::ShlAssign => BinaryOp::Assignment { op: Some(ArithOp::Shl) }, - ast::BinOp::ShrAssign => BinaryOp::Assignment { op: Some(ArithOp::Shr) }, - ast::BinOp::SubAssign => BinaryOp::Assignment { op: Some(ArithOp::Sub) }, - ast::BinOp::BitOrAssign => BinaryOp::Assignment { op: Some(ArithOp::BitOr) }, - ast::BinOp::BitAndAssign => BinaryOp::Assignment { op: Some(ArithOp::BitAnd) }, - ast::BinOp::BitXorAssign => BinaryOp::Assignment { op: Some(ArithOp::BitXor) }, } } }