Add flag to disallow opaque types for DisplayTarget::SourceCode

lowr · lowr · commit ac03de773f75 · 2023-04-12T19:03:48.000+09:00
diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs
@@ -150,6 +150,7 @@ pub trait HirDisplay {
         &'a self,
         db: &'a dyn HirDatabase,
         module_id: ModuleId,
+        allow_opaque: bool,
     ) -> Result<String, DisplaySourceCodeError> {
         let mut result = String::new();
         match self.hir_fmt(&mut HirFormatter {
@@ -160,7 +161,7 @@ pub trait HirDisplay {
             max_size: None,
             omit_verbose_types: false,
             closure_style: ClosureStyle::ImplFn,
-            display_target: DisplayTarget::SourceCode { module_id },
+            display_target: DisplayTarget::SourceCode { module_id, allow_opaque },
         }) {
             Ok(()) => {}
             Err(HirDisplayError::FmtError) => panic!("Writing to String can't fail!"),
@@ -249,25 +250,34 @@ pub enum DisplayTarget {
     Diagnostics,
     /// Display types for inserting them in source files.
     /// The generated code should compile, so paths need to be qualified.
-    SourceCode { module_id: ModuleId },
+    SourceCode { module_id: ModuleId, allow_opaque: bool },
     /// Only for test purpose to keep real types
     Test,
 }
 
 impl DisplayTarget {
-    fn is_source_code(&self) -> bool {
+    fn is_source_code(self) -> bool {
         matches!(self, Self::SourceCode { .. })
     }
-    fn is_test(&self) -> bool {
+
+    fn is_test(self) -> bool {
         matches!(self, Self::Test)
     }
+
+    fn allows_opaque(self) -> bool {
+        match self {
+            Self::SourceCode { allow_opaque, .. } => allow_opaque,
+            _ => true,
+        }
+    }
 }
 
 #[derive(Debug)]
 pub enum DisplaySourceCodeError {
     PathNotFound,
     UnknownType,
     Generator,
+    OpaqueType,
 }
 
 pub enum HirDisplayError {
@@ -768,7 +778,7 @@ impl HirDisplay for Ty {
                         };
                         write!(f, "{name}")?;
                     }
-                    DisplayTarget::SourceCode { module_id } => {
+                    DisplayTarget::SourceCode { module_id, allow_opaque: _ } => {
                         if let Some(path) = find_path::find_path(
                             db.upcast(),
                             ItemInNs::Types((*def_id).into()),
@@ -906,6 +916,11 @@ impl HirDisplay for Ty {
                 f.end_location_link();
             }
             TyKind::OpaqueType(opaque_ty_id, parameters) => {
+                if !f.display_target.allows_opaque() {
+                    return Err(HirDisplayError::DisplaySourceCodeError(
+                        DisplaySourceCodeError::OpaqueType,
+                    ));
+                }
                 let impl_trait_id = db.lookup_intern_impl_trait_id((*opaque_ty_id).into());
                 match impl_trait_id {
                     ImplTraitId::ReturnTypeImplTrait(func, idx) => {
@@ -953,8 +968,14 @@ impl HirDisplay for Ty {
                 }
             }
             TyKind::Closure(id, substs) => {
-                if f.display_target.is_source_code() && f.closure_style != ClosureStyle::ImplFn {
-                    never!("Only `impl Fn` is valid for displaying closures in source code");
+                if f.display_target.is_source_code() {
+                    if !f.display_target.allows_opaque() {
+                        return Err(HirDisplayError::DisplaySourceCodeError(
+                            DisplaySourceCodeError::OpaqueType,
+                        ));
+                    } else if f.closure_style != ClosureStyle::ImplFn {
+                        never!("Only `impl Fn` is valid for displaying closures in source code");
+                    }
                 }
                 match f.closure_style {
                     ClosureStyle::Hide => return write!(f, "{TYPE_HINT_TRUNCATION}"),
@@ -1053,6 +1074,11 @@ impl HirDisplay for Ty {
             }
             TyKind::Alias(AliasTy::Projection(p_ty)) => p_ty.hir_fmt(f)?,
             TyKind::Alias(AliasTy::Opaque(opaque_ty)) => {
+                if !f.display_target.allows_opaque() {
+                    return Err(HirDisplayError::DisplaySourceCodeError(
+                        DisplaySourceCodeError::OpaqueType,
+                    ));
+                }
                 let impl_trait_id = db.lookup_intern_impl_trait_id(opaque_ty.opaque_ty_id.into());
                 match impl_trait_id {
                     ImplTraitId::ReturnTypeImplTrait(func, idx) => {
diff --git a/crates/hir-ty/src/tests.rs b/crates/hir-ty/src/tests.rs
@@ -159,7 +159,7 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour
             let range = node.as_ref().original_file_range(&db);
             if let Some(expected) = types.remove(&range) {
                 let actual = if display_source {
-                    ty.display_source_code(&db, def.module(&db)).unwrap()
+                    ty.display_source_code(&db, def.module(&db), true).unwrap()
                 } else {
                     ty.display_test(&db).to_string()
                 };
@@ -175,7 +175,7 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour
             let range = node.as_ref().original_file_range(&db);
             if let Some(expected) = types.remove(&range) {
                 let actual = if display_source {
-                    ty.display_source_code(&db, def.module(&db)).unwrap()
+                    ty.display_source_code(&db, def.module(&db), true).unwrap()
                 } else {
                     ty.display_test(&db).to_string()
                 };
diff --git a/crates/ide-assists/src/handlers/add_explicit_type.rs b/crates/ide-assists/src/handlers/add_explicit_type.rs
@@ -69,7 +69,7 @@ pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext<'_>) -> O
         return None;
     }
 
-    let inferred_type = ty.display_source_code(ctx.db(), module.into()).ok()?;
+    let inferred_type = ty.display_source_code(ctx.db(), module.into(), false).ok()?;
     acc.add(
         AssistId("add_explicit_type", AssistKind::RefactorRewrite),
         format!("Insert explicit type `{inferred_type}`"),
diff --git a/crates/ide-assists/src/handlers/add_return_type.rs b/crates/ide-assists/src/handlers/add_return_type.rs
@@ -22,7 +22,7 @@ pub(crate) fn add_return_type(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt
     if ty.is_unit() {
         return None;
     }
-    let ty = ty.display_source_code(ctx.db(), module.into()).ok()?;
+    let ty = ty.display_source_code(ctx.db(), module.into(), true).ok()?;
 
     acc.add(
         AssistId("add_return_type", AssistKind::RefactorRewrite),
diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs
@@ -1884,7 +1884,7 @@ fn with_tail_expr(block: ast::BlockExpr, tail_expr: ast::Expr) -> ast::BlockExpr
 }
 
 fn format_type(ty: &hir::Type, ctx: &AssistContext<'_>, module: hir::Module) -> String {
-    ty.display_source_code(ctx.db(), module.into()).ok().unwrap_or_else(|| "_".to_string())
+    ty.display_source_code(ctx.db(), module.into(), true).ok().unwrap_or_else(|| "_".to_string())
 }
 
 fn make_ty(ty: &hir::Type, ctx: &AssistContext<'_>, module: hir::Module) -> ast::Type {
diff --git a/crates/ide-assists/src/handlers/generate_constant.rs b/crates/ide-assists/src/handlers/generate_constant.rs
@@ -46,7 +46,8 @@ pub(crate) fn generate_constant(acc: &mut Assists, ctx: &AssistContext<'_>) -> O
     let ty = ctx.sema.type_of_expr(&expr)?;
     let scope = ctx.sema.scope(statement.syntax())?;
     let constant_module = scope.module();
-    let type_name = ty.original().display_source_code(ctx.db(), constant_module.into()).ok()?;
+    let type_name =
+        ty.original().display_source_code(ctx.db(), constant_module.into(), false).ok()?;
     let target = statement.syntax().parent()?.text_range();
     let path = constant_token.syntax().ancestors().find_map(ast::Path::cast)?;
 
diff --git a/crates/ide-assists/src/handlers/generate_enum_variant.rs b/crates/ide-assists/src/handlers/generate_enum_variant.rs
@@ -192,7 +192,7 @@ fn expr_ty(
     scope: &hir::SemanticsScope<'_>,
 ) -> Option<ast::Type> {
     let ty = ctx.sema.type_of_expr(&arg).map(|it| it.adjusted())?;
-    let text = ty.display_source_code(ctx.db(), scope.module().into()).ok()?;
+    let text = ty.display_source_code(ctx.db(), scope.module().into(), false).ok()?;
     Some(make::ty(&text))
 }
 
diff --git a/crates/ide-assists/src/handlers/generate_function.rs b/crates/ide-assists/src/handlers/generate_function.rs
@@ -438,7 +438,7 @@ fn make_return_type(
             Some(ty) if ty.is_unit() => (None, false),
             Some(ty) => {
                 necessary_generic_params.extend(ty.generic_params(ctx.db()));
-                let rendered = ty.display_source_code(ctx.db(), target_module.into());
+                let rendered = ty.display_source_code(ctx.db(), target_module.into(), true);
                 match rendered {
                     Ok(rendered) => (Some(make::ty(&rendered)), false),
                     Err(_) => (Some(make::ty_placeholder()), true),
@@ -992,9 +992,9 @@ fn fn_arg_type(
             let famous_defs = &FamousDefs(&ctx.sema, ctx.sema.scope(fn_arg.syntax())?.krate());
             convert_reference_type(ty.strip_references(), ctx.db(), famous_defs)
                 .map(|conversion| conversion.convert_type(ctx.db()))
-                .or_else(|| ty.display_source_code(ctx.db(), target_module.into()).ok())
+                .or_else(|| ty.display_source_code(ctx.db(), target_module.into(), true).ok())
         } else {
-            ty.display_source_code(ctx.db(), target_module.into()).ok()
+            ty.display_source_code(ctx.db(), target_module.into(), true).ok()
         }
     }
 
diff --git a/crates/ide-assists/src/handlers/promote_local_to_const.rs b/crates/ide-assists/src/handlers/promote_local_to_const.rs
@@ -57,11 +57,13 @@ pub(crate) fn promote_local_to_const(acc: &mut Assists, ctx: &AssistContext<'_>)
     let local = ctx.sema.to_def(&pat)?;
     let ty = ctx.sema.type_of_pat(&pat.into())?.original;
 
-    if ty.contains_unknown() || ty.is_closure() {
-        cov_mark::hit!(promote_lcoal_not_applicable_if_ty_not_inferred);
-        return None;
-    }
-    let ty = ty.display_source_code(ctx.db(), module.into()).ok()?;
+    let ty = match ty.display_source_code(ctx.db(), module.into(), false) {
+        Ok(ty) => ty,
+        Err(_) => {
+            cov_mark::hit!(promote_local_not_applicable_if_ty_not_inferred);
+            return None;
+        }
+    };
 
     let initializer = let_stmt.initializer()?;
     if !is_body_const(&ctx.sema, &initializer) {
@@ -187,7 +189,7 @@ fn foo() {
 
     #[test]
     fn not_applicable_unknown_ty() {
-        cov_mark::check!(promote_lcoal_not_applicable_if_ty_not_inferred);
+        cov_mark::check!(promote_local_not_applicable_if_ty_not_inferred);
         check_assist_not_applicable(
             promote_local_to_const,
             r"
diff --git a/crates/ide-assists/src/handlers/replace_turbofish_with_explicit_type.rs b/crates/ide-assists/src/handlers/replace_turbofish_with_explicit_type.rs
@@ -55,7 +55,7 @@ pub(crate) fn replace_turbofish_with_explicit_type(
     let returned_type = match ctx.sema.type_of_expr(&initializer) {
         Some(returned_type) if !returned_type.original.contains_unknown() => {
             let module = ctx.sema.scope(let_stmt.syntax())?.module();
-            returned_type.original.display_source_code(ctx.db(), module.into()).ok()?
+            returned_type.original.display_source_code(ctx.db(), module.into(), false).ok()?
         }
         _ => {
             cov_mark::hit!(fallback_to_turbofish_type_if_type_info_not_available);
diff --git a/crates/ide-completion/src/completions/fn_param.rs b/crates/ide-completion/src/completions/fn_param.rs
@@ -127,7 +127,7 @@ fn params_from_stmt_list_scope(
         let module = scope.module().into();
         scope.process_all_names(&mut |name, def| {
             if let hir::ScopeDef::Local(local) = def {
-                if let Ok(ty) = local.ty(ctx.db).display_source_code(ctx.db, module) {
+                if let Ok(ty) = local.ty(ctx.db).display_source_code(ctx.db, module, true) {
                     cb(name, ty);
                 }
             }
diff --git a/crates/ide-completion/src/completions/type.rs b/crates/ide-completion/src/completions/type.rs
@@ -242,7 +242,7 @@ pub(crate) fn complete_ascribed_type(
         }
     }?
     .adjusted();
-    let ty_string = x.display_source_code(ctx.db, ctx.module.into()).ok()?;
+    let ty_string = x.display_source_code(ctx.db, ctx.module.into(), true).ok()?;
     acc.add(render_type_inference(ty_string, ctx));
     None
 }
diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs
@@ -116,7 +116,9 @@ impl<'a> PathTransform<'a> {
                         Some((
                             k,
                             ast::make::ty(
-                                &default.display_source_code(db, source_module.into()).ok()?,
+                                &default
+                                    .display_source_code(db, source_module.into(), false)
+                                    .ok()?,
                             ),
                         ))
                     }
diff --git a/crates/ide-diagnostics/src/handlers/missing_fields.rs b/crates/ide-diagnostics/src/handlers/missing_fields.rs
@@ -176,7 +176,9 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option<Vec<Ass
 fn make_ty(ty: &hir::Type, db: &dyn HirDatabase, module: hir::Module) -> ast::Type {
     let ty_str = match ty.as_adt() {
         Some(adt) => adt.name(db).to_string(),
-        None => ty.display_source_code(db, module.into()).ok().unwrap_or_else(|| "_".to_string()),
+        None => {
+            ty.display_source_code(db, module.into(), false).ok().unwrap_or_else(|| "_".to_string())
+        }
     };
 
     make::ty(&ty_str)
diff --git a/crates/ide-diagnostics/src/handlers/no_such_field.rs b/crates/ide-diagnostics/src/handlers/no_such_field.rs
@@ -69,7 +69,7 @@ fn missing_record_expr_field_fixes(
     let new_field = make::record_field(
         None,
         make::name(record_expr_field.field_name()?.ident_token()?.text()),
-        make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?),
+        make::ty(&new_field_type.display_source_code(sema.db, module.into(), true).ok()?),
     );
 
     let last_field = record_fields.fields().last()?;

Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext<'_>) -> O`
`69`	`69`	`return None;`
`70`	`70`	`}`
`71`	`71`
`72`		`- let inferred_type = ty.display_source_code(ctx.db(), module.into()).ok()?;`
	`72`	`+ let inferred_type = ty.display_source_code(ctx.db(), module.into(), false).ok()?;`
`73`	`73`	`acc.add(`
`74`	`74`	`AssistId("add_explicit_type", AssistKind::RefactorRewrite),`
`75`	`75`	format!("Insert explicit type `{inferred_type}`"),
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ pub(crate) fn add_return_type(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt`
`22`	`22`	`if ty.is_unit() {`
`23`	`23`	`return None;`
`24`	`24`	`}`
`25`		`- let ty = ty.display_source_code(ctx.db(), module.into()).ok()?;`
	`25`	`+ let ty = ty.display_source_code(ctx.db(), module.into(), true).ok()?;`
`26`	`26`
`27`	`27`	`acc.add(`
`28`	`28`	`AssistId("add_return_type", AssistKind::RefactorRewrite),`
Original file line number	Diff line number	Diff line change
`@@ -1884,7 +1884,7 @@ fn with_tail_expr(block: ast::BlockExpr, tail_expr: ast::Expr) -> ast::BlockExpr`
`1884`	`1884`	`}`
`1885`	`1885`
`1886`	`1886`	`fn format_type(ty: &hir::Type, ctx: &AssistContext<'_>, module: hir::Module) -> String {`
`1887`		`- ty.display_source_code(ctx.db(), module.into()).ok().unwrap_or_else(\|\| "_".to_string())`
	`1887`	`+ ty.display_source_code(ctx.db(), module.into(), true).ok().unwrap_or_else(\|\| "_".to_string())`
`1888`	`1888`	`}`
`1889`	`1889`
`1890`	`1890`	`fn make_ty(ty: &hir::Type, ctx: &AssistContext<'_>, module: hir::Module) -> ast::Type {`
Original file line number	Diff line number	Diff line change
`@@ -192,7 +192,7 @@ fn expr_ty(`
`192`	`192`	`scope: &hir::SemanticsScope<'_>,`
`193`	`193`	`) -> Option<ast::Type> {`
`194`	`194`	`let ty = ctx.sema.type_of_expr(&arg).map(\|it\| it.adjusted())?;`
`195`		`- let text = ty.display_source_code(ctx.db(), scope.module().into()).ok()?;`
	`195`	`+ let text = ty.display_source_code(ctx.db(), scope.module().into(), false).ok()?;`
`196`	`196`	`Some(make::ty(&text))`
`197`	`197`	`}`
`198`	`198`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ pub(crate) fn replace_turbofish_with_explicit_type(`
`55`	`55`	`let returned_type = match ctx.sema.type_of_expr(&initializer) {`
`56`	`56`	`Some(returned_type) if !returned_type.original.contains_unknown() => {`
`57`	`57`	`let module = ctx.sema.scope(let_stmt.syntax())?.module();`
`58`		`- returned_type.original.display_source_code(ctx.db(), module.into()).ok()?`
	`58`	`+ returned_type.original.display_source_code(ctx.db(), module.into(), false).ok()?`
`59`	`59`	`}`
`60`	`60`	`_ => {`
`61`	`61`	`cov_mark::hit!(fallback_to_turbofish_type_if_type_info_not_available);`