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

Merge #6765

Browse source 
6765: Fix file range computation in macros r=jonas-schievink a=jonas-schievink

This also aligns the diagnostics behavior of `TestDB` with the one from the real IDE (by making the logic from `semantics.rs` a method on `InFile<&SyntaxNode>`), which makes bugs like this easier to find.

This should fix the misplaced diagnostics seen in https://github.com/rust-analyzer/rust-analyzer/issues/6747 and other issues.

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
This commit is contained in:
bors[bot] 2020-12-08 18:13:54 +00:00 committed by GitHub
commit 87e3500636
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 89 additions and 98 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
crates/hir/src/lib.rs
View file

@ -39,7 +39,7 @@ pub use crate::{
Struct, Trait, Type, TypeAlias, TypeParam, Union, VariantDef,
},
has_source::HasSource,
semantics::{original_range, PathResolution, Semantics, SemanticsScope},
semantics::{PathResolution, Semantics, SemanticsScope},
};
pub use hir_def::{

76
crates/hir/src/semantics.rs
View file

@ -13,10 +13,7 @@ use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo};
use hir_ty::associated_type_shorthand_candidates;
use itertools::Itertools;
use rustc_hash::{FxHashMap, FxHashSet};
use syntax::{
algo::{find_node_at_offset, skip_trivia_token},
ast, AstNode, Direction, SyntaxNode, SyntaxToken, TextRange, TextSize,
};
use syntax::{algo::find_node_at_offset, ast, AstNode, SyntaxNode, SyntaxToken, TextSize};
use crate::{
code_model::Access,
@ -25,7 +22,7 @@ use crate::{
semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx},
source_analyzer::{resolve_hir_path, SourceAnalyzer},
AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef,
Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, VariantDef,
Module, ModuleDef, Name, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, VariantDef,
};
#[derive(Debug, Clone, PartialEq, Eq)]
@ -372,7 +369,7 @@ impl<'db> SemanticsImpl<'db> {
fn original_range(&self, node: &SyntaxNode) -> FileRange {
let node = self.find_file(node.clone());
original_range(self.db, node.as_ref())
node.as_ref().original_file_range(self.db.upcast())
}
fn diagnostics_display_range(&self, diagnostics: &dyn Diagnostic) -> FileRange {
@ -380,7 +377,7 @@ impl<'db> SemanticsImpl<'db> {
let root = self.db.parse_or_expand(src.file_id).unwrap();
let node = src.value.to_node(&root);
self.cache(root, src.file_id);
original_range(self.db, src.with_value(&node))
src.with_value(&node).original_file_range(self.db.upcast())
}
fn ancestors_with_macros(&self, node: SyntaxNode) -> impl Iterator<Item = SyntaxNode> + '_ {
@ -771,68 +768,3 @@ impl<'a> SemanticsScope<'a> {
resolve_hir_path(self.db, &self.resolver, &path)
}
}
// FIXME: Change `HasSource` trait to work with `Semantics` and remove this?
pub fn original_range(db: &dyn HirDatabase, node: InFile<&SyntaxNode>) -> FileRange {
if let Some(range) = original_range_opt(db, node) {
let original_file = range.file_id.original_file(db.upcast());
if range.file_id == original_file.into() {
return FileRange { file_id: original_file, range: range.value };
}
log::error!("Fail to mapping up more for {:?}", range);
return FileRange { file_id: range.file_id.original_file(db.upcast()), range: range.value };
}
// Fall back to whole macro call
if let Some(expansion) = node.file_id.expansion_info(db.upcast()) {
if let Some(call_node) = expansion.call_node() {
return FileRange {
file_id: call_node.file_id.original_file(db.upcast()),
range: call_node.value.text_range(),
};
}
}
FileRange { file_id: node.file_id.original_file(db.upcast()), range: node.value.text_range() }
}
fn original_range_opt(
db: &dyn HirDatabase,
node: InFile<&SyntaxNode>,
) -> Option<InFile<TextRange>> {
let expansion = node.file_id.expansion_info(db.upcast())?;
// the input node has only one token ?
let single = skip_trivia_token(node.value.first_token()?, Direction::Next)?
== skip_trivia_token(node.value.last_token()?, Direction::Prev)?;
Some(node.value.descendants().find_map(|it| {
let first = skip_trivia_token(it.first_token()?, Direction::Next)?;
let first = ascend_call_token(db, &expansion, node.with_value(first))?;
let last = skip_trivia_token(it.last_token()?, Direction::Prev)?;
let last = ascend_call_token(db, &expansion, node.with_value(last))?;
if (!single && first == last) || (first.file_id != last.file_id) {
return None;
}
Some(first.with_value(first.value.text_range().cover(last.value.text_range())))
})?)
}
fn ascend_call_token(
db: &dyn HirDatabase,
expansion: &ExpansionInfo,
token: InFile<SyntaxToken>,
) -> Option<InFile<SyntaxToken>> {
let (mapped, origin) = expansion.map_token_up(token.as_ref())?;
if origin != Origin::Call {
return None;
}
if let Some(info) = mapped.file_id.expansion_info(db.upcast()) {
return ascend_call_token(db, &info, mapped);
}
Some(mapped)
}

12
crates/hir_def/src/test_db.rs
View file

@ -158,17 +158,11 @@ impl TestDB {
let src = d.display_source();
let root = db.parse_or_expand(src.file_id).unwrap();
// Place all diagnostics emitted in macro files on the original caller.
// Note that this does *not* match IDE behavior.
let mut src = src.map(|ptr| ptr.to_node(&root));
while let Some(exp) = src.file_id.call_node(db) {
src = exp;
}
let node = src.map(|ptr| ptr.to_node(&root));
let frange = node.as_ref().original_file_range(db);
let file_id = src.file_id.original_file(db);
let range = src.value.text_range();
let message = d.message().to_owned();
actual.entry(file_id).or_default().push((range, message));
actual.entry(frange.file_id).or_default().push((frange.range, message));
});
for (file_id, diags) in actual.iter_mut() {

70
crates/hir_expand/src/lib.rs
View file

@ -20,11 +20,11 @@ pub use mbe::{ExpandError, ExpandResult};
use std::hash::Hash;
use std::sync::Arc;
use base_db::{impl_intern_key, salsa, CrateId, FileId};
use base_db::{impl_intern_key, salsa, CrateId, FileId, FileRange};
use syntax::{
algo,
algo::{self, skip_trivia_token},
ast::{self, AstNode},
SyntaxNode, SyntaxToken, TextSize,
Direction, SyntaxNode, SyntaxToken, TextRange, TextSize,
};
use crate::ast_id_map::FileAstId;
@ -445,6 +445,70 @@ impl InFile<SyntaxNode> {
}
}
impl<'a> InFile<&'a SyntaxNode> {
pub fn original_file_range(self, db: &dyn db::AstDatabase) -> FileRange {
if let Some(range) = original_range_opt(db, self) {
let original_file = range.file_id.original_file(db);
if range.file_id == original_file.into() {
return FileRange { file_id: original_file, range: range.value };
}
log::error!("Fail to mapping up more for {:?}", range);
return FileRange { file_id: range.file_id.original_file(db), range: range.value };
}
// Fall back to whole macro call.
let mut node = self.cloned();
while let Some(call_node) = node.file_id.call_node(db) {
node = call_node;
}
let orig_file = node.file_id.original_file(db);
assert_eq!(node.file_id, orig_file.into());
FileRange { file_id: orig_file, range: node.value.text_range() }
}
}
fn original_range_opt(
db: &dyn db::AstDatabase,
node: InFile<&SyntaxNode>,
) -> Option<InFile<TextRange>> {
let expansion = node.file_id.expansion_info(db)?;
// the input node has only one token ?
let single = skip_trivia_token(node.value.first_token()?, Direction::Next)?
== skip_trivia_token(node.value.last_token()?, Direction::Prev)?;
Some(node.value.descendants().find_map(|it| {
let first = skip_trivia_token(it.first_token()?, Direction::Next)?;
let first = ascend_call_token(db, &expansion, node.with_value(first))?;
let last = skip_trivia_token(it.last_token()?, Direction::Prev)?;
let last = ascend_call_token(db, &expansion, node.with_value(last))?;
if (!single && first == last) || (first.file_id != last.file_id) {
return None;
}
Some(first.with_value(first.value.text_range().cover(last.value.text_range())))
})?)
}
fn ascend_call_token(
db: &dyn db::AstDatabase,
expansion: &ExpansionInfo,
token: InFile<SyntaxToken>,
) -> Option<InFile<SyntaxToken>> {
let (mapped, origin) = expansion.map_token_up(token.as_ref())?;
if origin != Origin::Call {
return None;
}
if let Some(info) = mapped.file_id.expansion_info(db) {
return ascend_call_token(db, &info, mapped);
}
Some(mapped)
}
impl InFile<SyntaxToken> {
pub fn ancestors_with_macros(
self,

23
crates/ide/src/display/navigation_target.rs
View file

@ -1,7 +1,7 @@
//! FIXME: write short doc here
use either::Either;
use hir::{original_range, AssocItem, FieldSource, HasSource, InFile, ModuleSource};
use hir::{AssocItem, FieldSource, HasSource, InFile, ModuleSource};
use ide_db::base_db::{FileId, SourceDatabase};
use ide_db::{defs::Definition, RootDatabase};
use syntax::{
@ -62,7 +62,8 @@ impl NavigationTarget {
pub(crate) fn from_module_to_decl(db: &RootDatabase, module: hir::Module) -> NavigationTarget {
let name = module.name(db).map(|it| it.to_string().into()).unwrap_or_default();
if let Some(src) = module.declaration_source(db) {
let frange = original_range(db, src.as_ref().map(|it| it.syntax()));
let node = src.as_ref().map(|it| it.syntax());
let frange = node.original_file_range(db);
let mut res = NavigationTarget::from_syntax(
frange.file_id,
name,
@ -104,8 +105,8 @@ impl NavigationTarget {
let name =
node.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_"));
let focus_range =
node.value.name().map(|it| original_range(db, node.with_value(it.syntax())).range);
let frange = original_range(db, node.map(|it| it.syntax()));
node.value.name().map(|it| node.with_value(it.syntax()).original_file_range(db).range);
let frange = node.map(|it| it.syntax()).original_file_range(db);
NavigationTarget::from_syntax(
frange.file_id,
@ -124,7 +125,7 @@ impl NavigationTarget {
) -> NavigationTarget {
let name =
named.value.name().map(|it| it.text().clone()).unwrap_or_else(|| SmolStr::new("_"));
let frange = original_range(db, node.map(|it| it.syntax()));
let frange = node.map(|it| it.syntax()).original_file_range(db);
NavigationTarget::from_syntax(
frange.file_id,
@ -236,7 +237,7 @@ impl ToNav for hir::Module {
(node.syntax(), node.name().map(|it| it.syntax().text_range()))
}
};
let frange = original_range(db, src.with_value(syntax));
let frange = src.with_value(syntax).original_file_range(db);
NavigationTarget::from_syntax(frange.file_id, name, focus, frange.range, syntax.kind())
}
}
@ -246,14 +247,14 @@ impl ToNav for hir::ImplDef {
let src = self.source(db);
let derive_attr = self.is_builtin_derive(db);
let frange = if let Some(item) = &derive_attr {
original_range(db, item.syntax())
item.syntax().original_file_range(db)
} else {
original_range(db, src.as_ref().map(|it| it.syntax()))
src.as_ref().map(|it| it.syntax()).original_file_range(db)
};
let focus_range = if derive_attr.is_some() {
None
} else {
src.value.self_ty().map(|ty| original_range(db, src.with_value(ty.syntax())).range)
src.value.self_ty().map(|ty| src.with_value(ty.syntax()).original_file_range(db).range)
};
NavigationTarget::from_syntax(
@ -278,7 +279,7 @@ impl ToNav for hir::Field {
res
}
FieldSource::Pos(it) => {
let frange = original_range(db, src.with_value(it.syntax()));
let frange = src.with_value(it.syntax()).original_file_range(db);
NavigationTarget::from_syntax(
frange.file_id,
"".into(),
@ -331,7 +332,7 @@ impl ToNav for hir::Local {
}
Either::Right(it) => it.syntax().clone(),
};
let full_range = original_range(db, src.with_value(&node));
let full_range = src.with_value(&node).original_file_range(db);
let name = match self.name(db) {
Some(it) => it.to_string().into(),
None => "".into(),

4
crates/rust-analyzer/src/cli/analysis_stats.rs
View file

@ -8,7 +8,7 @@ use std::{
use hir::{
db::{AstDatabase, DefDatabase, HirDatabase},
original_range, AssocItem, Crate, HasSource, HirDisplay, ModuleDef,
AssocItem, Crate, HasSource, HirDisplay, ModuleDef,
};
use hir_def::FunctionId;
use hir_ty::{Ty, TypeWalk};
@ -232,7 +232,7 @@ impl AnalysisStatsCmd {
// But also, we should just turn the type mismatches into diagnostics and provide these
let root = db.parse_or_expand(src.file_id).unwrap();
let node = src.map(|e| e.to_node(&root).syntax().clone());
let original_range = original_range(db, node.as_ref());
let original_range = node.as_ref().original_file_range(db);
let path = vfs.file_path(original_range.file_id);
let line_index =
host.analysis().file_line_index(original_range.file_id).unwrap();