Skip to content

Commit

Permalink
Properly account for editions in names
Browse files Browse the repository at this point in the history
This PR touches a lot of parts. But the main changes are changing
`hir_expand::Name` to be raw edition-dependently and only when necessary
(unrelated to how the user originally wrote the identifier),
and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware
(this was done in rust-lang#17896, but the FIXMEs were fixed here).

It is possible that I missed some cases, but most IDE parts should properly
escape (or not escape) identifiers now.

The rules of thumb are:

 - If we show the identifier to the user, its rawness should be determined
   by the edition of the edited crate. This is nice for IDE features,
   but really important for changes we insert to the source code.
 - For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update
   tests when an edition becomes stable, to avoid churn).
 - For debugging tools (helper methods and logs), I used `Edition::LATEST`.
  • Loading branch information
ChayimFriedman2 committed Aug 16, 2024
1 parent a594a2d commit 3d6129d
Show file tree
Hide file tree
Showing 179 changed files with 2,480 additions and 1,245 deletions.
14 changes: 10 additions & 4 deletions src/tools/rust-analyzer/crates/hir-def/src/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use hir_expand::{name::Name, ExpandError, InFile};
use la_arena::{Arena, ArenaMap, Idx, RawIdx};
use rustc_hash::FxHashMap;
use smallvec::SmallVec;
use span::MacroFileId;
use span::{Edition, MacroFileId};
use syntax::{ast, AstPtr, SyntaxNodePtr};
use triomphe::Arc;

Expand Down Expand Up @@ -201,17 +201,23 @@ impl Body {
self.block_scopes.iter().map(move |&block| (block, db.block_def_map(block)))
}

pub fn pretty_print(&self, db: &dyn DefDatabase, owner: DefWithBodyId) -> String {
pretty::print_body_hir(db, self, owner)
pub fn pretty_print(
&self,
db: &dyn DefDatabase,
owner: DefWithBodyId,
edition: Edition,
) -> String {
pretty::print_body_hir(db, self, owner, edition)
}

pub fn pretty_print_expr(
&self,
db: &dyn DefDatabase,
owner: DefWithBodyId,
expr: ExprId,
edition: Edition,
) -> String {
pretty::print_expr_hir(db, self, owner, expr)
pretty::print_expr_hir(db, self, owner, expr, edition)
}

fn new(
Expand Down
65 changes: 41 additions & 24 deletions src/tools/rust-analyzer/crates/hir-def/src/body/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::fmt::{self, Write};

use itertools::Itertools;
use span::Edition;

use crate::{
hir::{
Expand All @@ -15,20 +16,26 @@ use crate::{

use super::*;

pub(super) fn print_body_hir(db: &dyn DefDatabase, body: &Body, owner: DefWithBodyId) -> String {
pub(super) fn print_body_hir(
db: &dyn DefDatabase,
body: &Body,
owner: DefWithBodyId,
edition: Edition,
) -> String {
let header = match owner {
DefWithBodyId::FunctionId(it) => {
it.lookup(db).id.resolved(db, |it| format!("fn {}", it.name.display(db.upcast())))
}
DefWithBodyId::FunctionId(it) => it
.lookup(db)
.id
.resolved(db, |it| format!("fn {}", it.name.display(db.upcast(), edition))),
DefWithBodyId::StaticId(it) => it
.lookup(db)
.id
.resolved(db, |it| format!("static {} = ", it.name.display(db.upcast()))),
.resolved(db, |it| format!("static {} = ", it.name.display(db.upcast(), edition))),
DefWithBodyId::ConstId(it) => it.lookup(db).id.resolved(db, |it| {
format!(
"const {} = ",
match &it.name {
Some(name) => name.display(db.upcast()).to_string(),
Some(name) => name.display(db.upcast(), edition).to_string(),
None => "_".to_owned(),
}
)
Expand All @@ -39,13 +46,13 @@ pub(super) fn print_body_hir(db: &dyn DefDatabase, body: &Body, owner: DefWithBo
let enum_loc = loc.parent.lookup(db);
format!(
"enum {}::{}",
enum_loc.id.item_tree(db)[enum_loc.id.value].name.display(db.upcast()),
loc.id.item_tree(db)[loc.id.value].name.display(db.upcast()),
enum_loc.id.item_tree(db)[enum_loc.id.value].name.display(db.upcast(), edition),
loc.id.item_tree(db)[loc.id.value].name.display(db.upcast(), edition),
)
}
};

let mut p = Printer { db, body, buf: header, indent_level: 0, needs_indent: false };
let mut p = Printer { db, body, buf: header, indent_level: 0, needs_indent: false, edition };
if let DefWithBodyId::FunctionId(it) = owner {
p.buf.push('(');
let function_data = &db.function_data(it);
Expand Down Expand Up @@ -86,8 +93,10 @@ pub(super) fn print_expr_hir(
body: &Body,
_owner: DefWithBodyId,
expr: ExprId,
edition: Edition,
) -> String {
let mut p = Printer { db, body, buf: String::new(), indent_level: 0, needs_indent: false };
let mut p =
Printer { db, body, buf: String::new(), indent_level: 0, needs_indent: false, edition };
p.print_expr(expr);
p.buf
}
Expand All @@ -113,6 +122,7 @@ struct Printer<'a> {
buf: String,
indent_level: usize,
needs_indent: bool,
edition: Edition,
}

impl Write for Printer<'_> {
Expand Down Expand Up @@ -173,13 +183,14 @@ impl Printer<'_> {
Expr::OffsetOf(offset_of) => {
w!(self, "builtin#offset_of(");
self.print_type_ref(&offset_of.container);
let edition = self.edition;
w!(
self,
", {})",
offset_of
.fields
.iter()
.format_with(".", |field, f| f(&field.display(self.db.upcast())))
.format_with(".", |field, f| f(&field.display(self.db.upcast(), edition)))
);
}
Expr::Path(path) => self.print_path(path),
Expand All @@ -201,7 +212,7 @@ impl Printer<'_> {
}
Expr::Loop { body, label } => {
if let Some(lbl) = label {
w!(self, "{}: ", self.body[*lbl].name.display(self.db.upcast()));
w!(self, "{}: ", self.body[*lbl].name.display(self.db.upcast(), self.edition));
}
w!(self, "loop ");
self.print_expr(*body);
Expand All @@ -221,10 +232,11 @@ impl Printer<'_> {
}
Expr::MethodCall { receiver, method_name, args, generic_args } => {
self.print_expr(*receiver);
w!(self, ".{}", method_name.display(self.db.upcast()));
w!(self, ".{}", method_name.display(self.db.upcast(), self.edition));
if let Some(args) = generic_args {
w!(self, "::<");
print_generic_args(self.db, args, self).unwrap();
let edition = self.edition;
print_generic_args(self.db, args, self, edition).unwrap();
w!(self, ">");
}
w!(self, "(");
Expand Down Expand Up @@ -259,13 +271,13 @@ impl Printer<'_> {
Expr::Continue { label } => {
w!(self, "continue");
if let Some(lbl) = label {
w!(self, " {}", self.body[*lbl].name.display(self.db.upcast()));
w!(self, " {}", self.body[*lbl].name.display(self.db.upcast(), self.edition));
}
}
Expr::Break { expr, label } => {
w!(self, "break");
if let Some(lbl) = label {
w!(self, " {}", self.body[*lbl].name.display(self.db.upcast()));
w!(self, " {}", self.body[*lbl].name.display(self.db.upcast(), self.edition));
}
if let Some(expr) = expr {
self.whitespace();
Expand Down Expand Up @@ -307,9 +319,10 @@ impl Printer<'_> {
}

w!(self, "{{");
let edition = self.edition;
self.indented(|p| {
for field in &**fields {
w!(p, "{}: ", field.name.display(self.db.upcast()));
w!(p, "{}: ", field.name.display(self.db.upcast(), edition));
p.print_expr(field.expr);
wln!(p, ",");
}
Expand All @@ -326,7 +339,7 @@ impl Printer<'_> {
}
Expr::Field { expr, name } => {
self.print_expr(*expr);
w!(self, ".{}", name.display(self.db.upcast()));
w!(self, ".{}", name.display(self.db.upcast(), self.edition));
}
Expr::Await { expr } => {
self.print_expr(*expr);
Expand Down Expand Up @@ -464,8 +477,9 @@ impl Printer<'_> {
}
Expr::Literal(lit) => self.print_literal(lit),
Expr::Block { id: _, statements, tail, label } => {
let label =
label.map(|lbl| format!("{}: ", self.body[lbl].name.display(self.db.upcast())));
let label = label.map(|lbl| {
format!("{}: ", self.body[lbl].name.display(self.db.upcast(), self.edition))
});
self.print_block(label.as_deref(), statements, tail);
}
Expr::Unsafe { id: _, statements, tail } => {
Expand Down Expand Up @@ -539,9 +553,10 @@ impl Printer<'_> {
}

w!(self, " {{");
let edition = self.edition;
self.indented(|p| {
for arg in args.iter() {
w!(p, "{}: ", arg.name.display(self.db.upcast()));
w!(p, "{}: ", arg.name.display(self.db.upcast(), edition));
p.print_pat(arg.pat);
wln!(p, ",");
}
Expand Down Expand Up @@ -686,11 +701,13 @@ impl Printer<'_> {
}

fn print_type_ref(&mut self, ty: &TypeRef) {
print_type_ref(self.db, ty, self).unwrap();
let edition = self.edition;
print_type_ref(self.db, ty, self, edition).unwrap();
}

fn print_path(&mut self, path: &Path) {
print_path(self.db, path, self).unwrap();
let edition = self.edition;
print_path(self.db, path, self, edition).unwrap();
}

fn print_binding(&mut self, id: BindingId) {
Expand All @@ -701,6 +718,6 @@ impl Printer<'_> {
BindingAnnotation::Ref => "ref ",
BindingAnnotation::RefMut => "ref mut ",
};
w!(self, "{}{}", mode, name.display(self.db.upcast()));
w!(self, "{}{}", mode, name.display(self.db.upcast(), self.edition));
}
}
6 changes: 3 additions & 3 deletions src/tools/rust-analyzer/crates/hir-def/src/body/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ fn main() {
},
);
}"#]]
.assert_eq(&body.pretty_print(&db, def))
.assert_eq(&body.pretty_print(&db, def, Edition::CURRENT))
}

#[test]
Expand Down Expand Up @@ -285,7 +285,7 @@ impl SsrError {
),
);
}"#]]
.assert_eq(&body.pretty_print(&db, def))
.assert_eq(&body.pretty_print(&db, def, Edition::CURRENT))
}

#[test]
Expand Down Expand Up @@ -333,5 +333,5 @@ fn f(a: i32, b: u32) -> String {
);
};
}"#]]
.assert_eq(&body.pretty_print(&db, def))
.assert_eq(&body.pretty_print(&db, def, Edition::CURRENT))
}
7 changes: 5 additions & 2 deletions src/tools/rust-analyzer/crates/hir-def/src/find_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,7 @@ mod tests {
use expect_test::{expect, Expect};
use hir_expand::db::ExpandDatabase;
use itertools::Itertools;
use span::Edition;
use stdx::format_to;
use syntax::ast::AstNode;
use test_fixture::WithFixture;
Expand Down Expand Up @@ -717,8 +718,10 @@ mod tests {
"{:7}(imports {}): {}\n",
format!("{:?}", prefix),
if ignore_local_imports { '✖' } else { '✔' },
found_path
.map_or_else(|| "<unresolvable>".to_owned(), |it| it.display(&db).to_string()),
found_path.map_or_else(
|| "<unresolvable>".to_owned(),
|it| it.display(&db, Edition::CURRENT).to_string()
),
);
}
expect.assert_eq(&res);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ pub(crate) fn parse(
}
}
ArgRef::Name(name, span) => {
let name = Name::new(name, tt::IdentIsRaw::No, call_ctx);
let name = Name::new(name, call_ctx);
if let Some((index, _)) = args.by_name(&name) {
record_usage(name, span);
// Name found in `args`, so we resolve it to its index.
Expand Down
13 changes: 9 additions & 4 deletions src/tools/rust-analyzer/crates/hir-def/src/hir/type_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use hir_expand::{
AstId,
};
use intern::{sym, Interned, Symbol};
use span::Edition;
use syntax::ast::{self, HasGenericArgs, HasName, IsString};

use crate::{
Expand Down Expand Up @@ -419,18 +420,22 @@ impl ConstRef {
param.default_val().map(|default| Self::from_const_arg(lower_ctx, Some(default)))
}

pub fn display<'a>(&'a self, db: &'a dyn ExpandDatabase) -> impl fmt::Display + 'a {
struct Display<'a>(&'a dyn ExpandDatabase, &'a ConstRef);
pub fn display<'a>(
&'a self,
db: &'a dyn ExpandDatabase,
edition: Edition,
) -> impl fmt::Display + 'a {
struct Display<'a>(&'a dyn ExpandDatabase, &'a ConstRef, Edition);
impl fmt::Display for Display<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.1 {
ConstRef::Scalar(s) => s.fmt(f),
ConstRef::Path(n) => n.display(self.0).fmt(f),
ConstRef::Path(n) => n.display(self.0, self.2).fmt(f),
ConstRef::Complex(_) => f.write_str("{const}"),
}
}
}
Display(db, self)
Display(db, self, edition)
}

// We special case literals and single identifiers, to speed up things.
Expand Down
16 changes: 11 additions & 5 deletions src/tools/rust-analyzer/crates/hir-def/src/import_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use hir_expand::name::Name;
use itertools::Itertools;
use rustc_hash::FxHashSet;
use smallvec::SmallVec;
use span::Edition;
use stdx::{format_to, TupleExt};
use syntax::ToSmolStr;
use triomphe::Arc;
Expand Down Expand Up @@ -66,7 +67,12 @@ impl ImportMap {
for (k, v) in self.item_to_info_map.iter() {
format_to!(out, "{:?} ({:?}) -> ", k, v.1);
for v in &v.0 {
format_to!(out, "{}:{:?}, ", v.name.display(db.upcast()), v.container);
format_to!(
out,
"{}:{:?}, ",
v.name.display(db.upcast(), Edition::CURRENT),
v.container
);
}
format_to!(out, "\n");
}
Expand All @@ -83,7 +89,7 @@ impl ImportMap {
// We've only collected items, whose name cannot be tuple field so unwrapping is fine.
.flat_map(|(&item, (info, _))| {
info.iter().enumerate().map(move |(idx, info)| {
(item, info.name.display(db.upcast()).to_smolstr(), idx as u32)
(item, info.name.unescaped().display(db.upcast()).to_smolstr(), idx as u32)
})
})
.collect();
Expand Down Expand Up @@ -461,7 +467,7 @@ fn search_maps(
query.search_mode.check(
&query.query,
query.case_sensitive,
&info.name.display(db.upcast()).to_smolstr(),
&info.name.unescaped().display(db.upcast()).to_smolstr(),
)
});
res.extend(iter.map(TupleExt::head));
Expand Down Expand Up @@ -577,7 +583,7 @@ mod tests {
Some(format!(
"{}::{}",
render_path(db, &trait_info[0]),
assoc_item_name.display(db.upcast())
assoc_item_name.display(db.upcast(), Edition::CURRENT)
))
}

Expand Down Expand Up @@ -616,7 +622,7 @@ mod tests {
module = parent;
}

segments.iter().rev().map(|it| it.display(db.upcast())).join("::")
segments.iter().rev().map(|it| it.display(db.upcast(), Edition::CURRENT)).join("::")
}

#[test]
Expand Down
Loading

0 comments on commit 3d6129d

Please sign in to comment.