From a6f17f74368ffdf917f47647096f34a815a6337e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 25 Sep 2021 13:27:53 +0300 Subject: [PATCH 1/3] minor: more readable code --- crates/parser/src/grammar/expressions.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/crates/parser/src/grammar/expressions.rs b/crates/parser/src/grammar/expressions.rs index 29310b71bd8..a627ca70d9e 100644 --- a/crates/parser/src/grammar/expressions.rs +++ b/crates/parser/src/grammar/expressions.rs @@ -1,8 +1,9 @@ mod atom; +use super::*; + pub(crate) use self::atom::{block_expr, match_arm_list}; pub(super) use self::atom::{literal, LITERAL_FIRST}; -use super::*; pub(super) enum StmtWithSemi { Yes, @@ -47,11 +48,6 @@ fn expr_no_struct(p: &mut Parser) { expr_bp(p, r, 1); } -fn is_expr_stmt_attr_allowed(kind: SyntaxKind) -> bool { - let forbid = matches!(kind, BIN_EXPR | RANGE_EXPR); - !forbid -} - pub(super) fn stmt(p: &mut Parser, with_semi: StmtWithSemi, prefer_expr: bool) { let m = p.start(); // test attr_on_expr_stmt @@ -79,13 +75,15 @@ pub(super) fn stmt(p: &mut Parser, with_semi: StmtWithSemi, prefer_expr: bool) { let (cm, blocklike) = expr_stmt(p); let kind = cm.as_ref().map(|cm| cm.kind()).unwrap_or(ERROR); - if has_attrs && !is_expr_stmt_attr_allowed(kind) { - // test_err attr_on_expr_not_allowed - // fn foo() { - // #[A] 1 + 2; - // #[B] if true {}; - // } - p.error(format!("attributes are not allowed on {:?}", kind)); + if has_attrs { + if matches!(kind, BIN_EXPR | RANGE_EXPR) { + // test_err attr_on_expr_not_allowed + // fn foo() { + // #[A] 1 + 2; + // #[B] if true {}; + // } + p.error(format!("attributes are not allowed on {:?}", kind)); + } } if p.at(T!['}']) || (prefer_expr && p.at(EOF)) { From 1567bbb73e48d897a55ec8281ab6191550fd6684 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 25 Sep 2021 14:02:33 +0300 Subject: [PATCH 2/3] minor: more focusted tests --- crates/parser/src/grammar/expressions.rs | 35 ++-- .../parser/inline/ok/0130_let_stmt.rast | 158 ++++-------------- .../parser/inline/ok/0130_let_stmt.rs | 10 +- .../parser/inline/ok/0193_let_stmt_init.rast | 28 ++++ .../parser/inline/ok/0193_let_stmt_init.rs | 1 + .../inline/ok/0194_let_stmt_ascription.rast | 30 ++++ .../inline/ok/0194_let_stmt_ascription.rs | 1 + 7 files changed, 107 insertions(+), 156 deletions(-) create mode 100644 crates/syntax/test_data/parser/inline/ok/0193_let_stmt_init.rast create mode 100644 crates/syntax/test_data/parser/inline/ok/0193_let_stmt_init.rs create mode 100644 crates/syntax/test_data/parser/inline/ok/0194_let_stmt_ascription.rast create mode 100644 crates/syntax/test_data/parser/inline/ok/0194_let_stmt_ascription.rs diff --git a/crates/parser/src/grammar/expressions.rs b/crates/parser/src/grammar/expressions.rs index a627ca70d9e..9f3e11819f9 100644 --- a/crates/parser/src/grammar/expressions.rs +++ b/crates/parser/src/grammar/expressions.rs @@ -115,6 +115,10 @@ pub(super) fn stmt(p: &mut Parser, with_semi: StmtWithSemi, prefer_expr: bool) { // } match with_semi { + StmtWithSemi::No => (), + StmtWithSemi::Optional => { + p.eat(T![;]); + } StmtWithSemi::Yes => { if blocklike.is_block() { p.eat(T![;]); @@ -122,48 +126,35 @@ pub(super) fn stmt(p: &mut Parser, with_semi: StmtWithSemi, prefer_expr: bool) { p.expect(T![;]); } } - StmtWithSemi::No => {} - StmtWithSemi::Optional => { - if p.at(T![;]) { - p.eat(T![;]); - } - } } m.complete(p, EXPR_STMT); } // test let_stmt - // fn foo() { - // let a; - // let b: i32; - // let c = 92; - // let d: i32 = 92; - // let e: !; - // let _: ! = {}; - // let f = #[attr]||{}; - // } + // fn f() { let x: i32 = 92; } fn let_stmt(p: &mut Parser, m: Marker, with_semi: StmtWithSemi) { - assert!(p.at(T![let])); p.bump(T![let]); patterns::pattern(p); if p.at(T![:]) { + // test let_stmt_ascription + // fn f() { let x: i32; } types::ascription(p); } if p.eat(T![=]) { + // test let_stmt_init + // fn f() { let x = 92; } expressions::expr_with_attrs(p); } match with_semi { + StmtWithSemi::No => (), + StmtWithSemi::Optional => { + p.eat(T![;]); + } StmtWithSemi::Yes => { p.expect(T![;]); } - StmtWithSemi::No => {} - StmtWithSemi::Optional => { - if p.at(T![;]) { - p.eat(T![;]); - } - } } m.complete(p, LET_STMT); } diff --git a/crates/syntax/test_data/parser/inline/ok/0130_let_stmt.rast b/crates/syntax/test_data/parser/inline/ok/0130_let_stmt.rast index c3a79836aa3..af3b11376b3 100644 --- a/crates/syntax/test_data/parser/inline/ok/0130_let_stmt.rast +++ b/crates/syntax/test_data/parser/inline/ok/0130_let_stmt.rast @@ -1,127 +1,35 @@ -SOURCE_FILE@0..135 - FN@0..134 +SOURCE_FILE@0..28 + FN@0..27 FN_KW@0..2 "fn" WHITESPACE@2..3 " " - NAME@3..6 - IDENT@3..6 "foo" - PARAM_LIST@6..8 - L_PAREN@6..7 "(" - R_PAREN@7..8 ")" - WHITESPACE@8..9 " " - BLOCK_EXPR@9..134 - L_CURLY@9..10 "{" - WHITESPACE@10..15 "\n " - LET_STMT@15..21 - LET_KW@15..18 "let" - WHITESPACE@18..19 " " - IDENT_PAT@19..20 - NAME@19..20 - IDENT@19..20 "a" - SEMICOLON@20..21 ";" - WHITESPACE@21..26 "\n " - LET_STMT@26..37 - LET_KW@26..29 "let" - WHITESPACE@29..30 " " - IDENT_PAT@30..31 - NAME@30..31 - IDENT@30..31 "b" - COLON@31..32 ":" - WHITESPACE@32..33 " " - PATH_TYPE@33..36 - PATH@33..36 - PATH_SEGMENT@33..36 - NAME_REF@33..36 - IDENT@33..36 "i32" - SEMICOLON@36..37 ";" - WHITESPACE@37..42 "\n " - LET_STMT@42..53 - LET_KW@42..45 "let" - WHITESPACE@45..46 " " - IDENT_PAT@46..47 - NAME@46..47 - IDENT@46..47 "c" - WHITESPACE@47..48 " " - EQ@48..49 "=" - WHITESPACE@49..50 " " - LITERAL@50..52 - INT_NUMBER@50..52 "92" - SEMICOLON@52..53 ";" - WHITESPACE@53..58 "\n " - LET_STMT@58..74 - LET_KW@58..61 "let" - WHITESPACE@61..62 " " - IDENT_PAT@62..63 - NAME@62..63 - IDENT@62..63 "d" - COLON@63..64 ":" - WHITESPACE@64..65 " " - PATH_TYPE@65..68 - PATH@65..68 - PATH_SEGMENT@65..68 - NAME_REF@65..68 - IDENT@65..68 "i32" - WHITESPACE@68..69 " " - EQ@69..70 "=" - WHITESPACE@70..71 " " - LITERAL@71..73 - INT_NUMBER@71..73 "92" - SEMICOLON@73..74 ";" - WHITESPACE@74..79 "\n " - LET_STMT@79..88 - LET_KW@79..82 "let" - WHITESPACE@82..83 " " - IDENT_PAT@83..84 - NAME@83..84 - IDENT@83..84 "e" - COLON@84..85 ":" - WHITESPACE@85..86 " " - NEVER_TYPE@86..87 - BANG@86..87 "!" - SEMICOLON@87..88 ";" - WHITESPACE@88..93 "\n " - LET_STMT@93..107 - LET_KW@93..96 "let" - WHITESPACE@96..97 " " - WILDCARD_PAT@97..98 - UNDERSCORE@97..98 "_" - COLON@98..99 ":" - WHITESPACE@99..100 " " - NEVER_TYPE@100..101 - BANG@100..101 "!" - WHITESPACE@101..102 " " - EQ@102..103 "=" - WHITESPACE@103..104 " " - BLOCK_EXPR@104..106 - L_CURLY@104..105 "{" - R_CURLY@105..106 "}" - SEMICOLON@106..107 ";" - WHITESPACE@107..112 "\n " - LET_STMT@112..132 - LET_KW@112..115 "let" - WHITESPACE@115..116 " " - IDENT_PAT@116..117 - NAME@116..117 - IDENT@116..117 "f" - WHITESPACE@117..118 " " - EQ@118..119 "=" - WHITESPACE@119..120 " " - CLOSURE_EXPR@120..131 - ATTR@120..127 - POUND@120..121 "#" - L_BRACK@121..122 "[" - META@122..126 - PATH@122..126 - PATH_SEGMENT@122..126 - NAME_REF@122..126 - IDENT@122..126 "attr" - R_BRACK@126..127 "]" - PARAM_LIST@127..129 - PIPE@127..128 "|" - PIPE@128..129 "|" - BLOCK_EXPR@129..131 - L_CURLY@129..130 "{" - R_CURLY@130..131 "}" - SEMICOLON@131..132 ";" - WHITESPACE@132..133 "\n" - R_CURLY@133..134 "}" - WHITESPACE@134..135 "\n" + NAME@3..4 + IDENT@3..4 "f" + PARAM_LIST@4..6 + L_PAREN@4..5 "(" + R_PAREN@5..6 ")" + WHITESPACE@6..7 " " + BLOCK_EXPR@7..27 + L_CURLY@7..8 "{" + WHITESPACE@8..9 " " + LET_STMT@9..25 + LET_KW@9..12 "let" + WHITESPACE@12..13 " " + IDENT_PAT@13..14 + NAME@13..14 + IDENT@13..14 "x" + COLON@14..15 ":" + WHITESPACE@15..16 " " + PATH_TYPE@16..19 + PATH@16..19 + PATH_SEGMENT@16..19 + NAME_REF@16..19 + IDENT@16..19 "i32" + WHITESPACE@19..20 " " + EQ@20..21 "=" + WHITESPACE@21..22 " " + LITERAL@22..24 + INT_NUMBER@22..24 "92" + SEMICOLON@24..25 ";" + WHITESPACE@25..26 " " + R_CURLY@26..27 "}" + WHITESPACE@27..28 "\n" diff --git a/crates/syntax/test_data/parser/inline/ok/0130_let_stmt.rs b/crates/syntax/test_data/parser/inline/ok/0130_let_stmt.rs index fa8ee49a231..8003999fd08 100644 --- a/crates/syntax/test_data/parser/inline/ok/0130_let_stmt.rs +++ b/crates/syntax/test_data/parser/inline/ok/0130_let_stmt.rs @@ -1,9 +1 @@ -fn foo() { - let a; - let b: i32; - let c = 92; - let d: i32 = 92; - let e: !; - let _: ! = {}; - let f = #[attr]||{}; -} +fn f() { let x: i32 = 92; } diff --git a/crates/syntax/test_data/parser/inline/ok/0193_let_stmt_init.rast b/crates/syntax/test_data/parser/inline/ok/0193_let_stmt_init.rast new file mode 100644 index 00000000000..cc5d72ff74c --- /dev/null +++ b/crates/syntax/test_data/parser/inline/ok/0193_let_stmt_init.rast @@ -0,0 +1,28 @@ +SOURCE_FILE@0..23 + FN@0..22 + FN_KW@0..2 "fn" + WHITESPACE@2..3 " " + NAME@3..4 + IDENT@3..4 "f" + PARAM_LIST@4..6 + L_PAREN@4..5 "(" + R_PAREN@5..6 ")" + WHITESPACE@6..7 " " + BLOCK_EXPR@7..22 + L_CURLY@7..8 "{" + WHITESPACE@8..9 " " + LET_STMT@9..20 + LET_KW@9..12 "let" + WHITESPACE@12..13 " " + IDENT_PAT@13..14 + NAME@13..14 + IDENT@13..14 "x" + WHITESPACE@14..15 " " + EQ@15..16 "=" + WHITESPACE@16..17 " " + LITERAL@17..19 + INT_NUMBER@17..19 "92" + SEMICOLON@19..20 ";" + WHITESPACE@20..21 " " + R_CURLY@21..22 "}" + WHITESPACE@22..23 "\n" diff --git a/crates/syntax/test_data/parser/inline/ok/0193_let_stmt_init.rs b/crates/syntax/test_data/parser/inline/ok/0193_let_stmt_init.rs new file mode 100644 index 00000000000..232c0db411e --- /dev/null +++ b/crates/syntax/test_data/parser/inline/ok/0193_let_stmt_init.rs @@ -0,0 +1 @@ +fn f() { let x = 92; } diff --git a/crates/syntax/test_data/parser/inline/ok/0194_let_stmt_ascription.rast b/crates/syntax/test_data/parser/inline/ok/0194_let_stmt_ascription.rast new file mode 100644 index 00000000000..41acb0dd983 --- /dev/null +++ b/crates/syntax/test_data/parser/inline/ok/0194_let_stmt_ascription.rast @@ -0,0 +1,30 @@ +SOURCE_FILE@0..23 + FN@0..22 + FN_KW@0..2 "fn" + WHITESPACE@2..3 " " + NAME@3..4 + IDENT@3..4 "f" + PARAM_LIST@4..6 + L_PAREN@4..5 "(" + R_PAREN@5..6 ")" + WHITESPACE@6..7 " " + BLOCK_EXPR@7..22 + L_CURLY@7..8 "{" + WHITESPACE@8..9 " " + LET_STMT@9..20 + LET_KW@9..12 "let" + WHITESPACE@12..13 " " + IDENT_PAT@13..14 + NAME@13..14 + IDENT@13..14 "x" + COLON@14..15 ":" + WHITESPACE@15..16 " " + PATH_TYPE@16..19 + PATH@16..19 + PATH_SEGMENT@16..19 + NAME_REF@16..19 + IDENT@16..19 "i32" + SEMICOLON@19..20 ";" + WHITESPACE@20..21 " " + R_CURLY@21..22 "}" + WHITESPACE@22..23 "\n" diff --git a/crates/syntax/test_data/parser/inline/ok/0194_let_stmt_ascription.rs b/crates/syntax/test_data/parser/inline/ok/0194_let_stmt_ascription.rs new file mode 100644 index 00000000000..a94161dffa2 --- /dev/null +++ b/crates/syntax/test_data/parser/inline/ok/0194_let_stmt_ascription.rs @@ -0,0 +1 @@ +fn f() { let x: i32; } From d72f7cf3af4ec652ae65cbd896993036a703a124 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 25 Sep 2021 14:10:25 +0300 Subject: [PATCH 3/3] internal: add => () rule; emphasize `n_items` rule --- .../src/utils/gen_trait_fn_body.rs | 6 ++--- docs/dev/style.md | 22 ++++++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) 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 b9c7da71b5c..eb4a23a8da3 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -439,10 +439,10 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { let eq_check = make::expr_bin_op(lhs, BinaryOp::CmpOp(CmpOp::Eq { negated: false }), rhs); - let mut case_count = 0; + let mut n_cases = 0; let mut arms = vec![]; for variant in enum_.variant_list()?.variants() { - case_count += 1; + n_cases += 1; match variant.field_list() { // => (Self::Bar { bin: l_bin }, Self::Bar { bin: r_bin }) => l_bin == r_bin, Some(ast::FieldList::RecordFieldList(list)) => { @@ -517,7 +517,7 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { let expr = match arms.len() { 0 => eq_check, _ => { - if case_count > arms.len() { + if n_cases > arms.len() { let lhs = make::wildcard_pat().into(); arms.push(make::match_arm(Some(lhs), None, eq_check)); } diff --git a/docs/dev/style.md b/docs/dev/style.md index 92e79508b6d..e11005c5604 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -849,7 +849,7 @@ Default names: * `res` -- "result of the function" local variable * `it` -- I don't really care about the name -* `n_foo` -- number of foos +* `n_foos` -- number of foos (prefer this to `foo_count`) * `foo_idx` -- index of `foo` Many names in rust-analyzer conflict with keywords. @@ -969,6 +969,26 @@ Don't use the `ref` keyword. Today, it is redundant. Between `ref` and mach ergonomics, the latter is more ergonomic in most cases, and is simpler (does not require a keyword). +## Empty Match Arms + +Ues `=> (),` when a match arm is intentionally empty: + +```rust +// GOOD +match result { + Ok(_) => (), + Err(err) => error!("{}", err), +} + +// BAD +match result { + Ok(_) => {} + Err(err) => error!("{}", err), +} +``` + +**Rationale:** consistency. + ## Functional Combinators Use high order monadic combinators like `map`, `then` when they are a natural choice; don't bend the code to fit into some combinator.