Repository: rust-analyzer/rust-analyzer
Pull request #9127 (2021-06-03 12:27:43)

internal: make variant fields inherit the enum's visibility in the ItemTree
No observable changes from what I can tell, but this is "more correct".

bors r+


Patch:
diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs
index 6208facd54..0e467d9071 100644
--- a/crates/hir_def/src/item_tree/lower.rs
+++ b/crates/hir_def/src/item_tree/lower.rs
@@ -276,10 +276,11 @@ impl<'a> Ctx<'a> {
         let visibility = self.lower_visibility(enum_);
         let name = enum_.name()?.as_name();
         let generic_params = self.lower_generic_params(GenericsOwner::Enum, enum_);
-        let variants = match &enum_.variant_list() {
-            Some(variant_list) => self.lower_variants(variant_list),
-            None => IdRange::new(self.next_variant_idx()..self.next_variant_idx()),
-        };
+        let variants =
+            self.with_inherited_visibility(visibility, |this| match &enum_.variant_list() {
+                Some(variant_list) => this.lower_variants(variant_list),
+                None => IdRange::new(this.next_variant_idx()..this.next_variant_idx()),
+            });
         let ast_id = self.source_ast_id_map.ast_id(enum_);
         let res = Enum { name, visibility, generic_params, variants, ast_id };
         Some(id(self.data().enums.alloc(res)))
diff --git a/crates/mbe/src/benchmark.rs b/crates/mbe/src/benchmark.rs
index 18eb97f0d5..38707ffa5b 100644
--- a/crates/mbe/src/benchmark.rs
+++ b/crates/mbe/src/benchmark.rs
@@ -187,7 +187,7 @@ fn invocation_fixtures(rules: &FxHashMap<String, MacroRules>) -> Vec<(String, tt
             let a = 1664525;
             let c = 1013904223;
             *seed = usize::wrapping_add(usize::wrapping_mul(*seed, a), c);
-            *seed
+            return *seed;
         }
         fn make_ident(ident: &str) -> tt::TokenTree {
             tt::Leaf::Ident(tt::Ident { id: tt::TokenId::unspecified(), text: SmolStr::new(ident) })
diff --git a/crates/mbe/src/expander/matcher.rs b/crates/mbe/src/expander/matcher.rs
index c982eb58f1..84ca3ff876 100644
--- a/crates/mbe/src/expander/matcher.rs
+++ b/crates/mbe/src/expander/matcher.rs
@@ -219,7 +219,7 @@ impl BindingsBuilder {
         bindings
     }
 
-    fn build_inner(&self, bindings: &mut Bindings, link_nodes: &[LinkNode<Rc<BindingKind>>]) {
+    fn build_inner(&self, bindings: &mut Bindings, link_nodes: &Vec<LinkNode<Rc<BindingKind>>>) {
         let mut nodes = Vec::new();
         self.collect_nodes(&link_nodes, &mut nodes);
 
@@ -301,7 +301,7 @@ impl BindingsBuilder {
 
     fn collect_nodes<'a>(
         &'a self,
-        link_nodes: &'a [LinkNode<Rc<BindingKind>>],
+        link_nodes: &'a Vec<LinkNode<Rc<BindingKind>>>,
         nodes: &mut Vec<&'a Rc<BindingKind>>,
     ) {
         link_nodes.iter().for_each(|it| match it {
@@ -494,8 +494,15 @@ fn match_loop_inner<'t>(
                         }
                         Some(err) => {
                             res.add_err(err);
-                            if let Some(fragment) = match_res.value {
-                                bindings_builder.push_fragment(&mut item.bindings, &name, fragment);
+                            match match_res.value {
+                                Some(fragment) => {
+                                    bindings_builder.push_fragment(
+                                        &mut item.bindings,
+                                        &name,
+                                        fragment,
+                                    );
+                                }
+                                _ => {}
                             }
                             item.is_error = true;
                             error_items.push(item);
@@ -571,9 +578,9 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match {
         );
         stdx::always!(cur_items.is_empty());
 
-        if !error_items.is_empty() {
+        if error_items.len() > 0 {
             error_recover_item = error_items.pop().map(|it| it.bindings);
-        } else if !eof_items.is_empty() {
+        } else if eof_items.len() > 0 {
             error_recover_item = Some(eof_items[0].bindings.clone());
         }
 
@@ -786,7 +793,7 @@ impl<'a> TtIter<'a> {
             _ => (),
         }
 
-        let tt = self.next().ok_or(())?.clone();
+        let tt = self.next().ok_or_else(|| ())?.clone();
         let punct = match tt {
             tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) if punct.spacing == tt::Spacing::Joint => {
                 punct
diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs
index 380a507443..b95374b76d 100644
--- a/crates/mbe/src/lib.rs
+++ b/crates/mbe/src/lib.rs
@@ -295,8 +295,8 @@ fn validate(pattern: &MetaTemplate) -> Result<(), ParseError> {
                 // Checks that no repetition which could match an empty token
                 // https://github.com/rust-lang/rust/blob/a58b1ed44f5e06976de2bdc4d7dc81c36a96934f/src/librustc_expand/mbe/macro_rules.rs#L558
 
-                if separator.is_none()
-                    && subtree.iter().all(|child_op| {
+                if separator.is_none() {
+                    if subtree.iter().all(|child_op| {
                         match child_op {
                             Op::Var { kind, .. } => {
                                 // vis is optional
@@ -314,9 +314,9 @@ fn validate(pattern: &MetaTemplate) -> Result<(), ParseError> {
                             Op::Subtree { .. } => {}
                         }
                         false
-                    })
-                {
-                    return Err(ParseError::RepetitionEmptyTokenTree);
+                    }) {
+                        return Err(ParseError::RepetitionEmptyTokenTree);
+                    }
                 }
                 validate(subtree)?
             }
diff --git a/crates/mbe/src/parser.rs b/crates/mbe/src/parser.rs
index 04c0d3e754..61b2a49551 100644
--- a/crates/mbe/src/parser.rs
+++ b/crates/mbe/src/parser.rs
@@ -213,7 +213,7 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul
     Ok(res)
 }
 
-fn eat_fragment_kind(src: &mut TtIter<'_>, mode: Mode) -> Result<Option<SmolStr>, ParseError> {
+fn eat_fragment_kind<'a>(src: &mut TtIter<'a>, mode: Mode) -> Result<Option<SmolStr>, ParseError> {
     if let Mode::Pattern = mode {
         src.expect_char(':').map_err(|()| err!("bad fragment specifier 1"))?;
         let ident = src.expect_ident().map_err(|()| err!("bad fragment specifier 1"))?;
diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs
index 978c75747a..b11172caf0 100644
--- a/crates/mbe/src/syntax_bridge.rs
+++ b/crates/mbe/src/syntax_bridge.rs
@@ -243,7 +243,8 @@ trait TokenConvertor {
     type Token: SrcToken;
 
     fn go(&mut self) -> tt::Subtree {
-        let mut subtree = tt::Subtree { delimiter: None, ..Default::default() };
+        let mut subtree = tt::Subtree::default();
+        subtree.delimiter = None;
         while self.peek().is_some() {
             self.collect_leaf(&mut subtree.token_trees);
         }
@@ -505,7 +506,7 @@ impl TokenConvertor for Convertor {
 
     fn peek(&self) -> Option<Self::Token> {
         if let Some((punct, mut offset)) = self.punct_offset.clone() {
-            offset += TextSize::of('.');
+            offset = offset + TextSize::of('.');
             if usize::from(offset) < punct.text().len() {
                 return Some(SynToken::Punch(punct, offset));
             }
diff --git a/crates/mbe/src/tt_iter.rs b/crates/mbe/src/tt_iter.rs
index bd54f2442b..99a8d250b7 100644
--- a/crates/mbe/src/tt_iter.rs
+++ b/crates/mbe/src/tt_iter.rs
@@ -138,7 +138,7 @@ impl<'a> TtIter<'a> {
             }
         }
         self.inner = self.inner.as_slice()[res.len()..].iter();
-        if res.is_empty() && err.is_none() {
+        if res.len() == 0 && err.is_none() {
             err = Some(err!("no tokens consumed"));
         }
         let res = match res.len() {
diff --git a/crates/parser/src/grammar/expressions/atom.rs b/crates/parser/src/grammar/expressions/atom.rs
index abdfca1feb..269f223e6e 100644
--- a/crates/parser/src/grammar/expressions/atom.rs
+++ b/crates/parser/src/grammar/expressions/atom.rs
@@ -252,10 +252,12 @@ fn closure_expr(p: &mut Parser) -> CompletedMarker {
         // test lambda_ret_block
         // fn main() { || -> i32 { 92 }(); }
         block_expr(p);
-    } else if p.at_ts(EXPR_FIRST) {
-        expr(p);
     } else {
-        p.error("expected expression");
+        if p.at_ts(EXPR_FIRST) {
+            expr(p);
+        } else {
+            p.error("expected expression");
+        }
     }
     m.complete(p, CLOSURE_EXPR)
 }
diff --git a/crates/syntax/src/parsing/reparsing.rs b/crates/syntax/src/parsing/reparsing.rs
index 304f47b3dc..4ad50ab729 100644
--- a/crates/syntax/src/parsing/reparsing.rs
+++ b/crates/syntax/src/parsing/reparsing.rs
@@ -36,8 +36,8 @@ pub(crate) fn incremental_reparse(
     None
 }
 
-fn reparse_token(
-    root: &SyntaxNode,
+fn reparse_token<'node>(
+    root: &'node SyntaxNode,
     edit: &Indel,
 ) -> Option<(GreenNode, Vec<SyntaxError>, TextRange)> {
     let prev_token = root.covering_element(edit.delete).as_token()?.clone();
@@ -84,8 +84,8 @@ fn reparse_token(
     }
 }
 
-fn reparse_block(
-    root: &SyntaxNode,
+fn reparse_block<'node>(
+    root: &'node SyntaxNode,
     edit: &Indel,
 ) -> Option<(GreenNode, Vec<SyntaxError>, TextRange)> {
     let (node, reparser) = find_reparsable_node(root, edit.delete)?;
diff --git a/xtask/src/codegen/gen_syntax.rs b/xtask/src/codegen/gen_syntax.rs
index b0b9e30db2..ba4b248489 100644
--- a/xtask/src/codegen/gen_syntax.rs
+++ b/xtask/src/codegen/gen_syntax.rs
@@ -94,16 +94,18 @@ fn generate_nodes(kinds: KindsSrc<'_>, grammar: &AstSrc) -> Result<String> {
                             support::children(&self.syntax)
                         }
                     }
-                } else if let Some(token_kind) = field.token_kind() {
-                    quote! {
-                        pub fn #method_name(&self) -> Option<#ty> {
-                            support::token(&self.syntax, #token_kind)
-                        }
-                    }
                 } else {
-                    quote! {
-                        pub fn #method_name(&self) -> Option<#ty> {
-                            support::child(&self.syntax)
+                    if let Some(token_kind) = field.token_kind() {
+                        quote! {
+                            pub fn #method_name(&self) -> Option<#ty> {
+                                support::token(&self.syntax, #token_kind)
+                            }
+                        }
+                    } else {
+                        quote! {
+                            pub fn #method_name(&self) -> Option<#ty> {
+                                support::child(&self.syntax)
+                            }
                         }
                     }
                 }
diff --git a/xtask/src/release/changelog.rs b/xtask/src/release/changelog.rs
index 2384a746f3..ffcae2cf72 100644
--- a/xtask/src/release/changelog.rs
+++ b/xtask/src/release/changelog.rs
@@ -132,7 +132,7 @@ fn parse_changelog_line(s: &str) -> Option<PrInfo> {
             return Some(PrInfo { kind, message: Some(message) });
         }
     };
-    let res = PrInfo { message, kind };
+    let res = PrInfo { kind, message };
     Some(res)
 }
 
@@ -152,7 +152,7 @@ fn parse_title_line(s: &str) -> PrInfo {
                 PrKind::Skip => None,
                 _ => Some(s[prefix.len()..].to_string()),
             };
-            return PrInfo { message, kind };
+            return PrInfo { kind, message };
         }
     }
     PrInfo { kind: PrKind::Other, message: Some(s.to_string()) }
diff --git a/xtask/src/tidy.rs b/xtask/src/tidy.rs
index 618cf12fb1..6f687a7883 100644
--- a/xtask/src/tidy.rs
+++ b/xtask/src/tidy.rs
@@ -33,7 +33,7 @@ fn check_code_formatting() {
     let _e = pushenv("RUSTUP_TOOLCHAIN", "stable");
     crate::ensure_rustfmt().unwrap();
     let res = cmd!("cargo fmt -- --check").run();
-    if res.is_err() {
+    if !res.is_ok() {
         let _ = cmd!("cargo fmt").run();
     }
     res.unwrap()
@@ -244,19 +244,19 @@ Zlib OR Apache-2.0 OR MIT
         .map(|it| it.trim())
         .map(|it| it[r#""license":"#.len()..].trim_matches('"'))
         .collect::<Vec<_>>();
-    licenses.sort_unstable();
+    licenses.sort();
     licenses.dedup();
     if licenses != expected {
         let mut diff = String::new();
 
-        diff.push_str("New Licenses:\n");
+        diff += &format!("New Licenses:\n");
         for &l in licenses.iter() {
             if !expected.contains(&l) {
                 diff += &format!("  {}\n", l)
             }
         }
 
-        diff.push_str("\nMissing Licenses:\n");
+        diff += &format!("\nMissing Licenses:\n");
         for &l in expected.iter() {
             if !licenses.contains(&l) {
                 diff += &format!("  {}\n", l)


Test patch:
diff --git a/crates/hir_def/src/item_tree/tests.rs b/crates/hir_def/src/item_tree/tests.rs
index b362add5c9..57686dc6e7 100644
--- a/crates/hir_def/src/item_tree/tests.rs
+++ b/crates/hir_def/src/item_tree/tests.rs
@@ -359,3 +359,41 @@ trait Tr<'a, T: 'a>: Super {}
         "#]],
     )
 }
+
+#[test]
+fn inherit_visibility() {
+    check(
+        r#"
+pub(crate) enum En {
+    Var1(u8),
+    Var2 {
+        fld: u8,
+    },
+}
+
+pub(crate) trait Tr {
+    fn f();
+    fn method(&self) {}
+}
+        "#,
+        expect![[r#"
+            pub(crate) enum En {
+                Var1(
+                    pub(crate) 0: u8,
+                ),
+                Var2 {
+                    pub(crate) fld: u8,
+                },
+            }
+
+            pub(crate) trait Tr<Self> {
+                pub(crate) fn f() -> ();
+
+                // flags = 0x3
+                pub(crate) fn method(
+                    _: &Self,
+                ) -> ();
+            }
+        "#]],
+    )
+}
diff --git a/crates/syntax/src/tests.rs b/crates/syntax/src/tests.rs
index 9f24261717..45f3c800fc 100644
--- a/crates/syntax/src/tests.rs
+++ b/crates/syntax/src/tests.rs
@@ -236,7 +236,7 @@ where
         }
     });
     dir_tests(&test_data_dir(), err_paths, "rast", |text, path| {
-        if f(text).is_ok() {
+        if let Ok(_) = f(text) {
             panic!("'{:?}' successfully parsed when it should have errored", path);
         } else {
             "ERROR\n".to_owned()
diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs
index ac5a9509d8..bd017567ca 100644
--- a/crates/test_utils/src/lib.rs
+++ b/crates/test_utils/src/lib.rs
@@ -238,9 +238,14 @@ fn extract_line_annotations(mut line: &str) -> Vec<LineAnnotation> {
     let mut res = Vec::new();
     let mut offset: TextSize = 0.into();
     let marker: fn(char) -> bool = if line.contains('^') { |c| c == '^' } else { |c| c == '|' };
-    while let Some(idx) = line.find(marker) {
-        offset += TextSize::try_from(idx).unwrap();
-        line = &line[idx..];
+    loop {
+        match line.find(marker) {
+            Some(idx) => {
+                offset += TextSize::try_from(idx).unwrap();
+                line = &line[idx..];
+            }
+            None => break,
+        };
 
         let mut len = line.chars().take_while(|&it| it == '^').count();
         let mut continuation = false;