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 #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 91aa3f4 commit 9d3cd5a
Show file tree
Hide file tree
Showing 180 changed files with 2,498 additions and 1,254 deletions.
14 changes: 10 additions & 4 deletions 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 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 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 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
2 changes: 1 addition & 1 deletion crates/hir-def/src/hir/format_args.rs
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 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
Loading

0 comments on commit 9d3cd5a

Please sign in to comment.