Rollup merge of #101759 - lnicola:rust-analyzer-2022-09-13, r=lnicola

⬆️ rust-analyzer

r? `@ghost`

src/tools/rust-analyzer/.github/workflows/release.yaml

@@ -34,6 +34,7 @@ jobs:
           - os: ubuntu-20.04
             target: x86_64-unknown-linux-gnu
             code-target: linux-x64
+            container: ubuntu:18.04
           - os: ubuntu-20.04
             target: aarch64-unknown-linux-gnu
             code-target: linux-arm64
@@ -49,6 +50,7 @@ jobs:
 
     name: dist (${{ matrix.target }})
     runs-on: ${{ matrix.os }}
+    container: ${{ matrix.container }}
 
     env:
       RA_TARGET: ${{ matrix.target }}
@@ -59,6 +61,14 @@ jobs:
         with:
           fetch-depth: ${{ env.FETCH_DEPTH }}
 
+      - name: Install toolchain dependencies
+        if: matrix.container == 'ubuntu:18.04'
+        shell: bash
+        run: |
+          apt-get update && apt-get install -y build-essential curl
+          curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused -fsSL "https://sh.rustup.rs" | sh -s -- --profile minimal --default-toolchain none -y
+          echo "${CARGO_HOME:-$HOME/.cargo}/bin" >> $GITHUB_PATH
+
       - name: Install Rust toolchain
         run: |
           rustup update --no-self-update stable

src/tools/rust-analyzer/crates/hir-ty/src/chalk_ext.rs

@@ -164,6 +164,8 @@ impl TyExt for Ty {
 
     fn dyn_trait(&self) -> Option<TraitId> {
         let trait_ref = match self.kind(Interner) {
+            // The principal trait bound should be the first element of the bounds. This is an
+            // invariant ensured by `TyLoweringContext::lower_dyn_trait()`.
             TyKind::Dyn(dyn_ty) => dyn_ty.bounds.skip_binders().interned().get(0).and_then(|b| {
                 match b.skip_binders() {
                     WhereClause::Implemented(trait_ref) => Some(trait_ref),

src/tools/rust-analyzer/crates/hir-ty/src/lower.rs

@@ -981,43 +981,72 @@ impl<'a> TyLoweringContext<'a> {
 
     fn lower_dyn_trait(&self, bounds: &[Interned<TypeBound>]) -> Ty {
         let self_ty = TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 0)).intern(Interner);
+        // INVARIANT: The principal trait bound must come first. Others may be in any order but
+        // should be in the same order for the same set but possibly different order of bounds in
+        // the input.
+        // This invariant is used by `TyExt::dyn_trait()` and chalk.
         let bounds = self.with_shifted_in(DebruijnIndex::ONE, |ctx| {
-            let bounds =
-                bounds.iter().flat_map(|b| ctx.lower_type_bound(b, self_ty.clone(), false));
-
-            let mut auto_traits = SmallVec::<[_; 8]>::new();
-            let mut regular_traits = SmallVec::<[_; 2]>::new();
-            let mut other_bounds = SmallVec::<[_; 8]>::new();
-            for bound in bounds {
-                if let Some(id) = bound.trait_id() {
-                    if ctx.db.trait_data(from_chalk_trait_id(id)).is_auto {
-                        auto_traits.push(bound);
-                    } else {
-                        regular_traits.push(bound);
+            let mut bounds: Vec<_> = bounds
+                .iter()
+                .flat_map(|b| ctx.lower_type_bound(b, self_ty.clone(), false))
+                .collect();
+
+            let mut multiple_regular_traits = false;
+            let mut multiple_same_projection = false;
+            bounds.sort_unstable_by(|lhs, rhs| {
+                use std::cmp::Ordering;
+                match (lhs.skip_binders(), rhs.skip_binders()) {
+                    (WhereClause::Implemented(lhs), WhereClause::Implemented(rhs)) => {
+                        let lhs_id = lhs.trait_id;
+                        let lhs_is_auto = ctx.db.trait_data(from_chalk_trait_id(lhs_id)).is_auto;
+                        let rhs_id = rhs.trait_id;
+                        let rhs_is_auto = ctx.db.trait_data(from_chalk_trait_id(rhs_id)).is_auto;
+
+                        if !lhs_is_auto && !rhs_is_auto {
+                            multiple_regular_traits = true;
+                        }
+                        // Note that the ordering here is important; this ensures the invariant
+                        // mentioned above.
+                        (lhs_is_auto, lhs_id).cmp(&(rhs_is_auto, rhs_id))
                     }
-                } else {
-                    other_bounds.push(bound);
+                    (WhereClause::Implemented(_), _) => Ordering::Less,
+                    (_, WhereClause::Implemented(_)) => Ordering::Greater,
+                    (WhereClause::AliasEq(lhs), WhereClause::AliasEq(rhs)) => {
+                        match (&lhs.alias, &rhs.alias) {
+                            (AliasTy::Projection(lhs_proj), AliasTy::Projection(rhs_proj)) => {
+                                // We only compare the `associated_ty_id`s. We shouldn't have
+                                // multiple bounds for an associated type in the correct Rust code,
+                                // and if we do, we error out.
+                                if lhs_proj.associated_ty_id == rhs_proj.associated_ty_id {
+                                    multiple_same_projection = true;
+                                }
+                                lhs_proj.associated_ty_id.cmp(&rhs_proj.associated_ty_id)
+                            }
+                            // We don't produce `AliasTy::Opaque`s yet.
+                            _ => unreachable!(),
+                        }
+                    }
+                    // We don't produce `WhereClause::{TypeOutlives, LifetimeOutlives}` yet.
+                    _ => unreachable!(),
                 }
-            }
+            });
 
-            if regular_traits.len() > 1 {
+            if multiple_regular_traits || multiple_same_projection {
                 return None;
             }
 
-            auto_traits.sort_unstable_by_key(|b| b.trait_id().unwrap());
-            auto_traits.dedup();
+            // As multiple occurrences of the same auto traits *are* permitted, we dedulicate the
+            // bounds. We shouldn't have repeated elements besides auto traits at this point.
+            bounds.dedup();
 
-            Some(QuantifiedWhereClauses::from_iter(
-                Interner,
-                regular_traits.into_iter().chain(other_bounds).chain(auto_traits),
-            ))
+            Some(QuantifiedWhereClauses::from_iter(Interner, bounds))
         });
 
         if let Some(bounds) = bounds {
             let bounds = crate::make_single_type_binders(bounds);
             TyKind::Dyn(DynTy { bounds, lifetime: static_lifetime() }).intern(Interner)
         } else {
-            // FIXME: report error (additional non-auto traits)
+            // FIXME: report error (additional non-auto traits or associated type rebound)
             TyKind::Error.intern(Interner)
         }
     }

src/tools/rust-analyzer/crates/hir-ty/src/tests/traits.rs

@@ -3900,6 +3900,34 @@ fn g(t: &(dyn Sync + T<Proj = ()> + Send)) {
     );
 }
 
+#[test]
+fn dyn_multiple_projection_bounds() {
+    check_no_mismatches(
+        r#"
+trait Trait {
+    type T;
+    type U;
+}
+
+fn f(t: &dyn Trait<T = (), U = ()>) {}
+fn g(t: &dyn Trait<U = (), T = ()>) {
+    f(t);
+}
+        "#,
+    );
+
+    check_types(
+        r#"
+trait Trait {
+    type T;
+}
+
+fn f(t: &dyn Trait<T = (), T = ()>) {}
+   //^&{unknown}
+        "#,
+    );
+}
+
 #[test]
 fn dyn_duplicate_auto_trait() {
     check_no_mismatches(

src/tools/rust-analyzer/crates/ide-assists/src/handlers/add_missing_match_arms.rs

@@ -5,6 +5,7 @@ use hir::{Adt, Crate, HasAttrs, HasSource, ModuleDef, Semantics};
 use ide_db::RootDatabase;
 use ide_db::{famous_defs::FamousDefs, helpers::mod_path_to_ast};
 use itertools::Itertools;
+use syntax::ast::edit_in_place::Removable;
 use syntax::ast::{self, make, AstNode, HasName, MatchArmList, MatchExpr, Pat};
 
 use crate::{

src/tools/rust-analyzer/crates/ide-assists/src/handlers/inline_call.rs

@@ -7,6 +7,7 @@ use ide_db::{
     imports::insert_use::remove_path_if_in_use_stmt,
     path_transform::PathTransform,
     search::{FileReference, SearchScope},
+    source_change::SourceChangeBuilder,
     syntax_helpers::{insert_whitespace_into_node::insert_ws_into, node_ext::expr_as_name_ref},
     RootDatabase,
 };
@@ -100,18 +101,7 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) ->
                 builder.edit_file(file_id);
                 let count = refs.len();
                 // The collects are required as we are otherwise iterating while mutating 🙅‍♀️🙅‍♂️
-                let (name_refs, name_refs_use): (Vec<_>, Vec<_>) = refs
-                    .into_iter()
-                    .filter_map(|file_ref| match file_ref.name {
-                        ast::NameLike::NameRef(name_ref) => Some(name_ref),
-                        _ => None,
-                    })
-                    .partition_map(|name_ref| {
-                        match name_ref.syntax().ancestors().find_map(ast::UseTree::cast) {
-                            Some(use_tree) => Either::Right(builder.make_mut(use_tree)),
-                            None => Either::Left(name_ref),
-                        }
-                    });
+                let (name_refs, name_refs_use) = split_refs_and_uses(builder, refs, Some);
                 let call_infos: Vec<_> = name_refs
                     .into_iter()
                     .filter_map(CallInfo::from_name_ref)
@@ -130,11 +120,7 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) ->
                     .count();
                 if replaced + name_refs_use.len() == count {
                     // we replaced all usages in this file, so we can remove the imports
-                    name_refs_use.into_iter().for_each(|use_tree| {
-                        if let Some(path) = use_tree.path() {
-                            remove_path_if_in_use_stmt(&path);
-                        }
-                    })
+                    name_refs_use.iter().for_each(remove_path_if_in_use_stmt);
                 } else {
                     remove_def = false;
                 }
@@ -153,6 +139,23 @@ pub(crate) fn inline_into_callers(acc: &mut Assists, ctx: &AssistContext<'_>) ->
     )
 }
 
+pub(super) fn split_refs_and_uses<T: ast::AstNode>(
+    builder: &mut SourceChangeBuilder,
+    iter: impl IntoIterator<Item = FileReference>,
+    mut map_ref: impl FnMut(ast::NameRef) -> Option<T>,
+) -> (Vec<T>, Vec<ast::Path>) {
+    iter.into_iter()
+        .filter_map(|file_ref| match file_ref.name {
+            ast::NameLike::NameRef(name_ref) => Some(name_ref),
+            _ => None,
+        })
+        .filter_map(|name_ref| match name_ref.syntax().ancestors().find_map(ast::UseTree::cast) {
+            Some(use_tree) => builder.make_mut(use_tree).path().map(Either::Right),
+            None => map_ref(name_ref).map(Either::Left),
+        })
+        .partition_map(|either| either)
+}
+
 // Assist: inline_call
 //
 // Inlines a function or method body creating a `let` statement per parameter unless the parameter

src/tools/rust-analyzer/crates/ide-assists/src/handlers/inline_type_alias.rs

@@ -3,7 +3,10 @@
 // - Remove unused aliases if there are no longer any users, see inline_call.rs.
 
 use hir::{HasSource, PathResolution};
-use ide_db::{defs::Definition, search::FileReference};
+use ide_db::{
+    defs::Definition, imports::insert_use::ast_to_remove_for_path_in_use_stmt,
+    search::FileReference,
+};
 use itertools::Itertools;
 use std::collections::HashMap;
 use syntax::{
@@ -16,6 +19,8 @@ use crate::{
     AssistId, AssistKind,
 };
 
+use super::inline_call::split_refs_and_uses;
+
 // Assist: inline_type_alias_uses
 //
 // Inline a type alias into all of its uses where possible.
@@ -31,7 +36,7 @@ use crate::{
 // ```
 // ->
 // ```
-// type A = i32;
+//
 // fn id(x: i32) -> i32 {
 //     x
 // };
@@ -58,32 +63,41 @@ pub(crate) fn inline_type_alias_uses(acc: &mut Assists, ctx: &AssistContext<'_>)
         name.syntax().text_range(),
         |builder| {
             let usages = usages.all();
+            let mut definition_deleted = false;
 
             let mut inline_refs_for_file = |file_id, refs: Vec<FileReference>| {
                 builder.edit_file(file_id);
 
-                let path_types: Vec<ast::PathType> = refs
-                    .into_iter()
-                    .filter_map(|file_ref| match file_ref.name {
-                        ast::NameLike::NameRef(path_type) => {
-                            path_type.syntax().ancestors().nth(3).and_then(ast::PathType::cast)
-                        }
-                        _ => None,
-                    })
-                    .collect();
+                let (path_types, path_type_uses) =
+                    split_refs_and_uses(builder, refs, |path_type| {
+                        path_type.syntax().ancestors().nth(3).and_then(ast::PathType::cast)
+                    });
 
+                path_type_uses
+                    .iter()
+                    .flat_map(ast_to_remove_for_path_in_use_stmt)
+                    .for_each(|x| builder.delete(x.syntax().text_range()));
                 for (target, replacement) in path_types.into_iter().filter_map(|path_type| {
                     let replacement = inline(&ast_alias, &path_type)?.to_text(&concrete_type);
                     let target = path_type.syntax().text_range();
                     Some((target, replacement))
                 }) {
                     builder.replace(target, replacement);
                 }
+
+                if file_id == ctx.file_id() {
+                    builder.delete(ast_alias.syntax().text_range());
+                    definition_deleted = true;
+                }
             };
 
             for (file_id, refs) in usages.into_iter() {
                 inline_refs_for_file(file_id, refs);
             }
+            if !definition_deleted {
+                builder.edit_file(ctx.file_id());
+                builder.delete(ast_alias.syntax().text_range());
+            }
         },
     )
 }
@@ -929,7 +943,7 @@ fn foo() {
 }
 "#,
                 r#"
-type A = u32;
+
 
 fn foo() {
     let _: u32 = 3;
@@ -960,13 +974,13 @@ fn foo() {
                 r#"
 //- /lib.rs
 mod foo;
-type T<E> = Vec<E>;
+
 fn f() -> Vec<&str> {
     vec!["hello"]
 }
 
 //- /foo.rs
-use super::T;
+
 fn foo() {
     let _: Vec<i8> = Vec::new();
 }
@@ -990,7 +1004,12 @@ fn foo() {
 }
 "#,
                 r#"
-use super::I;
+//- /lib.rs
+mod foo;
+
+
+//- /foo.rs
+
 fn foo() {
     let _: i32 = 0;
 }

src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs

@@ -1,6 +1,10 @@
 use either::Either;
 use ide_db::imports::merge_imports::{try_merge_imports, try_merge_trees, MergeBehavior};
-use syntax::{algo::neighbor, ast, match_ast, ted, AstNode, SyntaxElement, SyntaxNode};
+use syntax::{
+    algo::neighbor,
+    ast::{self, edit_in_place::Removable},
+    match_ast, ted, AstNode, SyntaxElement, SyntaxNode,
+};
 
 use crate::{
     assist_context::{AssistContext, Assists},
@@ -76,7 +80,7 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio
                 .collect();
             for edit in edits_mut {
                 match edit {
-                    Remove(it) => it.as_ref().either(ast::Use::remove, ast::UseTree::remove),
+                    Remove(it) => it.as_ref().either(Removable::remove, Removable::remove),
                     Replace(old, new) => ted::replace(old, new),
                 }
             }

src/tools/rust-analyzer/crates/ide-assists/src/handlers/move_bounds.rs

@@ -1,5 +1,9 @@
 use syntax::{
-    ast::{self, edit_in_place::GenericParamsOwnerEdit, make, AstNode, HasName, HasTypeBounds},
+    ast::{
+        self,
+        edit_in_place::{GenericParamsOwnerEdit, Removable},
+        make, AstNode, HasName, HasTypeBounds,
+    },
     match_ast,
 };