From bb45aca9093ffe61cf444d538b3de737117c9a64 Mon Sep 17 00:00:00 2001 From: Leander Tentrup Date: Thu, 2 Apr 2020 14:34:51 +0200 Subject: Flatten nested highlight ranges during DFS traversal --- crates/ra_ide/src/syntax_highlighting.rs | 55 +++++++++++++++++++++++--- crates/ra_ide/src/syntax_highlighting/tests.rs | 16 ++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) (limited to 'crates') diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs index 7fc94d3cd..eb1c54639 100644 --- a/crates/ra_ide/src/syntax_highlighting.rs +++ b/crates/ra_ide/src/syntax_highlighting.rs @@ -24,7 +24,7 @@ use crate::{call_info::call_info_for_token, Analysis, FileId}; pub(crate) use html::highlight_as_html; pub use tags::{Highlight, HighlightModifier, HighlightModifiers, HighlightTag}; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct HighlightedRange { pub range: TextRange, pub highlight: Highlight, @@ -55,13 +55,55 @@ pub(crate) fn highlight( }; let mut bindings_shadow_count: FxHashMap = FxHashMap::default(); - let mut res = Vec::new(); + // We use a stack for the DFS traversal below. + // When we leave a node, the we use it to flatten the highlighted ranges. + let mut res: Vec> = vec![Vec::new()]; let mut current_macro_call: Option = None; // Walk all nodes, keeping track of whether we are inside a macro or not. // If in macro, expand it first and highlight the expanded code. for event in root.preorder_with_tokens() { + match &event { + WalkEvent::Enter(_) => res.push(Vec::new()), + WalkEvent::Leave(_) => { + /* Flattens the highlighted ranges. + * + * For example `#[cfg(feature = "foo")]` contains the nested ranges: + * 1) parent-range: Attribute [0, 23) + * 2) child-range: String [16, 21) + * + * The following code implements the flattening, for our example this results to: + * `[Attribute [0, 16), String [16, 21), Attribute [21, 23)]` + */ + let children = res.pop().unwrap(); + let prev = res.last_mut().unwrap(); + let needs_flattening = !children.is_empty() + && !prev.is_empty() + && children.first().unwrap().range.is_subrange(&prev.last().unwrap().range); + if !needs_flattening { + prev.extend(children); + } else { + let mut parent = prev.pop().unwrap(); + for ele in children { + assert!(ele.range.is_subrange(&parent.range)); + let mut cloned = parent.clone(); + parent.range = TextRange::from_to(parent.range.start(), ele.range.start()); + cloned.range = TextRange::from_to(ele.range.end(), cloned.range.end()); + if !parent.range.is_empty() { + prev.push(parent); + } + prev.push(ele); + parent = cloned; + } + if !parent.range.is_empty() { + prev.push(parent); + } + } + } + }; + let current = res.last_mut().expect("during DFS traversal, the stack must not be empty"); + let event_range = match &event { WalkEvent::Enter(it) => it.text_range(), WalkEvent::Leave(it) => it.text_range(), @@ -77,7 +119,7 @@ pub(crate) fn highlight( WalkEvent::Enter(Some(mc)) => { current_macro_call = Some(mc.clone()); if let Some(range) = macro_call_range(&mc) { - res.push(HighlightedRange { + current.push(HighlightedRange { range, highlight: HighlightTag::Macro.into(), binding_hash: None, @@ -119,7 +161,7 @@ pub(crate) fn highlight( if let Some(token) = element.as_token().cloned().and_then(ast::RawString::cast) { let expanded = element_to_highlight.as_token().unwrap().clone(); - if highlight_injection(&mut res, &sema, token, expanded).is_some() { + if highlight_injection(current, &sema, token, expanded).is_some() { continue; } } @@ -127,11 +169,12 @@ pub(crate) fn highlight( if let Some((highlight, binding_hash)) = highlight_element(&sema, &mut bindings_shadow_count, element_to_highlight) { - res.push(HighlightedRange { range, highlight, binding_hash }); + current.push(HighlightedRange { range, highlight, binding_hash }); } } - res + assert_eq!(res.len(), 1, "after DFS traversal, the stack should only contain a single element"); + res.pop().unwrap() } fn macro_call_range(macro_call: &ast::MacroCall) -> Option { diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs index 98c030791..7f442bd0f 100644 --- a/crates/ra_ide/src/syntax_highlighting/tests.rs +++ b/crates/ra_ide/src/syntax_highlighting/tests.rs @@ -131,3 +131,19 @@ fn test_ranges() { assert_eq!(&highlights[0].highlight.to_string(), "field.declaration"); } + +#[test] +fn test_flattening() { + let (analysis, file_id) = single_file(r#"#[cfg(feature = "foo")]"#); + + let highlights = analysis.highlight(file_id).unwrap(); + + // The source code snippet contains 2 nested highlights: + // 1) Attribute spanning the whole string + // 2) The string "foo" + // The resulting flattening splits the attribute range: + assert_eq!(highlights.len(), 3); + assert_eq!(&highlights[0].highlight.to_string(), "attribute"); + assert_eq!(&highlights[1].highlight.to_string(), "string_literal"); + assert_eq!(&highlights[2].highlight.to_string(), "attribute"); +} -- cgit v1.2.3 From bf96d46fee1242ad701cb037a03c9575e84221b1 Mon Sep 17 00:00:00 2001 From: Leander Tentrup Date: Mon, 6 Apr 2020 23:00:09 +0200 Subject: Simplify HTML highlighter and add test case for highlight_injection logic --- .../ra_ide/src/snapshots/highlight_injection.html | 39 +++++++++++++ crates/ra_ide/src/snapshots/highlighting.html | 10 ++-- crates/ra_ide/src/syntax_highlighting.rs | 8 ++- crates/ra_ide/src/syntax_highlighting/html.rs | 66 ++++++++-------------- crates/ra_ide/src/syntax_highlighting/tests.rs | 33 +++++++---- 5 files changed, 97 insertions(+), 59 deletions(-) create mode 100644 crates/ra_ide/src/snapshots/highlight_injection.html (limited to 'crates') diff --git a/crates/ra_ide/src/snapshots/highlight_injection.html b/crates/ra_ide/src/snapshots/highlight_injection.html new file mode 100644 index 000000000..6ec13bd80 --- /dev/null +++ b/crates/ra_ide/src/snapshots/highlight_injection.html @@ -0,0 +1,39 @@ + + +
fn fixture(ra_fixture: &str) {}
+
+fn main() {
+    fixture(r#"
+        trait Foo {
+            fn foo() {
+                println!("2 + 2 = {}", 4);
+            }
+        }"#
+    );
+}
\ No newline at end of file diff --git a/crates/ra_ide/src/snapshots/highlighting.html b/crates/ra_ide/src/snapshots/highlighting.html index 495b07f69..214dcbb62 100644 --- a/crates/ra_ide/src/snapshots/highlighting.html +++ b/crates/ra_ide/src/snapshots/highlighting.html @@ -26,7 +26,7 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd .keyword.unsafe { color: #BC8383; font-weight: bold; } .control { font-style: italic; } -
#[derive(Clone, Debug)]
+
#[derive(Clone, Debug)]
 struct Foo {
     pub x: i32,
     pub y: i32,
@@ -36,11 +36,11 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
     foo::<'a, i32>()
 }
 
-macro_rules! def_fn {
+macro_rules! def_fn {
     ($($tt:tt)*) => {$($tt)*}
 }
 
-def_fn! {
+def_fn! {
     fn bar() -> u32 {
         100
     }
@@ -48,7 +48,7 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 
 // comment
 fn main() {
-    println!("Hello, {}!", 92);
+    println!("Hello, {}!", 92);
 
     let mut vec = Vec::new();
     if true {
@@ -73,7 +73,7 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 impl<T> Option<T> {
     fn and<U>(self, other: Option<U>) -> Option<(T, U)> {
         match other {
-            None => unimplemented!(),
+            None => unimplemented!(),
             Nope => Nope,
         }
     }
diff --git a/crates/ra_ide/src/syntax_highlighting.rs b/crates/ra_ide/src/syntax_highlighting.rs
index eb1c54639..7d908f987 100644
--- a/crates/ra_ide/src/syntax_highlighting.rs
+++ b/crates/ra_ide/src/syntax_highlighting.rs
@@ -174,7 +174,13 @@ pub(crate) fn highlight(
     }
 
     assert_eq!(res.len(), 1, "after DFS traversal, the stack should only contain a single element");
-    res.pop().unwrap()
+    let res = res.pop().unwrap();
+    // Check that ranges are sorted and disjoint
+    assert!(res
+        .iter()
+        .zip(res.iter().skip(1))
+        .all(|(left, right)| left.range.end() <= right.range.start()));
+    res
 }
 
 fn macro_call_range(macro_call: &ast::MacroCall) -> Option {
diff --git a/crates/ra_ide/src/syntax_highlighting/html.rs b/crates/ra_ide/src/syntax_highlighting/html.rs
index e13766c9d..4496529a1 100644
--- a/crates/ra_ide/src/syntax_highlighting/html.rs
+++ b/crates/ra_ide/src/syntax_highlighting/html.rs
@@ -1,9 +1,9 @@
 //! Renders a bit of code as HTML.
 
 use ra_db::SourceDatabase;
-use ra_syntax::AstNode;
+use ra_syntax::{AstNode, TextUnit};
 
-use crate::{FileId, HighlightedRange, RootDatabase};
+use crate::{FileId, RootDatabase};
 
 use super::highlight;
 
@@ -21,51 +21,35 @@ pub(crate) fn highlight_as_html(db: &RootDatabase, file_id: FileId, rainbow: boo
         )
     }
 
-    let mut ranges = highlight(db, file_id, None);
-    ranges.sort_by_key(|it| it.range.start());
-    // quick non-optimal heuristic to intersect token ranges and highlighted ranges
-    let mut frontier = 0;
-    let mut could_intersect: Vec<&HighlightedRange> = Vec::new();
-
+    let ranges = highlight(db, file_id, None);
+    let text = parse.tree().syntax().to_string();
+    let mut prev_pos = TextUnit::from(0);
     let mut buf = String::new();
     buf.push_str(&STYLE);
     buf.push_str("
");
-    let tokens = parse.tree().syntax().descendants_with_tokens().filter_map(|it| it.into_token());
-    for token in tokens {
-        could_intersect.retain(|it| token.text_range().start() <= it.range.end());
-        while let Some(r) = ranges.get(frontier) {
-            if r.range.start() <= token.text_range().end() {
-                could_intersect.push(r);
-                frontier += 1;
-            } else {
-                break;
-            }
-        }
-        let text = html_escape(&token.text());
-        let ranges = could_intersect
-            .iter()
-            .filter(|it| token.text_range().is_subrange(&it.range))
-            .collect::>();
-        if ranges.is_empty() {
+    for range in &ranges {
+        if range.range.start() > prev_pos {
+            let curr = &text[prev_pos.to_usize()..range.range.start().to_usize()];
+            let text = html_escape(curr);
             buf.push_str(&text);
-        } else {
-            let classes = ranges
-                .iter()
-                .map(|it| it.highlight.to_string().replace('.', " "))
-                .collect::>()
-                .join(" ");
-            let binding_hash = ranges.first().and_then(|x| x.binding_hash);
-            let color = match (rainbow, binding_hash) {
-                (true, Some(hash)) => format!(
-                    " data-binding-hash=\"{}\" style=\"color: {};\"",
-                    hash,
-                    rainbowify(hash)
-                ),
-                _ => "".into(),
-            };
-            buf.push_str(&format!("{}", classes, color, text));
         }
+        let curr = &text[range.range.start().to_usize()..range.range.end().to_usize()];
+
+        let class = range.highlight.to_string().replace('.', " ");
+        let color = match (rainbow, range.binding_hash) {
+            (true, Some(hash)) => {
+                format!(" data-binding-hash=\"{}\" style=\"color: {};\"", hash, rainbowify(hash))
+            }
+            _ => "".into(),
+        };
+        buf.push_str(&format!("{}", class, color, html_escape(curr)));
+
+        prev_pos = range.range.end();
     }
+    // Add the remaining (non-highlighted) text
+    let curr = &text[prev_pos.to_usize()..];
+    let text = html_escape(curr);
+    buf.push_str(&text);
     buf.push_str("
"); buf } diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs index 7f442bd0f..110887c2a 100644 --- a/crates/ra_ide/src/syntax_highlighting/tests.rs +++ b/crates/ra_ide/src/syntax_highlighting/tests.rs @@ -134,16 +134,25 @@ fn test_ranges() { #[test] fn test_flattening() { - let (analysis, file_id) = single_file(r#"#[cfg(feature = "foo")]"#); - - let highlights = analysis.highlight(file_id).unwrap(); - - // The source code snippet contains 2 nested highlights: - // 1) Attribute spanning the whole string - // 2) The string "foo" - // The resulting flattening splits the attribute range: - assert_eq!(highlights.len(), 3); - assert_eq!(&highlights[0].highlight.to_string(), "attribute"); - assert_eq!(&highlights[1].highlight.to_string(), "string_literal"); - assert_eq!(&highlights[2].highlight.to_string(), "attribute"); + let (analysis, file_id) = single_file( + r##" +fn fixture(ra_fixture: &str) {} + +fn main() { + fixture(r#" + trait Foo { + fn foo() { + println!("2 + 2 = {}", 4); + } + }"# + ); +}"## + .trim(), + ); + + let dst_file = project_dir().join("crates/ra_ide/src/snapshots/highlight_injection.html"); + let actual_html = &analysis.highlight_as_html(file_id, false).unwrap(); + let expected_html = &read_text(&dst_file); + fs::write(dst_file, &actual_html).unwrap(); + assert_eq_text!(expected_html, actual_html); } -- cgit v1.2.3 From d8f6013404a88d845cda6793162e7ac24b0ccbf2 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 7 Apr 2020 18:23:18 +0200 Subject: Fix names of test modules --- crates/ra_ide/src/completion/complete_record.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates') diff --git a/crates/ra_ide/src/completion/complete_record.rs b/crates/ra_ide/src/completion/complete_record.rs index 79f5c8c8f..b180e2388 100644 --- a/crates/ra_ide/src/completion/complete_record.rs +++ b/crates/ra_ide/src/completion/complete_record.rs @@ -59,7 +59,7 @@ fn pattern_ascribed_fields(record_pat: &ast::RecordPat) -> Vec { #[cfg(test)] mod tests { - mod record_lit_tests { + mod record_pat_tests { use insta::assert_debug_snapshot; use crate::completion::{test_utils::do_completion, CompletionItem, CompletionKind}; @@ -205,7 +205,7 @@ mod tests { } } - mod record_pat_tests { + mod record_lit_tests { use insta::assert_debug_snapshot; use crate::completion::{test_utils::do_completion, CompletionItem, CompletionKind}; -- cgit v1.2.3 From 7819d99d6bc617ee8653e9dc2fa4d82072d6c594 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 7 Apr 2020 18:25:47 +0200 Subject: Add functional update test --- crates/ra_ide/src/completion/complete_record.rs | 33 +++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'crates') diff --git a/crates/ra_ide/src/completion/complete_record.rs b/crates/ra_ide/src/completion/complete_record.rs index b180e2388..2352ced5f 100644 --- a/crates/ra_ide/src/completion/complete_record.rs +++ b/crates/ra_ide/src/completion/complete_record.rs @@ -410,5 +410,38 @@ mod tests { ] "###); } + + #[test] + fn completes_functional_update() { + let completions = complete( + r" + struct S { + foo1: u32, + foo2: u32, + } + + fn main() { + let foo1 = 1; + let s = S { + foo1, + <|> + .. loop {} + } + } + ", + ); + assert_debug_snapshot!(completions, @r###" + [ + CompletionItem { + label: "foo2", + source_range: [221; 221), + delete: [221; 221), + insert: "foo2", + kind: Field, + detail: "u32", + }, + ] + "###); + } } } -- cgit v1.2.3 From 4c29214bba65d23e18875bd060325c489be5a8e4 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 7 Apr 2020 17:09:02 +0200 Subject: Move computation of missing fields into hir --- crates/ra_hir/src/code_model.rs | 31 ++--- crates/ra_hir/src/semantics.rs | 28 +++-- crates/ra_hir/src/source_analyzer.rs | 90 +++++++++++--- crates/ra_hir_ty/src/expr.rs | 158 ++++++++++++++---------- crates/ra_ide/src/completion/complete_record.rs | 59 ++------- 5 files changed, 198 insertions(+), 168 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index c6f3bdb8e..9baebf643 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -1027,8 +1027,16 @@ impl Type { ty: Ty, ) -> Option { let krate = resolver.krate()?; + Some(Type::new_with_resolver_inner(db, krate, resolver, ty)) + } + pub(crate) fn new_with_resolver_inner( + db: &dyn HirDatabase, + krate: CrateId, + resolver: &Resolver, + ty: Ty, + ) -> Type { let environment = TraitEnvironment::lower(db, &resolver); - Some(Type { krate, ty: InEnvironment { value: ty, environment } }) + Type { krate, ty: InEnvironment { value: ty, environment } } } fn new(db: &dyn HirDatabase, krate: CrateId, lexical_env: impl HasResolver, ty: Ty) -> Type { @@ -1152,27 +1160,6 @@ impl Type { res } - pub fn variant_fields( - &self, - db: &dyn HirDatabase, - def: VariantDef, - ) -> Vec<(StructField, Type)> { - // FIXME: check that ty and def match - match &self.ty.value { - Ty::Apply(a_ty) => { - let field_types = db.field_types(def.into()); - def.fields(db) - .into_iter() - .map(|it| { - let ty = field_types[it.id].clone().subst(&a_ty.parameters); - (it, self.derived(ty)) - }) - .collect() - } - _ => Vec::new(), - } - } - pub fn autoderef<'a>(&'a self, db: &'a dyn HirDatabase) -> impl Iterator + 'a { // There should be no inference vars in types passed here // FIXME check that? diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs index 2ad231d36..2707e422d 100644 --- a/crates/ra_hir/src/semantics.rs +++ b/crates/ra_hir/src/semantics.rs @@ -23,7 +23,7 @@ use crate::{ semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx}, source_analyzer::{resolve_hir_path, SourceAnalyzer}, AssocItem, Function, HirFileId, ImplDef, InFile, Local, MacroDef, Module, ModuleDef, Name, - Origin, Path, ScopeDef, StructField, Trait, Type, TypeParam, VariantDef, + Origin, Path, ScopeDef, StructField, Trait, Type, TypeParam, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -187,14 +187,6 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { self.analyze(field.syntax()).resolve_record_field(self.db, field) } - pub fn resolve_record_literal(&self, record_lit: &ast::RecordLit) -> Option { - self.analyze(record_lit.syntax()).resolve_record_literal(self.db, record_lit) - } - - pub fn resolve_record_pattern(&self, record_pat: &ast::RecordPat) -> Option { - self.analyze(record_pat.syntax()).resolve_record_pattern(record_pat) - } - pub fn resolve_macro_call(&self, macro_call: &ast::MacroCall) -> Option { let sa = self.analyze(macro_call.syntax()); let macro_call = self.find_file(macro_call.syntax().clone()).with_value(macro_call); @@ -212,6 +204,24 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> { // FIXME: use this instead? // pub fn resolve_name_ref(&self, name_ref: &ast::NameRef) -> Option; + pub fn record_literal_missing_fields( + &self, + literal: &ast::RecordLit, + ) -> Vec<(StructField, Type)> { + self.analyze(literal.syntax()) + .record_literal_missing_fields(self.db, literal) + .unwrap_or_default() + } + + pub fn record_pattern_missing_fields( + &self, + pattern: &ast::RecordPat, + ) -> Vec<(StructField, Type)> { + self.analyze(pattern.syntax()) + .record_pattern_missing_fields(self.db, pattern) + .unwrap_or_default() + } + pub fn to_def(&self, src: &T) -> Option { let src = self.find_file(src.syntax().clone()).with_value(src).cloned(); T::to_def(self, src) diff --git a/crates/ra_hir/src/source_analyzer.rs b/crates/ra_hir/src/source_analyzer.rs index 815ca158c..45631f8fd 100644 --- a/crates/ra_hir/src/source_analyzer.rs +++ b/crates/ra_hir/src/source_analyzer.rs @@ -14,10 +14,13 @@ use hir_def::{ }, expr::{ExprId, Pat, PatId}, resolver::{resolver_for_scope, Resolver, TypeNs, ValueNs}, - AsMacroCall, DefWithBodyId, + AsMacroCall, DefWithBodyId, LocalStructFieldId, StructFieldId, VariantId, }; use hir_expand::{hygiene::Hygiene, name::AsName, HirFileId, InFile}; -use hir_ty::InferenceResult; +use hir_ty::{ + expr::{record_literal_missing_fields, record_pattern_missing_fields}, + InferenceResult, Substs, Ty, +}; use ra_syntax::{ ast::{self, AstNode}, SyntaxNode, SyntaxNodePtr, TextUnit, @@ -25,8 +28,10 @@ use ra_syntax::{ use crate::{ db::HirDatabase, semantics::PathResolution, Adt, Const, EnumVariant, Function, Local, MacroDef, - ModPath, ModuleDef, Path, PathKind, Static, Struct, Trait, Type, TypeAlias, TypeParam, + ModPath, ModuleDef, Path, PathKind, Static, Struct, StructField, Trait, Type, TypeAlias, + TypeParam, }; +use ra_db::CrateId; /// `SourceAnalyzer` is a convenience wrapper which exposes HIR API in terms of /// original source files. It should not be used inside the HIR itself. @@ -164,23 +169,6 @@ impl SourceAnalyzer { Some((struct_field.into(), local)) } - pub(crate) fn resolve_record_literal( - &self, - db: &dyn HirDatabase, - record_lit: &ast::RecordLit, - ) -> Option { - let expr_id = self.expr_id(db, &record_lit.clone().into())?; - self.infer.as_ref()?.variant_resolution_for_expr(expr_id).map(|it| it.into()) - } - - pub(crate) fn resolve_record_pattern( - &self, - record_pat: &ast::RecordPat, - ) -> Option { - let pat_id = self.pat_id(&record_pat.clone().into())?; - self.infer.as_ref()?.variant_resolution_for_pat(pat_id).map(|it| it.into()) - } - pub(crate) fn resolve_macro_call( &self, db: &dyn HirDatabase, @@ -231,6 +219,68 @@ impl SourceAnalyzer { resolve_hir_path(db, &self.resolver, &hir_path) } + pub(crate) fn record_literal_missing_fields( + &self, + db: &dyn HirDatabase, + literal: &ast::RecordLit, + ) -> Option> { + let krate = self.resolver.krate()?; + let body = self.body.as_ref()?; + let infer = self.infer.as_ref()?; + + let expr_id = self.expr_id(db, &literal.clone().into())?; + let substs = match &infer.type_of_expr[expr_id] { + Ty::Apply(a_ty) => &a_ty.parameters, + _ => return None, + }; + + let (variant, missing_fields, _exhaustive) = + record_literal_missing_fields(db, infer, expr_id, &body[expr_id])?; + let res = self.missing_fields(db, krate, substs, variant, missing_fields); + Some(res) + } + + pub(crate) fn record_pattern_missing_fields( + &self, + db: &dyn HirDatabase, + pattern: &ast::RecordPat, + ) -> Option> { + let krate = self.resolver.krate()?; + let body = self.body.as_ref()?; + let infer = self.infer.as_ref()?; + + let pat_id = self.pat_id(&pattern.clone().into())?; + let substs = match &infer.type_of_pat[pat_id] { + Ty::Apply(a_ty) => &a_ty.parameters, + _ => return None, + }; + + let (variant, missing_fields) = + record_pattern_missing_fields(db, infer, pat_id, &body[pat_id])?; + let res = self.missing_fields(db, krate, substs, variant, missing_fields); + Some(res) + } + + fn missing_fields( + &self, + db: &dyn HirDatabase, + krate: CrateId, + substs: &Substs, + variant: VariantId, + missing_fields: Vec, + ) -> Vec<(StructField, Type)> { + let field_types = db.field_types(variant); + + missing_fields + .into_iter() + .map(|local_id| { + let field = StructFieldId { parent: variant, local_id }; + let ty = field_types[local_id].clone().subst(substs); + (field.into(), Type::new_with_resolver_inner(db, krate, &self.resolver, ty)) + }) + .collect() + } + pub(crate) fn expand( &self, db: &dyn HirDatabase, diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 1e7395b16..b4592fbf5 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs @@ -2,12 +2,8 @@ use std::sync::Arc; -use hir_def::{ - path::{path, Path}, - resolver::HasResolver, - AdtId, FunctionId, -}; -use hir_expand::{diagnostics::DiagnosticSink, name::Name}; +use hir_def::{path::path, resolver::HasResolver, AdtId, FunctionId}; +use hir_expand::diagnostics::DiagnosticSink; use ra_syntax::ast; use ra_syntax::AstPtr; use rustc_hash::FxHashSet; @@ -29,7 +25,7 @@ pub use hir_def::{ ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp, }, - VariantId, + LocalStructFieldId, VariantId, }; pub struct ExprValidator<'a, 'b: 'a> { @@ -50,14 +46,37 @@ impl<'a, 'b> ExprValidator<'a, 'b> { pub fn validate_body(&mut self, db: &dyn HirDatabase) { let body = db.body(self.func.into()); - for e in body.exprs.iter() { - if let (id, Expr::RecordLit { path, fields, spread }) = e { - self.validate_record_literal(id, path, fields, *spread, db); - } else if let (id, Expr::Match { expr, arms }) = e { + for (id, expr) in body.exprs.iter() { + if let Some((variant_def, missed_fields, true)) = + record_literal_missing_fields(db, &self.infer, id, expr) + { + // XXX: only look at source_map if we do have missing fields + let (_, source_map) = db.body_with_source_map(self.func.into()); + + if let Ok(source_ptr) = source_map.expr_syntax(id) { + if let Some(expr) = source_ptr.value.left() { + let root = source_ptr.file_syntax(db.upcast()); + if let ast::Expr::RecordLit(record_lit) = expr.to_node(&root) { + if let Some(field_list) = record_lit.record_field_list() { + let variant_data = variant_data(db.upcast(), variant_def); + let missed_fields = missed_fields + .into_iter() + .map(|idx| variant_data.fields()[idx].name.clone()) + .collect(); + self.sink.push(MissingFields { + file: source_ptr.file_id, + field_list: AstPtr::new(&field_list), + missed_fields, + }) + } + } + } + } + } + if let Expr::Match { expr, arms } = expr { self.validate_match(id, *expr, arms, db, self.infer.clone()); } } - let body_expr = &body[body.body_expr]; if let Expr::Block { tail: Some(t), .. } = body_expr { self.validate_results_in_tail_expr(body.body_expr, *t, db); @@ -146,61 +165,6 @@ impl<'a, 'b> ExprValidator<'a, 'b> { } } - fn validate_record_literal( - &mut self, - id: ExprId, - _path: &Option, - fields: &[RecordLitField], - spread: Option, - db: &dyn HirDatabase, - ) { - if spread.is_some() { - return; - }; - let variant_def: VariantId = match self.infer.variant_resolution_for_expr(id) { - Some(VariantId::UnionId(_)) | None => return, - Some(it) => it, - }; - if let VariantId::UnionId(_) = variant_def { - return; - } - - let variant_data = variant_data(db.upcast(), variant_def); - - let lit_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect(); - let missed_fields: Vec = variant_data - .fields() - .iter() - .filter_map(|(_f, d)| { - let name = d.name.clone(); - if lit_fields.contains(&name) { - None - } else { - Some(name) - } - }) - .collect(); - if missed_fields.is_empty() { - return; - } - let (_, source_map) = db.body_with_source_map(self.func.into()); - - if let Ok(source_ptr) = source_map.expr_syntax(id) { - if let Some(expr) = source_ptr.value.left() { - let root = source_ptr.file_syntax(db.upcast()); - if let ast::Expr::RecordLit(record_lit) = expr.to_node(&root) { - if let Some(field_list) = record_lit.record_field_list() { - self.sink.push(MissingFields { - file: source_ptr.file_id, - field_list: AstPtr::new(&field_list), - missed_fields, - }) - } - } - } - } - } - fn validate_results_in_tail_expr(&mut self, body_id: ExprId, id: ExprId, db: &dyn HirDatabase) { // the mismatch will be on the whole block currently let mismatch = match self.infer.type_mismatch_for_expr(body_id) { @@ -233,3 +197,63 @@ impl<'a, 'b> ExprValidator<'a, 'b> { } } } + +pub fn record_literal_missing_fields( + db: &dyn HirDatabase, + infer: &InferenceResult, + id: ExprId, + expr: &Expr, +) -> Option<(VariantId, Vec, /*exhaustive*/ bool)> { + let (fields, exhausitve) = match expr { + Expr::RecordLit { path: _, fields, spread } => (fields, spread.is_none()), + _ => return None, + }; + + let variant_def = infer.variant_resolution_for_expr(id)?; + if let VariantId::UnionId(_) = variant_def { + return None; + } + + let variant_data = variant_data(db.upcast(), variant_def); + + let specified_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect(); + let missed_fields: Vec = variant_data + .fields() + .iter() + .filter_map(|(f, d)| if specified_fields.contains(&d.name) { None } else { Some(f) }) + .collect(); + if missed_fields.is_empty() { + return None; + } + Some((variant_def, missed_fields, exhausitve)) +} + +pub fn record_pattern_missing_fields( + db: &dyn HirDatabase, + infer: &InferenceResult, + id: PatId, + pat: &Pat, +) -> Option<(VariantId, Vec)> { + let fields = match pat { + Pat::Record { path: _, args } => args, + _ => return None, + }; + + let variant_def = infer.variant_resolution_for_pat(id)?; + if let VariantId::UnionId(_) = variant_def { + return None; + } + + let variant_data = variant_data(db.upcast(), variant_def); + + let specified_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect(); + let missed_fields: Vec = variant_data + .fields() + .iter() + .filter_map(|(f, d)| if specified_fields.contains(&d.name) { None } else { Some(f) }) + .collect(); + if missed_fields.is_empty() { + return None; + } + Some((variant_def, missed_fields)) +} diff --git a/crates/ra_ide/src/completion/complete_record.rs b/crates/ra_ide/src/completion/complete_record.rs index 2352ced5f..f46bcee5c 100644 --- a/crates/ra_ide/src/completion/complete_record.rs +++ b/crates/ra_ide/src/completion/complete_record.rs @@ -1,60 +1,19 @@ //! Complete fields in record literals and patterns. -use ra_syntax::{ast, ast::NameOwner, SmolStr}; - use crate::completion::{CompletionContext, Completions}; pub(super) fn complete_record(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { - let (ty, variant, already_present_fields) = - match (ctx.record_lit_pat.as_ref(), ctx.record_lit_syntax.as_ref()) { - (None, None) => return None, - (Some(_), Some(_)) => unreachable!("A record cannot be both a literal and a pattern"), - (Some(record_pat), _) => ( - ctx.sema.type_of_pat(&record_pat.clone().into())?, - ctx.sema.resolve_record_pattern(record_pat)?, - pattern_ascribed_fields(record_pat), - ), - (_, Some(record_lit)) => ( - ctx.sema.type_of_expr(&record_lit.clone().into())?, - ctx.sema.resolve_record_literal(record_lit)?, - literal_ascribed_fields(record_lit), - ), - }; + let missing_fields = match (ctx.record_lit_pat.as_ref(), ctx.record_lit_syntax.as_ref()) { + (None, None) => return None, + (Some(_), Some(_)) => unreachable!("A record cannot be both a literal and a pattern"), + (Some(record_pat), _) => ctx.sema.record_pattern_missing_fields(record_pat), + (_, Some(record_lit)) => ctx.sema.record_literal_missing_fields(record_lit), + }; - for (field, field_ty) in ty.variant_fields(ctx.db, variant).into_iter().filter(|(field, _)| { - // FIXME: already_present_names better be `Vec` - !already_present_fields.contains(&SmolStr::from(field.name(ctx.db).to_string())) - }) { - acc.add_field(ctx, field, &field_ty); + for (field, ty) in missing_fields { + acc.add_field(ctx, field, &ty) } - Some(()) -} -fn literal_ascribed_fields(record_lit: &ast::RecordLit) -> Vec { - record_lit - .record_field_list() - .map(|field_list| field_list.fields()) - .map(|fields| { - fields - .into_iter() - .filter_map(|field| field.name_ref()) - .map(|name_ref| name_ref.text().clone()) - .collect() - }) - .unwrap_or_default() -} - -fn pattern_ascribed_fields(record_pat: &ast::RecordPat) -> Vec { - record_pat - .record_field_pat_list() - .map(|pat_list| { - pat_list - .record_field_pats() - .filter_map(|fild_pat| fild_pat.name()) - .chain(pat_list.bind_pats().filter_map(|bind_pat| bind_pat.name())) - .map(|name| name.text().clone()) - .collect() - }) - .unwrap_or_default() + Some(()) } #[cfg(test)] -- cgit v1.2.3 From 9e3c8438475308bffaced3d9842299676037d036 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 8 Apr 2020 12:19:41 +0200 Subject: fmt --- crates/ra_proc_macro_srv/src/proc_macro/diagnostic.rs | 8 +++++--- crates/ra_proc_macro_srv/src/proc_macro/mod.rs | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'crates') diff --git a/crates/ra_proc_macro_srv/src/proc_macro/diagnostic.rs b/crates/ra_proc_macro_srv/src/proc_macro/diagnostic.rs index 9029f8815..55d93917c 100644 --- a/crates/ra_proc_macro_srv/src/proc_macro/diagnostic.rs +++ b/crates/ra_proc_macro_srv/src/proc_macro/diagnostic.rs @@ -54,12 +54,14 @@ pub struct Diagnostic { } macro_rules! diagnostic_child_methods { - ($spanned:ident, $regular:ident, $level:expr) => ( + ($spanned:ident, $regular:ident, $level:expr) => { /// Adds a new child diagnostic message to `self` with the level /// identified by this method's name with the given `spans` and /// `message`. pub fn $spanned(mut self, spans: S, message: T) -> Diagnostic - where S: MultiSpan, T: Into + where + S: MultiSpan, + T: Into, { self.children.push(Diagnostic::spanned(spans, $level, message)); self @@ -71,7 +73,7 @@ macro_rules! diagnostic_child_methods { self.children.push(Diagnostic::new($level, message)); self } - ) + }; } /// Iterator over the children diagnostics of a `Diagnostic`. diff --git a/crates/ra_proc_macro_srv/src/proc_macro/mod.rs b/crates/ra_proc_macro_srv/src/proc_macro/mod.rs index e35a6ff8b..ee0dc9722 100644 --- a/crates/ra_proc_macro_srv/src/proc_macro/mod.rs +++ b/crates/ra_proc_macro_srv/src/proc_macro/mod.rs @@ -169,13 +169,13 @@ pub mod token_stream { pub struct Span(bridge::client::Span); macro_rules! diagnostic_method { - ($name:ident, $level:expr) => ( + ($name:ident, $level:expr) => { /// Creates a new `Diagnostic` with the given `message` at the span /// `self`. pub fn $name>(self, message: T) -> Diagnostic { Diagnostic::spanned(self, $level, message) } - ) + }; } impl Span { -- cgit v1.2.3 From 53d05448c1d44b3ac2a46d54b40c3f2653fa0312 Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Wed, 8 Apr 2020 18:34:20 +0800 Subject: Add L_DOLLAR for TYPE_RECOVERY_SET --- crates/ra_mbe/src/tests.rs | 17 +++++++++++++++++ crates/ra_parser/src/grammar/types.rs | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) (limited to 'crates') diff --git a/crates/ra_mbe/src/tests.rs b/crates/ra_mbe/src/tests.rs index a7fcea0ac..254318e23 100644 --- a/crates/ra_mbe/src/tests.rs +++ b/crates/ra_mbe/src/tests.rs @@ -1614,6 +1614,23 @@ fn test_issue_2520() { ); } +#[test] +fn test_issue_3861() { + let macro_fixture = parse_macro( + r#" + macro_rules! rgb_color { + ($p:expr, $t: ty) => { + pub fn new() { + let _ = 0 as $t << $p; + } + }; + } + "#, + ); + + macro_fixture.expand_items(r#"rgb_color!(8 + 8, u32);"#); +} + #[test] fn test_repeat_bad_var() { // FIXME: the second rule of the macro should be removed and an error about diff --git a/crates/ra_parser/src/grammar/types.rs b/crates/ra_parser/src/grammar/types.rs index 2c00bce80..386969d2d 100644 --- a/crates/ra_parser/src/grammar/types.rs +++ b/crates/ra_parser/src/grammar/types.rs @@ -7,7 +7,7 @@ pub(super) const TYPE_FIRST: TokenSet = paths::PATH_FIRST.union(token_set![ DYN_KW, L_ANGLE, ]); -const TYPE_RECOVERY_SET: TokenSet = token_set![R_PAREN, COMMA]; +const TYPE_RECOVERY_SET: TokenSet = token_set![R_PAREN, COMMA, L_DOLLAR]; pub(crate) fn type_(p: &mut Parser) { type_with_bounds_cond(p, true); -- cgit v1.2.3 From 35a69d09ee8f997c681c77b6e621906e243caeec Mon Sep 17 00:00:00 2001 From: Luca Barbieri Date: Fri, 3 Apr 2020 21:12:07 +0200 Subject: Fix warnings emitted when compiling as part of rustc --- crates/ra_syntax/src/algo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates') diff --git a/crates/ra_syntax/src/algo.rs b/crates/ra_syntax/src/algo.rs index 191123c8e..8d1098036 100644 --- a/crates/ra_syntax/src/algo.rs +++ b/crates/ra_syntax/src/algo.rs @@ -316,7 +316,7 @@ impl<'a> SyntaxRewriter<'a> { } } -impl<'a> ops::AddAssign for SyntaxRewriter<'_> { +impl ops::AddAssign for SyntaxRewriter<'_> { fn add_assign(&mut self, rhs: SyntaxRewriter) { assert!(rhs.f.is_none()); self.replacements.extend(rhs.replacements) -- cgit v1.2.3 From 941615748d9000e66ff4400ae519dc60410a11f7 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Tue, 7 Apr 2020 15:32:14 -0700 Subject: fix panic in match checking when tuple enum missing pattern --- crates/ra_hir_ty/src/_match.rs | 60 +++++++++++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 15 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index f29a25505..037db5cf8 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -235,7 +235,10 @@ impl From for PatIdOrWild { } #[derive(Debug, Clone, Copy, PartialEq)] -pub struct MatchCheckNotImplemented; +pub enum MatchCheckErr { + NotImplemented, + MalformedMatchArm, +} /// The return type of `is_useful` is either an indication of usefulness /// of the match arm, or an error in the case the match statement @@ -244,7 +247,7 @@ pub struct MatchCheckNotImplemented; /// /// The `std::result::Result` type is used here rather than a custom enum /// to allow the use of `?`. -pub type MatchCheckResult = Result; +pub type MatchCheckResult = Result; #[derive(Debug)] /// A row in a Matrix. @@ -335,12 +338,12 @@ impl PatStack { Expr::Literal(Literal::Bool(_)) => None, // perhaps this is actually unreachable given we have // already checked that these match arms have the appropriate type? - _ => return Err(MatchCheckNotImplemented), + _ => return Err(MatchCheckErr::NotImplemented), } } (Pat::Wild, constructor) => Some(self.expand_wildcard(cx, constructor)?), (Pat::Path(_), Constructor::Enum(constructor)) => { - // enums with no associated data become `Pat::Path` + // unit enum variants become `Pat::Path` let pat_id = self.head().as_id().expect("we know this isn't a wild"); if !enum_variant_matches(cx, pat_id, *constructor) { None @@ -348,16 +351,23 @@ impl PatStack { Some(self.to_tail()) } } - (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(constructor)) => { + (Pat::TupleStruct { args: ref pat_ids, .. }, Constructor::Enum(enum_constructor)) => { let pat_id = self.head().as_id().expect("we know this isn't a wild"); - if !enum_variant_matches(cx, pat_id, *constructor) { + if !enum_variant_matches(cx, pat_id, *enum_constructor) { None } else { + // If the enum variant matches, then we need to confirm + // that the number of patterns aligns with the expected + // number of patterns for that enum variant. + if pat_ids.len() != constructor.arity(cx)? { + return Err(MatchCheckErr::MalformedMatchArm); + } + Some(self.replace_head_with(pat_ids)) } } - (Pat::Or(_), _) => return Err(MatchCheckNotImplemented), - (_, _) => return Err(MatchCheckNotImplemented), + (Pat::Or(_), _) => return Err(MatchCheckErr::NotImplemented), + (_, _) => return Err(MatchCheckErr::NotImplemented), }; Ok(result) @@ -514,7 +524,7 @@ pub(crate) fn is_useful( return if any_useful { Ok(Usefulness::Useful) } else if found_unimplemented { - Err(MatchCheckNotImplemented) + Err(MatchCheckErr::NotImplemented) } else { Ok(Usefulness::NotUseful) }; @@ -567,7 +577,7 @@ pub(crate) fn is_useful( } if found_unimplemented { - Err(MatchCheckNotImplemented) + Err(MatchCheckErr::NotImplemented) } else { Ok(Usefulness::NotUseful) } @@ -604,7 +614,7 @@ impl Constructor { match cx.db.enum_data(e.parent).variants[e.local_id].variant_data.as_ref() { VariantData::Tuple(struct_field_data) => struct_field_data.len(), VariantData::Unit => 0, - _ => return Err(MatchCheckNotImplemented), + _ => return Err(MatchCheckErr::NotImplemented), } } }; @@ -637,20 +647,20 @@ fn pat_constructor(cx: &MatchCheckCtx, pat: PatIdOrWild) -> MatchCheckResult Some(Constructor::Tuple { arity: pats.len() }), Pat::Lit(lit_expr) => match cx.body.exprs[lit_expr] { Expr::Literal(Literal::Bool(val)) => Some(Constructor::Bool(val)), - _ => return Err(MatchCheckNotImplemented), + _ => return Err(MatchCheckErr::NotImplemented), }, Pat::TupleStruct { .. } | Pat::Path(_) => { let pat_id = pat.as_id().expect("we already know this pattern is not a wild"); let variant_id = - cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckNotImplemented)?; + cx.infer.variant_resolution_for_pat(pat_id).ok_or(MatchCheckErr::NotImplemented)?; match variant_id { VariantId::EnumVariantId(enum_variant_id) => { Some(Constructor::Enum(enum_variant_id)) } - _ => return Err(MatchCheckNotImplemented), + _ => return Err(MatchCheckErr::NotImplemented), } } - _ => return Err(MatchCheckNotImplemented), + _ => return Err(MatchCheckErr::NotImplemented), }; Ok(res) @@ -1324,6 +1334,26 @@ mod tests { check_diagnostic(content); } + #[test] + fn malformed_match_arm_tuple_enum_missing_pattern() { + let content = r" + enum Either { + A, + B(u32), + } + fn test_fn() { + match Either::A { + Either::A => (), + Either::B() => (), + } + } + "; + + // We are testing to be sure we don't panic here when the match + // arm `Either::B` is missing its pattern. + check_no_diagnostic(content); + } + #[test] fn enum_not_in_scope() { let content = r" -- cgit v1.2.3 From 36c110ee096d46b02aa2a5adfaf138cd8c3872a7 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Tue, 7 Apr 2020 15:39:50 -0700 Subject: match checking add additional test for match checking tuple with missing pattern --- crates/ra_hir_ty/src/_match.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'crates') diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 037db5cf8..9e9a9d047 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -1334,6 +1334,20 @@ mod tests { check_diagnostic(content); } + #[test] + fn malformed_match_arm_tuple_missing_pattern() { + let content = r" + fn test_fn() { + match (0) { + () => (), + } + } + "; + + // Match arms with the incorrect type are filtered out. + check_diagnostic(content); + } + #[test] fn malformed_match_arm_tuple_enum_missing_pattern() { let content = r" -- cgit v1.2.3