squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Merge #4175

Browse source 
4175: Introduce HirDisplay method for rendering source code & use it in add_function assist r=flodiebold a=TimoFreiberg

Next feature for #3639.

So far the only change in the new `HirDisplay` method is that paths are qualified, but more changes will be necessary (omitting the function name from function types, returning an error instead of printing `"{unknown}"`, probably more).

Is that approach okay?

Co-authored-by: Timo Freiberg <timo.freiberg@gmail.com>
This commit is contained in:
bors[bot] 2020-05-09 09:29:11 +00:00 committed by GitHub
commit 25e37e2c93
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 300 additions and 99 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

174
crates/ra_assists/src/handlers/add_function.rs
View file

@ -43,16 +43,12 @@ pub(crate) fn add_function(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
return None;
}
let target_module = if let Some(qualifier) = path.qualifier() {
if let Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) =
ctx.sema.resolve_path(&qualifier)
{
Some(module.definition_source(ctx.sema.db))
} else {
return None;
}
} else {
None
let target_module = match path.qualifier() {
Some(qualifier) => match ctx.sema.resolve_path(&qualifier) {
Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) => Some(module),
_ => return None,
},
None => None,
};
let function_builder = FunctionBuilder::from_call(&ctx, &call, &path, target_module)?;
@ -83,25 +79,29 @@ struct FunctionBuilder {
}
impl FunctionBuilder {
/// Prepares a generated function that matches `call` in `generate_in`
/// (or as close to `call` as possible, if `generate_in` is `None`)
/// Prepares a generated function that matches `call`.
/// The function is generated in `target_module` or next to `call`
fn from_call(
ctx: &AssistContext,
call: &ast::CallExpr,
path: &ast::Path,
target_module: Option<hir::InFile<hir::ModuleSource>>,
target_module: Option<hir::Module>,
) -> Option<Self> {
let needs_pub = target_module.is_some();
let mut file = ctx.frange.file_id;
let target = if let Some(target_module) = target_module {
let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, target_module)?;
file = in_file;
target
} else {
next_space_for_fn_after_call_site(&call)?
let target = match &target_module {
Some(target_module) => {
let module_source = target_module.definition_source(ctx.db);
let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, &module_source)?;
file = in_file;
target
}
None => next_space_for_fn_after_call_site(&call)?,
};
let needs_pub = target_module.is_some();
let target_module = target_module.or_else(|| ctx.sema.scope(target.syntax()).module())?;
let fn_name = fn_name(&path)?;
let (type_params, params) = fn_args(ctx, &call)?;
let (type_params, params) = fn_args(ctx, target_module, &call)?;
Some(Self { target, fn_name, type_params, params, file, needs_pub })
}
@ -144,6 +144,15 @@ enum GeneratedFunctionTarget {
InEmptyItemList(ast::ItemList),
}
impl GeneratedFunctionTarget {
fn syntax(&self) -> &SyntaxNode {
match self {
GeneratedFunctionTarget::BehindItem(it) => it,
GeneratedFunctionTarget::InEmptyItemList(it) => it.syntax(),
}
}
}
fn fn_name(call: &ast::Path) -> Option<ast::Name> {
let name = call.segment()?.syntax().to_string();
Some(ast::make::name(&name))
@ -152,17 +161,17 @@ fn fn_name(call: &ast::Path) -> Option<ast::Name> {
/// Computes the type variables and arguments required for the generated function
fn fn_args(
ctx: &AssistContext,
target_module: hir::Module,
call: &ast::CallExpr,
) -> Option<(Option<ast::TypeParamList>, ast::ParamList)> {
let mut arg_names = Vec::new();
let mut arg_types = Vec::new();
for arg in call.arg_list()?.args() {
let arg_name = match fn_arg_name(&arg) {
arg_names.push(match fn_arg_name(&arg) {
Some(name) => name,
None => String::from("arg"),
};
arg_names.push(arg_name);
arg_types.push(match fn_arg_type(ctx, &arg) {
});
arg_types.push(match fn_arg_type(ctx, target_module, &arg) {
Some(ty) => ty,
None => String::from("()"),
});
@ -218,12 +227,21 @@ fn fn_arg_name(fn_arg: &ast::Expr) -> Option<String> {
}
}
fn fn_arg_type(ctx: &AssistContext, fn_arg: &ast::Expr) -> Option<String> {
fn fn_arg_type(
ctx: &AssistContext,
target_module: hir::Module,
fn_arg: &ast::Expr,
) -> Option<String> {
let ty = ctx.sema.type_of_expr(fn_arg)?;
if ty.is_unknown() {
return None;
}
Some(ty.display(ctx.sema.db).to_string())
if let Ok(rendered) = ty.display_source_code(ctx.db, target_module.into()) {
Some(rendered)
} else {
None
}
}
/// Returns the position inside the current mod or file
@ -252,10 +270,10 @@ fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option<GeneratedFu
fn next_space_for_fn_in_module(
db: &dyn hir::db::AstDatabase,
module: hir::InFile<hir::ModuleSource>,
module_source: &hir::InFile<hir::ModuleSource>,
) -> Option<(FileId, GeneratedFunctionTarget)> {
let file = module.file_id.original_file(db);
let assist_item = match module.value {
let file = module_source.file_id.original_file(db);
let assist_item = match &module_source.value {
hir::ModuleSource::SourceFile(it) => {
if let Some(last_item) = it.items().last() {
GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())
@ -599,8 +617,33 @@ fn bar(foo: impl Foo) {
}
#[test]
#[ignore]
// FIXME print paths properly to make this test pass
fn borrowed_arg() {
check_assist(
add_function,
r"
struct Baz;
fn baz() -> Baz { todo!() }
fn foo() {
bar<|>(&baz())
}
",
r"
struct Baz;
fn baz() -> Baz { todo!() }
fn foo() {
bar(&baz())
}
fn bar(baz: &Baz) {
<|>todo!()
}
",
)
}
#[test]
fn add_function_with_qualified_path_arg() {
check_assist(
add_function,
@ -609,10 +652,8 @@ mod Baz {
pub struct Bof;
pub fn baz() -> Bof { Bof }
}
mod Foo {
fn foo() {
<|>bar(super::Baz::baz())
}
fn foo() {
<|>bar(Baz::baz())
}
",
r"
@ -620,14 +661,12 @@ mod Baz {
pub struct Bof;
pub fn baz() -> Bof { Bof }
}
mod Foo {
fn foo() {
bar(super::Baz::baz())
}
fn foo() {
bar(Baz::baz())
}
fn bar(baz: super::Baz::Bof) {
<|>todo!()
}
fn bar(baz: Baz::Bof) {
<|>todo!()
}
",
)
@ -808,6 +847,40 @@ fn foo() {
)
}
#[test]
#[ignore]
// Ignored until local imports are supported.
// See https://github.com/rust-analyzer/rust-analyzer/issues/1165
fn qualified_path_uses_correct_scope() {
check_assist(
add_function,
"
mod foo {
pub struct Foo;
}
fn bar() {
use foo::Foo;
let foo = Foo;
baz<|>(foo)
}
",
"
mod foo {
pub struct Foo;
}
fn bar() {
use foo::Foo;
let foo = Foo;
baz(foo)
}
fn baz(foo: foo::Foo) {
<|>todo!()
}
",
)
}
#[test]
fn add_function_in_module_containing_other_items() {
check_assist(
@ -919,21 +992,6 @@ fn bar(baz: ()) {}
)
}
#[test]
fn add_function_not_applicable_if_function_path_not_singleton() {
// In the future this assist could be extended to generate functions
// if the path is in the same crate (or even the same workspace).
// For the beginning, I think this is fine.
check_assist_not_applicable(
add_function,
r"
fn foo() {
other_crate::bar<|>();
}
",
)
}
#[test]
#[ignore]
fn create_method_with_no_args() {

9
crates/ra_hir/src/code_model.rs
View file

@ -22,8 +22,11 @@ use hir_expand::{
MacroDefId, MacroDefKind,
};
use hir_ty::{
autoderef, display::HirFormatter, expr::ExprValidator, method_resolution, ApplicationTy,
Canonical, InEnvironment, Substs, TraitEnvironment, Ty, TyDefId, TypeCtor,
autoderef,
display::{HirDisplayError, HirFormatter},
expr::ExprValidator,
method_resolution, ApplicationTy, Canonical, InEnvironment, Substs, TraitEnvironment, Ty,
TyDefId, TypeCtor,
};
use ra_db::{CrateId, CrateName, Edition, FileId};
use ra_prof::profile;
@ -1319,7 +1322,7 @@ impl Type {
}
impl HirDisplay for Type {
fn hir_fmt(&self, f: &mut HirFormatter) -> std::fmt::Result {
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
self.ty.value.hir_fmt(f)
}
}

176
crates/ra_hir_ty/src/display.rs
View file

@ -6,28 +6,42 @@ use crate::{
db::HirDatabase, utils::generics, ApplicationTy, CallableDef, FnSig, GenericPredicate,
Obligation, ProjectionTy, Substs, TraitRef, Ty, TypeCtor,
};
use hir_def::{generics::TypeParamProvenance, AdtId, AssocContainerId, Lookup};
use hir_def::{
find_path, generics::TypeParamProvenance, item_scope::ItemInNs, AdtId, AssocContainerId,
Lookup, ModuleId,
};
use hir_expand::name::Name;
pub struct HirFormatter<'a, 'b> {
pub struct HirFormatter<'a> {
pub db: &'a dyn HirDatabase,
fmt: &'a mut fmt::Formatter<'b>,
fmt: &'a mut dyn fmt::Write,
buf: String,
curr_size: usize,
pub(crate) max_size: Option<usize>,
omit_verbose_types: bool,
display_target: DisplayTarget,
}
pub trait HirDisplay {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result;
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError>;
/// Returns a `Display`able type that is human-readable.
/// Use this for showing types to the user (e.g. diagnostics)
fn display<'a>(&'a self, db: &'a dyn HirDatabase) -> HirDisplayWrapper<'a, Self>
where
Self: Sized,
{
HirDisplayWrapper(db, self, None, false)
HirDisplayWrapper {
db,
t: self,
max_size: None,
omit_verbose_types: false,
display_target: DisplayTarget::Diagnostics,
}
}
/// Returns a `Display`able type that is human-readable and tries to be succinct.
/// Use this for showing types to the user where space is constrained (e.g. doc popups)
fn display_truncated<'a>(
&'a self,
db: &'a dyn HirDatabase,
@ -36,16 +50,46 @@ pub trait HirDisplay {
where
Self: Sized,
{
HirDisplayWrapper(db, self, max_size, true)
HirDisplayWrapper {
db,
t: self,
max_size,
omit_verbose_types: true,
display_target: DisplayTarget::Diagnostics,
}
}
/// Returns a String representation of `self` that can be inserted into the given module.
/// Use this when generating code (e.g. assists)
fn display_source_code<'a>(
&'a self,
db: &'a dyn HirDatabase,
module_id: ModuleId,
) -> Result<String, DisplaySourceCodeError> {
let mut result = String::new();
match self.hir_fmt(&mut HirFormatter {
db,
fmt: &mut result,
buf: String::with_capacity(20),
curr_size: 0,
max_size: None,
omit_verbose_types: false,
display_target: DisplayTarget::SourceCode { module_id },
}) {
Ok(()) => {}
Err(HirDisplayError::FmtError) => panic!("Writing to String can't fail!"),
Err(HirDisplayError::DisplaySourceCodeError(e)) => return Err(e),
};
Ok(result)
}
}
impl<'a, 'b> HirFormatter<'a, 'b> {
impl<'a> HirFormatter<'a> {
pub fn write_joined<T: HirDisplay>(
&mut self,
iter: impl IntoIterator<Item = T>,
sep: &str,
) -> fmt::Result {
) -> Result<(), HirDisplayError> {
let mut first = true;
for e in iter {
if !first {
@ -58,14 +102,14 @@ impl<'a, 'b> HirFormatter<'a, 'b> {
}
/// This allows using the `write!` macro directly with a `HirFormatter`.
pub fn write_fmt(&mut self, args: fmt::Arguments) -> fmt::Result {
pub fn write_fmt(&mut self, args: fmt::Arguments) -> Result<(), HirDisplayError> {
// We write to a buffer first to track output size
self.buf.clear();
fmt::write(&mut self.buf, args)?;
self.curr_size += self.buf.len();
// Then we write to the internal formatter from the buffer
self.fmt.write_str(&self.buf)
self.fmt.write_str(&self.buf).map_err(HirDisplayError::from)
}
pub fn should_truncate(&self) -> bool {
@ -81,34 +125,76 @@ impl<'a, 'b> HirFormatter<'a, 'b> {
}
}
pub struct HirDisplayWrapper<'a, T>(&'a dyn HirDatabase, &'a T, Option<usize>, bool);
#[derive(Clone, Copy)]
enum DisplayTarget {
/// Display types for inlays, doc popups, autocompletion, etc...
/// Showing `{unknown}` or not qualifying paths is fine here.
/// There's no reason for this to fail.
Diagnostics,
/// Display types for inserting them in source files.
/// The generated code should compile, so paths need to be qualified.
SourceCode { module_id: ModuleId },
}
#[derive(Debug)]
pub enum DisplaySourceCodeError {
PathNotFound,
}
pub enum HirDisplayError {
/// Errors that can occur when generating source code
DisplaySourceCodeError(DisplaySourceCodeError),
/// `FmtError` is required to be compatible with std::fmt::Display
FmtError,
}
impl From<fmt::Error> for HirDisplayError {
fn from(_: fmt::Error) -> Self {
Self::FmtError
}
}
pub struct HirDisplayWrapper<'a, T> {
db: &'a dyn HirDatabase,
t: &'a T,
max_size: Option<usize>,
omit_verbose_types: bool,
display_target: DisplayTarget,
}
impl<'a, T> fmt::Display for HirDisplayWrapper<'a, T>
where
T: HirDisplay,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.1.hir_fmt(&mut HirFormatter {
db: self.0,
match self.t.hir_fmt(&mut HirFormatter {
db: self.db,
fmt: f,
buf: String::with_capacity(20),
curr_size: 0,
max_size: self.2,
omit_verbose_types: self.3,
})
max_size: self.max_size,
omit_verbose_types: self.omit_verbose_types,
display_target: self.display_target,
}) {
Ok(()) => Ok(()),
Err(HirDisplayError::FmtError) => Err(fmt::Error),
Err(HirDisplayError::DisplaySourceCodeError(_)) => {
// This should never happen
panic!("HirDisplay failed when calling Display::fmt!")
}
}
}
}
const TYPE_HINT_TRUNCATION: &str = "…";
impl HirDisplay for &Ty {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result {
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
HirDisplay::hir_fmt(*self, f)
}
}
impl HirDisplay for ApplicationTy {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result {
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
if f.should_truncate() {
return write!(f, "{}", TYPE_HINT_TRUNCATION);
}
@ -191,12 +277,30 @@ impl HirDisplay for ApplicationTy {
}
}
TypeCtor::Adt(def_id) => {
let name = match def_id {
AdtId::StructId(it) => f.db.struct_data(it).name.clone(),
AdtId::UnionId(it) => f.db.union_data(it).name.clone(),
AdtId::EnumId(it) => f.db.enum_data(it).name.clone(),
};
write!(f, "{}", name)?;
match f.display_target {
DisplayTarget::Diagnostics => {
let name = match def_id {
AdtId::StructId(it) => f.db.struct_data(it).name.clone(),
AdtId::UnionId(it) => f.db.union_data(it).name.clone(),
AdtId::EnumId(it) => f.db.enum_data(it).name.clone(),
};
write!(f, "{}", name)?;
}
DisplayTarget::SourceCode { module_id } => {
if let Some(path) = find_path::find_path(
f.db.upcast(),
ItemInNs::Types(def_id.into()),
module_id,
) {
write!(f, "{}", path)?;
} else {
return Err(HirDisplayError::DisplaySourceCodeError(
DisplaySourceCodeError::PathNotFound,
));
}
}
}
if self.parameters.len() > 0 {
let mut non_default_parameters = Vec::with_capacity(self.parameters.len());
let parameters_to_write = if f.omit_verbose_types() {
@ -269,7 +373,7 @@ impl HirDisplay for ApplicationTy {
}
impl HirDisplay for ProjectionTy {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result {
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
if f.should_truncate() {
return write!(f, "{}", TYPE_HINT_TRUNCATION);
}
@ -287,7 +391,7 @@ impl HirDisplay for ProjectionTy {
}
impl HirDisplay for Ty {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result {
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
if f.should_truncate() {
return write!(f, "{}", TYPE_HINT_TRUNCATION);
}
@ -332,7 +436,7 @@ impl HirDisplay for Ty {
fn write_bounds_like_dyn_trait(
predicates: &[GenericPredicate],
f: &mut HirFormatter,
) -> fmt::Result {
) -> Result<(), HirDisplayError> {
// Note: This code is written to produce nice results (i.e.
// corresponding to surface Rust) for types that can occur in
// actual Rust. It will have weird results if the predicates
@ -394,7 +498,7 @@ fn write_bounds_like_dyn_trait(
}
impl TraitRef {
fn hir_fmt_ext(&self, f: &mut HirFormatter, use_as: bool) -> fmt::Result {
fn hir_fmt_ext(&self, f: &mut HirFormatter, use_as: bool) -> Result<(), HirDisplayError> {
if f.should_truncate() {
return write!(f, "{}", TYPE_HINT_TRUNCATION);
}
@ -416,19 +520,19 @@ impl TraitRef {
}
impl HirDisplay for TraitRef {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result {
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
self.hir_fmt_ext(f, false)
}
}
impl HirDisplay for &GenericPredicate {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result {
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
HirDisplay::hir_fmt(*self, f)
}
}
impl HirDisplay for GenericPredicate {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result {
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
if f.should_truncate() {
return write!(f, "{}", TYPE_HINT_TRUNCATION);
}
@ -452,15 +556,15 @@ impl HirDisplay for GenericPredicate {
}
impl HirDisplay for Obligation {
fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result {
match self {
Obligation::Trait(tr) => write!(f, "Implements({})", tr.display(f.db)),
fn hir_fmt(&self, f: &mut HirFormatter) -> Result<(), HirDisplayError> {
Ok(match self {
Obligation::Trait(tr) => write!(f, "Implements({})", tr.display(f.db))?,
Obligation::Projection(proj) => write!(
f,
"Normalize({} => {})",
proj.projection_ty.display(f.db),
proj.ty.display(f.db)
),
}
)?,
})
}
}

17
crates/ra_hir_ty/src/tests.rs
View file

@ -6,6 +6,7 @@ mod patterns;
mod traits;
mod method_resolution;
mod macros;
mod display_source_code;
use std::sync::Arc;
@ -16,7 +17,7 @@ use hir_def::{
item_scope::ItemScope,
keys,
nameres::CrateDefMap,
AssocItemId, DefWithBodyId, LocalModuleId, Lookup, ModuleDefId,
AssocItemId, DefWithBodyId, LocalModuleId, Lookup, ModuleDefId, ModuleId,
};
use hir_expand::{db::AstDatabase, InFile};
use insta::assert_snapshot;
@ -37,6 +38,18 @@ use crate::{
// update the snapshots.
fn type_at_pos(db: &TestDB, pos: FilePosition) -> String {
type_at_pos_displayed(db, pos, |ty, _| ty.display(db).to_string())
}
fn displayed_source_at_pos(db: &TestDB, pos: FilePosition) -> String {
type_at_pos_displayed(db, pos, |ty, module_id| ty.display_source_code(db, module_id).unwrap())
}
fn type_at_pos_displayed(
db: &TestDB,
pos: FilePosition,
display_fn: impl FnOnce(&Ty, ModuleId) -> String,
) -> String {
let file = db.parse(pos.file_id).ok().unwrap();
let expr = algo::find_node_at_offset::<ast::Expr>(file.syntax(), pos.offset).unwrap();
let fn_def = expr.syntax().ancestors().find_map(ast::FnDef::cast).unwrap();
@ -49,7 +62,7 @@ fn type_at_pos(db: &TestDB, pos: FilePosition) -> String {
if let Some(expr_id) = source_map.node_expr(InFile::new(pos.file_id.into(), &expr)) {
let infer = db.infer(func.into());
let ty = &infer[expr_id];
return ty.display(db).to_string();
return display_fn(ty, module);
}
panic!("Can't find expression")
}

23
crates/ra_hir_ty/src/tests/display_source_code.rs Normal file
View file

@ -0,0 +1,23 @@
use super::displayed_source_at_pos;
use crate::test_db::TestDB;
use ra_db::fixture::WithFixture;
#[test]
fn qualify_path_to_submodule() {
let (db, pos) = TestDB::with_position(
r#"
//- /main.rs
mod foo {
pub struct Foo;
}
fn bar() {
let foo: foo::Foo = foo::Foo;
foo<|>
}
"#,
);
assert_eq!("foo::Foo", displayed_source_at_pos(&db, pos));
}