]> git.lizzy.rs Git - rust.git/commitdiff
Auto merge of #13367 - matklad:fix-problem-matchers, r=Veykril
authorbors <bors@rust-lang.org>
Mon, 10 Oct 2022 07:54:55 +0000 (07:54 +0000)
committerbors <bors@rust-lang.org>
Mon, 10 Oct 2022 07:54:55 +0000 (07:54 +0000)
fix: in VSCode, correctly resolve relative paths to errors

VS Code problem matcher are restricted to be static "regexes". You can't create a problem matcher dynamically, and you can't use custom code in lieu of problem matcher.

This creates a problem for rust/cargo compiler errors. They use paths relative to the root of the Cargo workspace, but VS Code doesn't necessary know where that root is.

Luckily, there's a way out: our current problem matcher is defined like this:

    "fileLocation": [ "autoDetect", "${workspaceRoot}" ],

That means that relative pahts would be resoleved relative to workspace root. VS Code allows to specify a command inside `${}`. So we can plug custom logic there to fetch Cargo's workspace root!

And that's exactly what this PR is doing!

crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs
crates/ide-assists/src/handlers/generate_impl.rs
crates/ide-assists/src/utils.rs
crates/ide/src/goto_definition.rs
crates/syntax/src/ast/edit_in_place.rs
crates/syntax/src/ast/make.rs

index 8d5cab283d0616d3297e864a782f61e4965975cd..970e948dfd930f4226be3c9697f0e19dead8d044 100644 (file)
@@ -9,7 +9,7 @@
     search::FileReference,
     FxHashSet, RootDatabase,
 };
-use itertools::{Itertools, Position};
+use itertools::Itertools;
 use syntax::{
     ast::{
         self, edit::IndentLevel, edit_in_place::Indent, make, AstNode, HasAttrs, HasGenericParams,
@@ -298,37 +298,7 @@ fn update_variant(variant: &ast::Variant, generics: Option<ast::GenericParamList
     let name = variant.name()?;
     let ty = generics
         .filter(|generics| generics.generic_params().count() > 0)
-        .map(|generics| {
-            let mut generic_str = String::with_capacity(8);
-
-            for (p, more) in generics.generic_params().with_position().map(|p| match p {
-                Position::First(p) | Position::Middle(p) => (p, true),
-                Position::Last(p) | Position::Only(p) => (p, false),
-            }) {
-                match p {
-                    ast::GenericParam::ConstParam(konst) => {
-                        if let Some(name) = konst.name() {
-                            generic_str.push_str(name.text().as_str());
-                        }
-                    }
-                    ast::GenericParam::LifetimeParam(lt) => {
-                        if let Some(lt) = lt.lifetime() {
-                            generic_str.push_str(lt.text().as_str());
-                        }
-                    }
-                    ast::GenericParam::TypeParam(ty) => {
-                        if let Some(name) = ty.name() {
-                            generic_str.push_str(name.text().as_str());
-                        }
-                    }
-                }
-                if more {
-                    generic_str.push_str(", ");
-                }
-            }
-
-            make::ty(&format!("{}<{}>", &name.text(), &generic_str))
-        })
+        .map(|generics| make::ty(&format!("{}{}", &name.text(), generics.to_generic_args())))
         .unwrap_or_else(|| make::ty(&name.text()));
 
     // change from a record to a tuple field list
index 68287a20bf8068ac45677908396ee9bf06c366fb..307cea3d0a4f875aee3a92d954c133c0688c22c1 100644 (file)
@@ -52,6 +52,7 @@ mod tests {
 
     use super::*;
 
+    // FIXME: break up into separate test fns
     #[test]
     fn test_add_impl() {
         check_assist(
@@ -134,6 +135,18 @@ impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> Defaulted<'a, 'b, T
             }"#,
         );
 
+        check_assist(
+            generate_impl,
+            r#"
+            struct Defaulted<const N: i32 = 0> {}$0"#,
+            r#"
+            struct Defaulted<const N: i32 = 0> {}
+
+            impl<const N: i32> Defaulted<N> {
+                $0
+            }"#,
+        );
+
         check_assist(
             generate_impl,
             r#"pub trait Trait {}
index 4ab6e2627fa75413837b55be578c1a237a4147c3..38396cd7d7bafd47c36e4d4d473915cdc41f827a 100644 (file)
@@ -2,8 +2,6 @@
 
 use std::ops;
 
-use itertools::Itertools;
-
 pub(crate) use gen_trait_fn_body::gen_trait_fn_body;
 use hir::{db::HirDatabase, HirDisplay, Semantics};
 use ide_db::{famous_defs::FamousDefs, path_transform::PathTransform, RootDatabase, SnippetCap};
@@ -15,7 +13,7 @@
         edit_in_place::{AttrsOwnerEdit, Removable},
         make, HasArgList, HasAttrs, HasGenericParams, HasName, HasTypeBounds, Whitespace,
     },
-    ted, AstNode, AstToken, Direction, SmolStr, SourceFile,
+    ted, AstNode, AstToken, Direction, SourceFile,
     SyntaxKind::*,
     SyntaxNode, TextRange, TextSize, T,
 };
@@ -424,34 +422,44 @@ pub(crate) fn generate_trait_impl_text(adt: &ast::Adt, trait_text: &str, code: &
 }
 
 fn generate_impl_text_inner(adt: &ast::Adt, trait_text: Option<&str>, code: &str) -> String {
-    let generic_params = adt.generic_param_list();
+    // Ensure lifetime params are before type & const params
+    let generic_params = adt.generic_param_list().map(|generic_params| {
+        let lifetime_params =
+            generic_params.lifetime_params().map(ast::GenericParam::LifetimeParam);
+        let ty_or_const_params = generic_params.type_or_const_params().filter_map(|param| {
+            // remove defaults since they can't be specified in impls
+            match param {
+                ast::TypeOrConstParam::Type(param) => {
+                    let param = param.clone_for_update();
+                    param.remove_default();
+                    Some(ast::GenericParam::TypeParam(param))
+                }
+                ast::TypeOrConstParam::Const(param) => {
+                    let param = param.clone_for_update();
+                    param.remove_default();
+                    Some(ast::GenericParam::ConstParam(param))
+                }
+            }
+        });
+
+        make::generic_param_list(itertools::chain(lifetime_params, ty_or_const_params))
+    });
+
+    // FIXME: use syntax::make & mutable AST apis instead
+    // `trait_text` and `code` can't be opaque blobs of text
     let mut buf = String::with_capacity(code.len());
+
+    // Copy any cfg attrs from the original adt
     buf.push_str("\n\n");
-    adt.attrs()
-        .filter(|attr| attr.as_simple_call().map(|(name, _arg)| name == "cfg").unwrap_or(false))
-        .for_each(|attr| buf.push_str(format!("{}\n", attr).as_str()));
+    let cfg_attrs = adt
+        .attrs()
+        .filter(|attr| attr.as_simple_call().map(|(name, _arg)| name == "cfg").unwrap_or(false));
+    cfg_attrs.for_each(|attr| buf.push_str(&format!("{attr}\n")));
+
+    // `impl{generic_params} {trait_text} for {name}{generic_params.to_generic_args()}`
     buf.push_str("impl");
     if let Some(generic_params) = &generic_params {
-        let lifetimes = generic_params.lifetime_params().map(|lt| format!("{}", lt.syntax()));
-        let toc_params = generic_params.type_or_const_params().map(|toc_param| {
-            let type_param = match toc_param {
-                ast::TypeOrConstParam::Type(x) => x,
-                ast::TypeOrConstParam::Const(x) => return x.syntax().to_string(),
-            };
-            let mut buf = String::new();
-            if let Some(it) = type_param.name() {
-                format_to!(buf, "{}", it.syntax());
-            }
-            if let Some(it) = type_param.colon_token() {
-                format_to!(buf, "{} ", it);
-            }
-            if let Some(it) = type_param.type_bound_list() {
-                format_to!(buf, "{}", it.syntax());
-            }
-            buf
-        });
-        let generics = lifetimes.chain(toc_params).format(", ");
-        format_to!(buf, "<{}>", generics);
+        format_to!(buf, "{generic_params}");
     }
     buf.push(' ');
     if let Some(trait_text) = trait_text {
@@ -460,23 +468,15 @@ fn generate_impl_text_inner(adt: &ast::Adt, trait_text: Option<&str>, code: &str
     }
     buf.push_str(&adt.name().unwrap().text());
     if let Some(generic_params) = generic_params {
-        let lifetime_params = generic_params
-            .lifetime_params()
-            .filter_map(|it| it.lifetime())
-            .map(|it| SmolStr::from(it.text()));
-        let toc_params = generic_params
-            .type_or_const_params()
-            .filter_map(|it| it.name())
-            .map(|it| SmolStr::from(it.text()));
-        format_to!(buf, "<{}>", lifetime_params.chain(toc_params).format(", "))
+        format_to!(buf, "{}", generic_params.to_generic_args());
     }
 
     match adt.where_clause() {
         Some(where_clause) => {
-            format_to!(buf, "\n{}\n{{\n{}\n}}", where_clause, code);
+            format_to!(buf, "\n{where_clause}\n{{\n{code}\n}}");
         }
         None => {
-            format_to!(buf, " {{\n{}\n}}", code);
+            format_to!(buf, " {{\n{code}\n}}");
         }
     }
 
index f86ea61d1586febd45d96994b91eb179eddde5f9..d0be1b3f4047942fd53ef846bea823316709059a 100644 (file)
@@ -48,10 +48,14 @@ pub(crate) fn goto_definition(
             _ => 1,
         })?;
     if let Some(doc_comment) = token_as_doc_comment(&original_token) {
-        return doc_comment.get_definition_with_descend_at(sema, position.offset, |def, _, _| {
-            let nav = def.try_to_nav(db)?;
-            Some(RangeInfo::new(original_token.text_range(), vec![nav]))
-        });
+        return doc_comment.get_definition_with_descend_at(
+            sema,
+            position.offset,
+            |def, _, link_range| {
+                let nav = def.try_to_nav(db)?;
+                Some(RangeInfo::new(link_range, vec![nav]))
+            },
+        );
     }
     let navs = sema
         .descend_into_macros(original_token.clone())
index eadebbe8a212bab90ff4a0857012ec762611865b..229e7419b736febecf341b114dc86b121e6e006d 100644 (file)
@@ -235,6 +235,24 @@ pub fn add_generic_param(&self, generic_param: ast::GenericParam) {
             }
         }
     }
+
+    /// Constructs a matching [`ast::GenericArgList`]
+    pub fn to_generic_args(&self) -> ast::GenericArgList {
+        let args = self.generic_params().filter_map(|param| match param {
+            ast::GenericParam::LifetimeParam(it) => {
+                Some(ast::GenericArg::LifetimeArg(make::lifetime_arg(it.lifetime()?)))
+            }
+            ast::GenericParam::TypeParam(it) => {
+                Some(ast::GenericArg::TypeArg(make::type_arg(make::ext::ty_name(it.name()?))))
+            }
+            ast::GenericParam::ConstParam(it) => {
+                // Name-only const params get parsed as `TypeArg`s
+                Some(ast::GenericArg::TypeArg(make::type_arg(make::ext::ty_name(it.name()?))))
+            }
+        });
+
+        make::generic_arg_list(args)
+    }
 }
 
 impl ast::WhereClause {
@@ -248,6 +266,42 @@ pub fn add_predicate(&self, predicate: ast::WherePred) {
     }
 }
 
+impl ast::TypeParam {
+    pub fn remove_default(&self) {
+        if let Some((eq, last)) = self
+            .syntax()
+            .children_with_tokens()
+            .find(|it| it.kind() == T![=])
+            .zip(self.syntax().last_child_or_token())
+        {
+            ted::remove_all(eq..=last);
+
+            // remove any trailing ws
+            if let Some(last) = self.syntax().last_token().filter(|it| it.kind() == WHITESPACE) {
+                last.detach();
+            }
+        }
+    }
+}
+
+impl ast::ConstParam {
+    pub fn remove_default(&self) {
+        if let Some((eq, last)) = self
+            .syntax()
+            .children_with_tokens()
+            .find(|it| it.kind() == T![=])
+            .zip(self.syntax().last_child_or_token())
+        {
+            ted::remove_all(eq..=last);
+
+            // remove any trailing ws
+            if let Some(last) = self.syntax().last_token().filter(|it| it.kind() == WHITESPACE) {
+                last.detach();
+            }
+        }
+    }
+}
+
 pub trait Removable: AstNode {
     fn remove(&self);
 }
@@ -264,7 +318,7 @@ fn remove(&self) {
 impl ast::PathSegment {
     pub fn get_or_create_generic_arg_list(&self) -> ast::GenericArgList {
         if self.generic_arg_list().is_none() {
-            let arg_list = make::generic_arg_list().clone_for_update();
+            let arg_list = make::generic_arg_list(empty()).clone_for_update();
             ted::append_child(self.syntax(), arg_list.syntax());
         }
         self.generic_arg_list().unwrap()
index 83f8bbac5880bfd5fa542f03e90af2110039d169..c9a21e12c085b3f3ed7464c7555371838725bd59 100644 (file)
@@ -88,6 +88,9 @@ pub fn empty_block_expr() -> ast::BlockExpr {
         block_expr(None, None)
     }
 
+    pub fn ty_name(name: ast::Name) -> ast::Type {
+        ty_path(ident_path(&format!("{name}")))
+    }
     pub fn ty_bool() -> ast::Type {
         ty_path(ident_path("bool"))
     }
@@ -160,6 +163,7 @@ pub fn assoc_item_list() -> ast::AssocItemList {
     ast_from_text("impl C for D {}")
 }
 
+// FIXME: `ty_params` should be `ast::GenericArgList`
 pub fn impl_(
     ty: ast::Path,
     params: Option<ast::GenericParamList>,
@@ -185,10 +189,6 @@ pub fn impl_trait(
     ast_from_text(&format!("impl{ty_params} {trait_} for {ty}{ty_params} {{}}"))
 }
 
-pub(crate) fn generic_arg_list() -> ast::GenericArgList {
-    ast_from_text("const S: T<> = ();")
-}
-
 pub fn path_segment(name_ref: ast::NameRef) -> ast::PathSegment {
     ast_from_text(&format!("type __ = {name_ref};"))
 }
@@ -718,6 +718,21 @@ pub fn generic_param_list(
     ast_from_text(&format!("fn f<{args}>() {{ }}"))
 }
 
+pub fn type_arg(ty: ast::Type) -> ast::TypeArg {
+    ast_from_text(&format!("const S: T<{ty}> = ();"))
+}
+
+pub fn lifetime_arg(lifetime: ast::Lifetime) -> ast::LifetimeArg {
+    ast_from_text(&format!("const S: T<{lifetime}> = ();"))
+}
+
+pub(crate) fn generic_arg_list(
+    args: impl IntoIterator<Item = ast::GenericArg>,
+) -> ast::GenericArgList {
+    let args = args.into_iter().join(", ");
+    ast_from_text(&format!("const S: T<{args}> = ();"))
+}
+
 pub fn visibility_pub_crate() -> ast::Visibility {
     ast_from_text("pub(crate) struct S")
 }