Skip to content

Commit bd2219b

Browse files
bors[bot]matklad
bors[bot]
and
matklad
authored
Merge #9890
9890: internal: refactor binary operator handling r=matklad a=matklad bors r+ 🤖 Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
2 parents 01b589b + 90357a9 commit bd2219b

File tree

19 files changed

+265
-413
lines changed

19 files changed

+265
-413
lines changed

‎crates/hir_def/src/body/lower.rs

Lines changed: 3 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@ use crate::{
2727
builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint},
2828
db::DefDatabase,
2929
expr::{
30-
dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Label,
31-
LabelId, Literal, LogicOp, MatchArm, MatchGuard, Ordering, Pat, PatId, RecordFieldPat,
32-
RecordLitField, Statement,
30+
dummy_expr_id, Array, BindingAnnotation, Expr, ExprId, Label, LabelId, Literal, MatchArm,
31+
MatchGuard, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
3332
},
3433
intern::Interned,
3534
item_scope::BuiltinShadowMode,
@@ -509,7 +508,7 @@ impl ExprCollector<'_> {
509508
ast::Expr::BinExpr(e) => {
510509
let lhs = self.collect_expr_opt(e.lhs());
511510
let rhs = self.collect_expr_opt(e.rhs());
512-
let op = e.op_kind().map(BinaryOp::from);
511+
let op = e.op_kind();
513512
self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr)
514513
}
515514
ast::Expr::TupleExpr(e) => {
@@ -954,50 +953,6 @@ impl ExprCollector<'_> {
954953
}
955954
}
956955

957-
impl From<ast::BinOp> for BinaryOp {
958-
fn from(ast_op: ast::BinOp) -> Self {
959-
match ast_op {
960-
ast::BinOp::BooleanOr => BinaryOp::LogicOp(LogicOp::Or),
961-
ast::BinOp::BooleanAnd => BinaryOp::LogicOp(LogicOp::And),
962-
ast::BinOp::EqualityTest => BinaryOp::CmpOp(CmpOp::Eq { negated: false }),
963-
ast::BinOp::NegatedEqualityTest => BinaryOp::CmpOp(CmpOp::Eq { negated: true }),
964-
ast::BinOp::LesserEqualTest => {
965-
BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: false })
966-
}
967-
ast::BinOp::GreaterEqualTest => {
968-
BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: false })
969-
}
970-
ast::BinOp::LesserTest => {
971-
BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Less, strict: true })
972-
}
973-
ast::BinOp::GreaterTest => {
974-
BinaryOp::CmpOp(CmpOp::Ord { ordering: Ordering::Greater, strict: true })
975-
}
976-
ast::BinOp::Addition => BinaryOp::ArithOp(ArithOp::Add),
977-
ast::BinOp::Multiplication => BinaryOp::ArithOp(ArithOp::Mul),
978-
ast::BinOp::Subtraction => BinaryOp::ArithOp(ArithOp::Sub),
979-
ast::BinOp::Division => BinaryOp::ArithOp(ArithOp::Div),
980-
ast::BinOp::Remainder => BinaryOp::ArithOp(ArithOp::Rem),
981-
ast::BinOp::LeftShift => BinaryOp::ArithOp(ArithOp::Shl),
982-
ast::BinOp::RightShift => BinaryOp::ArithOp(ArithOp::Shr),
983-
ast::BinOp::BitwiseXor => BinaryOp::ArithOp(ArithOp::BitXor),
984-
ast::BinOp::BitwiseOr => BinaryOp::ArithOp(ArithOp::BitOr),
985-
ast::BinOp::BitwiseAnd => BinaryOp::ArithOp(ArithOp::BitAnd),
986-
ast::BinOp::Assignment => BinaryOp::Assignment { op: None },
987-
ast::BinOp::AddAssign => BinaryOp::Assignment { op: Some(ArithOp::Add) },
988-
ast::BinOp::DivAssign => BinaryOp::Assignment { op: Some(ArithOp::Div) },
989-
ast::BinOp::MulAssign => BinaryOp::Assignment { op: Some(ArithOp::Mul) },
990-
ast::BinOp::RemAssign => BinaryOp::Assignment { op: Some(ArithOp::Rem) },
991-
ast::BinOp::ShlAssign => BinaryOp::Assignment { op: Some(ArithOp::Shl) },
992-
ast::BinOp::ShrAssign => BinaryOp::Assignment { op: Some(ArithOp::Shr) },
993-
ast::BinOp::SubAssign => BinaryOp::Assignment { op: Some(ArithOp::Sub) },
994-
ast::BinOp::BitOrAssign => BinaryOp::Assignment { op: Some(ArithOp::BitOr) },
995-
ast::BinOp::BitAndAssign => BinaryOp::Assignment { op: Some(ArithOp::BitAnd) },
996-
ast::BinOp::BitXorAssign => BinaryOp::Assignment { op: Some(ArithOp::BitXor) },
997-
}
998-
}
999-
}
1000-
1001956
impl From<ast::LiteralKind> for Literal {
1002957
fn from(ast_lit_kind: ast::LiteralKind) -> Self {
1003958
match ast_lit_kind {

‎crates/hir_def/src/expr.rs

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
1515
use hir_expand::name::Name;
1616
use la_arena::{Idx, RawIdx};
17-
use syntax::ast::RangeOp;
1817

1918
use crate::{
2019
builtin_type::{BuiltinFloat, BuiltinInt, BuiltinUint},
@@ -24,6 +23,8 @@ use crate::{
2423
BlockId,
2524
};
2625

26+
pub use syntax::ast::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering, RangeOp, UnaryOp};
27+
2728
pub type ExprId = Idx<Expr>;
2829
pub(crate) fn dummy_expr_id() -> ExprId {
2930
ExprId::from_raw(RawIdx::from(!0))
@@ -179,47 +180,6 @@ pub enum Expr {
179180
Literal(Literal),
180181
}
181182

182-
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
183-
pub enum BinaryOp {
184-
LogicOp(LogicOp),
185-
ArithOp(ArithOp),
186-
CmpOp(CmpOp),
187-
Assignment { op: Option<ArithOp> },
188-
}
189-
190-
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
191-
pub enum LogicOp {
192-
And,
193-
Or,
194-
}
195-
196-
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
197-
pub enum CmpOp {
198-
Eq { negated: bool },
199-
Ord { ordering: Ordering, strict: bool },
200-
}
201-
202-
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
203-
pub enum Ordering {
204-
Less,
205-
Greater,
206-
}
207-
208-
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
209-
pub enum ArithOp {
210-
Add,
211-
Mul,
212-
Sub,
213-
Div,
214-
Rem,
215-
Shl,
216-
Shr,
217-
BitXor,
218-
BitOr,
219-
BitAnd,
220-
}
221-
222-
pub use syntax::ast::PrefixOp as UnaryOp;
223183
#[derive(Debug, Clone, Eq, PartialEq)]
224184
pub enum Array {
225185
ElementList(Vec<ExprId>),

‎crates/ide/src/syntax_highlighting/highlight.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ pub(super) fn element(
126126
let ty = sema.type_of_expr(&expr)?.original;
127127
if ty.is_raw_ptr() {
128128
HlTag::Operator(HlOperator::Other) | HlMod::Unsafe
129-
} else if let Some(ast::PrefixOp::Deref) = prefix_expr.op_kind() {
129+
} else if let Some(ast::UnaryOp::Deref) = prefix_expr.op_kind() {
130130
HlOperator::Other.into()
131131
} else {
132132
HlPunct::Other.into()

‎crates/ide_assists/src/handlers/apply_demorgan.rs

Lines changed: 17 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,20 @@ use crate::{utils::invert_boolean_expression, AssistContext, AssistId, AssistKin
1919
// ->
2020
// ```
2121
// fn main() {
22-
// if !(x == 4 && !(y < 3.14)) {}
22+
// if !(x == 4 && y >= 3.14) {}
2323
// }
2424
// ```
2525
pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
2626
let expr = ctx.find_node_at_offset::<ast::BinExpr>()?;
2727
let op = expr.op_kind()?;
2828
let op_range = expr.op_token()?.text_range();
29-
let opposite_op = opposite_logic_op(op)?;
29+
30+
let opposite_op = match op {
31+
ast::BinaryOp::LogicOp(ast::LogicOp::And) => "||",
32+
ast::BinaryOp::LogicOp(ast::LogicOp::Or) => "&&",
33+
_ => return None,
34+
};
35+
3036
let cursor_in_range = op_range.contains_range(ctx.frange.range);
3137
if !cursor_in_range {
3238
return None;
@@ -85,7 +91,7 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<(
8591
.and_then(|paren_expr| paren_expr.syntax().parent())
8692
.and_then(ast::PrefixExpr::cast)
8793
.and_then(|prefix_expr| {
88-
if prefix_expr.op_kind().unwrap() == ast::PrefixOp::Not {
94+
if prefix_expr.op_kind().unwrap() == ast::UnaryOp::Not {
8995
Some(prefix_expr)
9096
} else {
9197
None
@@ -99,7 +105,7 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<(
99105
if let Some(paren_expr) = paren_expr {
100106
for term in terms {
101107
let range = term.syntax().text_range();
102-
let not_term = invert_boolean_expression(&ctx.sema, term);
108+
let not_term = invert_boolean_expression(term);
103109

104110
edit.replace(range, not_term.syntax().text());
105111
}
@@ -114,37 +120,28 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<(
114120
} else {
115121
if let Some(lhs) = terms.pop_front() {
116122
let lhs_range = lhs.syntax().text_range();
117-
let not_lhs = invert_boolean_expression(&ctx.sema, lhs);
123+
let not_lhs = invert_boolean_expression(lhs);
118124

119125
edit.replace(lhs_range, format!("!({}", not_lhs.syntax().text()));
120126
}
121127

122128
if let Some(rhs) = terms.pop_back() {
123129
let rhs_range = rhs.syntax().text_range();
124-
let not_rhs = invert_boolean_expression(&ctx.sema, rhs);
130+
let not_rhs = invert_boolean_expression(rhs);
125131

126132
edit.replace(rhs_range, format!("{})", not_rhs.syntax().text()));
127133
}
128134

129135
for term in terms {
130136
let term_range = term.syntax().text_range();
131-
let not_term = invert_boolean_expression(&ctx.sema, term);
137+
let not_term = invert_boolean_expression(term);
132138
edit.replace(term_range, not_term.syntax().text());
133139
}
134140
}
135141
},
136142
)
137143
}
138144

139-
// Return the opposite text for a given logical operator, if it makes sense
140-
fn opposite_logic_op(kind: ast::BinOp) -> Option<&'static str> {
141-
match kind {
142-
ast::BinOp::BooleanOr => Some("&&"),
143-
ast::BinOp::BooleanAnd => Some("||"),
144-
_ => None,
145-
}
146-
}
147-
148145
#[cfg(test)]
149146
mod tests {
150147
use crate::tests::{check_assist, check_assist_not_applicable};
@@ -156,40 +153,12 @@ mod tests {
156153
check_assist(
157154
apply_demorgan,
158155
r#"
159-
//- minicore: ord, derive
160-
#[derive(PartialEq, Eq, PartialOrd, Ord)]
161-
struct S;
162-
163-
fn f() {
164-
S < S &&$0 S <= S
165-
}
166-
"#,
167-
r#"
168-
#[derive(PartialEq, Eq, PartialOrd, Ord)]
169-
struct S;
170-
171-
fn f() {
172-
!(S >= S || S > S)
173-
}
174-
"#,
175-
);
176-
177-
check_assist(
178-
apply_demorgan,
179-
r#"
180-
//- minicore: ord, derive
181156
struct S;
182-
183-
fn f() {
184-
S < S &&$0 S <= S
185-
}
157+
fn f() { S < S &&$0 S <= S }
186158
"#,
187159
r#"
188160
struct S;
189-
190-
fn f() {
191-
!(!(S < S) || !(S <= S))
192-
}
161+
fn f() { !(S >= S || S > S) }
193162
"#,
194163
);
195164
}
@@ -199,39 +168,12 @@ fn f() {
199168
check_assist(
200169
apply_demorgan,
201170
r#"
202-
//- minicore: ord, derive
203-
#[derive(PartialEq, Eq, PartialOrd, Ord)]
204-
struct S;
205-
206-
fn f() {
207-
S > S &&$0 S >= S
208-
}
209-
"#,
210-
r#"
211-
#[derive(PartialEq, Eq, PartialOrd, Ord)]
212-
struct S;
213-
214-
fn f() {
215-
!(S <= S || S < S)
216-
}
217-
"#,
218-
);
219-
check_assist(
220-
apply_demorgan,
221-
r#"
222-
//- minicore: ord, derive
223171
struct S;
224-
225-
fn f() {
226-
S > S &&$0 S >= S
227-
}
172+
fn f() { S > S &&$0 S >= S }
228173
"#,
229174
r#"
230175
struct S;
231-
232-
fn f() {
233-
!(!(S > S) || !(S >= S))
234-
}
176+
fn f() { !(S <= S || S < S) }
235177
"#,
236178
);
237179
}

‎crates/ide_assists/src/handlers/convert_bool_then.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ pub(crate) fn convert_if_to_bool_then(acc: &mut Assists, ctx: &AssistContext) ->
9797
e => e,
9898
};
9999

100-
let cond = if invert_cond { invert_boolean_expression(&ctx.sema, cond) } else { cond };
100+
let cond = if invert_cond { invert_boolean_expression(cond) } else { cond };
101101
let arg_list = make::arg_list(Some(make::expr_closure(None, closure_body)));
102102
let mcall = make::expr_method_call(cond, make::name_ref("then"), arg_list);
103103
builder.replace(target, mcall.to_string());

‎crates/ide_assists/src/handlers/early_return.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext)
115115
let new_expr = {
116116
let then_branch =
117117
make::block_expr(once(make::expr_stmt(early_expression).into()), None);
118-
let cond = invert_boolean_expression(&ctx.sema, cond_expr);
118+
let cond = invert_boolean_expression(cond_expr);
119119
make::expr_if(make::condition(cond, None), then_branch, None)
120120
.indent(if_indent_level)
121121
};

‎crates/ide_assists/src/handlers/extract_function.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ fn expr_require_exclusive_access(ctx: &AssistContext, expr: &ast::Expr) -> Optio
892892
let parent = expr.syntax().parent()?;
893893

894894
if let Some(bin_expr) = ast::BinExpr::cast(parent.clone()) {
895-
if bin_expr.op_kind()?.is_assignment() {
895+
if matches!(bin_expr.op_kind()?, ast::BinaryOp::Assignment { .. }) {
896896
return Some(bin_expr.lhs()?.syntax() == expr.syntax());
897897
}
898898
return Some(false);

‎crates/ide_assists/src/handlers/flip_binexpr.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use syntax::ast::{AstNode, BinExpr, BinOp};
1+
use syntax::ast::{self, AstNode, BinExpr};
22

33
use crate::{AssistContext, AssistId, AssistKind, Assists};
44

@@ -56,14 +56,19 @@ enum FlipAction {
5656
DontFlip,
5757
}
5858

59-
impl From<BinOp> for FlipAction {
60-
fn from(op_kind: BinOp) -> Self {
59+
impl From<ast::BinaryOp> for FlipAction {
60+
fn from(op_kind: ast::BinaryOp) -> Self {
6161
match op_kind {
62-
kind if kind.is_assignment() => FlipAction::DontFlip,
63-
BinOp::GreaterTest => FlipAction::FlipAndReplaceOp("<"),
64-
BinOp::GreaterEqualTest => FlipAction::FlipAndReplaceOp("<="),
65-
BinOp::LesserTest => FlipAction::FlipAndReplaceOp(">"),
66-
BinOp::LesserEqualTest => FlipAction::FlipAndReplaceOp(">="),
62+
ast::BinaryOp::Assignment { .. } => FlipAction::DontFlip,
63+
ast::BinaryOp::CmpOp(ast::CmpOp::Ord { ordering, strict }) => {
64+
let rev_op = match (ordering, strict) {
65+
(ast::Ordering::Less, true) => ">",
66+
(ast::Ordering::Less, false) => ">=",
67+
(ast::Ordering::Greater, true) => "<",
68+
(ast::Ordering::Greater, false) => "<=",
69+
};
70+
FlipAction::FlipAndReplaceOp(rev_op)
71+
}
6772
_ => FlipAction::Flip,
6873
}
6974
}

‎crates/ide_assists/src/handlers/invert_if.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub(crate) fn invert_if(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
4747
};
4848

4949
acc.add(AssistId("invert_if", AssistKind::RefactorRewrite), "Invert if", if_range, |edit| {
50-
let flip_cond = invert_boolean_expression(&ctx.sema, cond.clone());
50+
let flip_cond = invert_boolean_expression(cond.clone());
5151
edit.replace_ast(cond, flip_cond);
5252

5353
let else_node = else_block.syntax();

0 commit comments

Comments
 (0)