From a7858bb7bf0a784d56b2b9ef97785a4fa78f7853 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 25 Aug 2019 19:28:32 +0200 Subject: [PATCH 1/2] Report type mismatches in analysis-stats Only the number usually; each one individually when running with -v. --- crates/ra_cli/src/analysis_stats.rs | 38 ++++++++++++++++++++++++++++- crates/ra_hir/src/code_model.rs | 2 +- crates/ra_hir/src/expr.rs | 12 ++++----- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/crates/ra_cli/src/analysis_stats.rs b/crates/ra_cli/src/analysis_stats.rs index 7e7e6c073ce..77fa1d26e6b 100644 --- a/crates/ra_cli/src/analysis_stats.rs +++ b/crates/ra_cli/src/analysis_stats.rs @@ -1,7 +1,7 @@ use std::{collections::HashSet, fmt::Write, path::Path, time::Instant}; use ra_db::SourceDatabase; -use ra_hir::{Crate, HasSource, ImplItem, ModuleDef, Ty}; +use ra_hir::{Crate, HasSource, HirDisplay, ImplItem, ModuleDef, Ty}; use ra_syntax::AstNode; use crate::Result; @@ -66,6 +66,7 @@ pub fn run(verbose: bool, memory_usage: bool, path: &Path, only: Option<&str>) - let mut num_exprs = 0; let mut num_exprs_unknown = 0; let mut num_exprs_partially_unknown = 0; + let mut num_type_mismatches = 0; for f in funcs { let name = f.name(db); let mut msg = format!("processing: {}", name); @@ -100,6 +101,40 @@ pub fn run(verbose: bool, memory_usage: bool, path: &Path, only: Option<&str>) - num_exprs_partially_unknown += 1; } } + if let Some(mismatch) = inference_result.type_mismatch_for_expr(expr_id) { + num_type_mismatches += 1; + if verbose { + let src = f.source(db); + let original_file = src.file_id.original_file(db); + let path = db.file_relative_path(original_file); + let line_index = host.analysis().file_line_index(original_file).unwrap(); + let body_source_map = f.body_source_map(db); + let syntax_node = body_source_map.expr_syntax(expr_id); + let line_col = syntax_node.map(|syntax_node| { + ( + line_index.line_col(syntax_node.range().start()), + line_index.line_col(syntax_node.range().end()), + ) + }); + let line_col = match line_col { + Some((start, end)) => format!( + "{}:{}-{}:{}", + start.line + 1, + start.col_utf16, + end.line + 1, + end.col_utf16 + ), + None => "?:?".to_string(), + }; + bar.println(format!( + "{} {}: Expected {}, got {}", + path.display(), + line_col, + mismatch.expected.display(db), + mismatch.actual.display(db) + )); + } + } } bar.inc(1); } @@ -115,6 +150,7 @@ pub fn run(verbose: bool, memory_usage: bool, path: &Path, only: Option<&str>) - num_exprs_partially_unknown, (num_exprs_partially_unknown * 100 / num_exprs) ); + println!("Type mismatches: {}", num_type_mismatches); println!("Inference: {:?}, {}", inference_time.elapsed(), ra_prof::memory_usage()); println!("Total: {:?}, {}", analysis_time.elapsed(), ra_prof::memory_usage()); diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 66a58efed3d..0f9ff97f101 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -617,7 +617,7 @@ impl Function { self.data(db).name.clone() } - pub(crate) fn body_source_map(self, db: &impl HirDatabase) -> Arc { + pub fn body_source_map(self, db: &impl HirDatabase) -> Arc { db.body_with_source_map(self.into()).1 } diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs index 7cdc7555c55..57225ae91cf 100644 --- a/crates/ra_hir/src/expr.rs +++ b/crates/ra_hir/src/expr.rs @@ -128,27 +128,27 @@ impl Index for Body { } impl BodySourceMap { - pub(crate) fn expr_syntax(&self, expr: ExprId) -> Option { + pub fn expr_syntax(&self, expr: ExprId) -> Option { self.expr_map_back.get(expr).cloned() } - pub(crate) fn syntax_expr(&self, ptr: SyntaxNodePtr) -> Option { + pub fn syntax_expr(&self, ptr: SyntaxNodePtr) -> Option { self.expr_map.get(&ptr).cloned() } - pub(crate) fn node_expr(&self, node: &ast::Expr) -> Option { + pub fn node_expr(&self, node: &ast::Expr) -> Option { self.expr_map.get(&SyntaxNodePtr::new(node.syntax())).cloned() } - pub(crate) fn pat_syntax(&self, pat: PatId) -> Option { + pub fn pat_syntax(&self, pat: PatId) -> Option { self.pat_map_back.get(pat).cloned() } - pub(crate) fn node_pat(&self, node: &ast::Pat) -> Option { + pub fn node_pat(&self, node: &ast::Pat) -> Option { self.pat_map.get(&Either::A(AstPtr::new(node))).cloned() } - pub(crate) fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr { + pub fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr { self.field_map[&(expr, field)] } } From f92177cfb5088809892455262841e24cf1ecf5b6 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 30 Aug 2019 20:16:28 +0200 Subject: [PATCH 2/2] Add an expr_source method analogous to the source methods in the code model ... and use that instead of exposing the source map. --- crates/ra_cli/src/analysis_stats.rs | 53 ++++++++++++++--------------- crates/ra_hir/src/code_model.rs | 51 ++++++++++++++++++++------- crates/ra_hir/src/code_model/src.rs | 30 ++++++++++++++-- crates/ra_hir/src/expr.rs | 12 +++---- crates/ra_hir/src/lib.rs | 6 ++-- crates/ra_hir/src/source_binder.rs | 6 ++-- crates/ra_hir/src/ty/infer.rs | 4 +-- crates/ra_syntax/src/ptr.rs | 5 ++- 8 files changed, 107 insertions(+), 60 deletions(-) diff --git a/crates/ra_cli/src/analysis_stats.rs b/crates/ra_cli/src/analysis_stats.rs index 77fa1d26e6b..d355fa2e83a 100644 --- a/crates/ra_cli/src/analysis_stats.rs +++ b/crates/ra_cli/src/analysis_stats.rs @@ -1,7 +1,7 @@ use std::{collections::HashSet, fmt::Write, path::Path, time::Instant}; use ra_db::SourceDatabase; -use ra_hir::{Crate, HasSource, HirDisplay, ImplItem, ModuleDef, Ty}; +use ra_hir::{Crate, HasBodySource, HasSource, HirDisplay, ImplItem, ModuleDef, Ty}; use ra_syntax::AstNode; use crate::Result; @@ -104,35 +104,34 @@ pub fn run(verbose: bool, memory_usage: bool, path: &Path, only: Option<&str>) - if let Some(mismatch) = inference_result.type_mismatch_for_expr(expr_id) { num_type_mismatches += 1; if verbose { - let src = f.source(db); - let original_file = src.file_id.original_file(db); - let path = db.file_relative_path(original_file); - let line_index = host.analysis().file_line_index(original_file).unwrap(); - let body_source_map = f.body_source_map(db); - let syntax_node = body_source_map.expr_syntax(expr_id); - let line_col = syntax_node.map(|syntax_node| { - ( - line_index.line_col(syntax_node.range().start()), - line_index.line_col(syntax_node.range().end()), - ) - }); - let line_col = match line_col { - Some((start, end)) => format!( - "{}:{}-{}:{}", + let src = f.expr_source(db, expr_id); + if let Some(src) = src { + // FIXME: it might be nice to have a function (on Analysis?) that goes from Source -> (LineCol, LineCol) directly + let original_file = src.file_id.original_file(db); + let path = db.file_relative_path(original_file); + let line_index = host.analysis().file_line_index(original_file).unwrap(); + let (start, end) = ( + line_index.line_col(src.ast.syntax().text_range().start()), + line_index.line_col(src.ast.syntax().text_range().end()), + ); + bar.println(format!( + "{} {}:{}-{}:{}: Expected {}, got {}", + path.display(), start.line + 1, start.col_utf16, end.line + 1, - end.col_utf16 - ), - None => "?:?".to_string(), - }; - bar.println(format!( - "{} {}: Expected {}, got {}", - path.display(), - line_col, - mismatch.expected.display(db), - mismatch.actual.display(db) - )); + end.col_utf16, + mismatch.expected.display(db), + mismatch.actual.display(db) + )); + } else { + bar.println(format!( + "{}: Expected {}, got {}", + name, + mismatch.expected.display(db), + mismatch.actual.display(db) + )); + } } } } diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 0f9ff97f101..f7efc1b6653 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -510,18 +510,6 @@ pub enum DefWithBody { impl_froms!(DefWithBody: Function, Const, Static); impl DefWithBody { - pub fn infer(self, db: &impl HirDatabase) -> Arc { - db.infer(self) - } - - pub fn body(self, db: &impl HirDatabase) -> Arc { - db.body_hir(self) - } - - pub fn body_source_map(self, db: &impl HirDatabase) -> Arc { - db.body_with_source_map(self).1 - } - /// Builds a resolver for code inside this item. pub(crate) fn resolver(self, db: &impl HirDatabase) -> Resolver { match self { @@ -532,6 +520,43 @@ impl DefWithBody { } } +pub trait HasBody: Copy { + fn infer(self, db: &impl HirDatabase) -> Arc; + fn body(self, db: &impl HirDatabase) -> Arc; + fn body_source_map(self, db: &impl HirDatabase) -> Arc; +} + +impl HasBody for T +where + T: Into + Copy + HasSource, +{ + fn infer(self, db: &impl HirDatabase) -> Arc { + db.infer(self.into()) + } + + fn body(self, db: &impl HirDatabase) -> Arc { + db.body_hir(self.into()) + } + + fn body_source_map(self, db: &impl HirDatabase) -> Arc { + db.body_with_source_map(self.into()).1 + } +} + +impl HasBody for DefWithBody { + fn infer(self, db: &impl HirDatabase) -> Arc { + db.infer(self) + } + + fn body(self, db: &impl HirDatabase) -> Arc { + db.body_hir(self) + } + + fn body_source_map(self, db: &impl HirDatabase) -> Arc { + db.body_with_source_map(self).1 + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Function { pub(crate) id: FunctionId, @@ -617,7 +642,7 @@ impl Function { self.data(db).name.clone() } - pub fn body_source_map(self, db: &impl HirDatabase) -> Arc { + pub(crate) fn body_source_map(self, db: &impl HirDatabase) -> Arc { db.body_with_source_map(self.into()).1 } diff --git a/crates/ra_hir/src/code_model/src.rs b/crates/ra_hir/src/code_model/src.rs index 32bd9c661ee..e5bae16ab5b 100644 --- a/crates/ra_hir/src/code_model/src.rs +++ b/crates/ra_hir/src/code_model/src.rs @@ -1,9 +1,9 @@ -use ra_syntax::ast; +use ra_syntax::ast::{self, AstNode}; use crate::{ ids::AstItemDef, AstDatabase, Const, DefDatabase, Enum, EnumVariant, FieldSource, Function, - HirFileId, MacroDef, Module, ModuleSource, Static, Struct, StructField, Trait, TypeAlias, - Union, + HasBody, HirDatabase, HirFileId, MacroDef, Module, ModuleSource, Static, Struct, StructField, + Trait, TypeAlias, Union, }; pub struct Source { @@ -108,3 +108,27 @@ impl HasSource for MacroDef { Source { file_id: self.id.0.file_id(), ast: self.id.0.to_node(db) } } } + +pub trait HasBodySource: HasBody + HasSource +where + Self::Ast: AstNode, +{ + fn expr_source( + self, + db: &impl HirDatabase, + expr_id: crate::expr::ExprId, + ) -> Option> { + let source_map = self.body_source_map(db); + let expr_syntax = source_map.expr_syntax(expr_id)?; + let source = self.source(db); + let node = expr_syntax.to_node(&source.ast.syntax()); + ast::Expr::cast(node).map(|ast| Source { file_id: source.file_id, ast }) + } +} + +impl HasBodySource for T +where + T: HasBody + HasSource, + T::Ast: AstNode, +{ +} diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs index 57225ae91cf..7cdc7555c55 100644 --- a/crates/ra_hir/src/expr.rs +++ b/crates/ra_hir/src/expr.rs @@ -128,27 +128,27 @@ impl Index for Body { } impl BodySourceMap { - pub fn expr_syntax(&self, expr: ExprId) -> Option { + pub(crate) fn expr_syntax(&self, expr: ExprId) -> Option { self.expr_map_back.get(expr).cloned() } - pub fn syntax_expr(&self, ptr: SyntaxNodePtr) -> Option { + pub(crate) fn syntax_expr(&self, ptr: SyntaxNodePtr) -> Option { self.expr_map.get(&ptr).cloned() } - pub fn node_expr(&self, node: &ast::Expr) -> Option { + pub(crate) fn node_expr(&self, node: &ast::Expr) -> Option { self.expr_map.get(&SyntaxNodePtr::new(node.syntax())).cloned() } - pub fn pat_syntax(&self, pat: PatId) -> Option { + pub(crate) fn pat_syntax(&self, pat: PatId) -> Option { self.pat_map_back.get(pat).cloned() } - pub fn node_pat(&self, node: &ast::Pat) -> Option { + pub(crate) fn node_pat(&self, node: &ast::Pat) -> Option { self.pat_map.get(&Either::A(AstPtr::new(node))).cloned() } - pub fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr { + pub(crate) fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr { self.field_map[&(expr, field)] } } diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs index 018fcd096fe..752653ad7a3 100644 --- a/crates/ra_hir/src/lib.rs +++ b/crates/ra_hir/src/lib.rs @@ -75,8 +75,8 @@ pub use self::{ pub use self::code_model::{ docs::{DocDef, Docs, Documentation}, - src::{HasSource, Source}, + src::{HasBodySource, HasSource, Source}, BuiltinType, Const, ConstData, Container, Crate, CrateDependency, DefWithBody, Enum, - EnumVariant, FieldSource, FnData, Function, MacroDef, Module, ModuleDef, ModuleSource, Static, - Struct, StructField, Trait, TypeAlias, Union, + EnumVariant, FieldSource, FnData, Function, HasBody, MacroDef, Module, ModuleDef, ModuleSource, + Static, Struct, StructField, Trait, TypeAlias, Union, }; diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs index 56ff7da3ab8..43aec201a7b 100644 --- a/crates/ra_hir/src/source_binder.rs +++ b/crates/ra_hir/src/source_binder.rs @@ -27,9 +27,9 @@ use crate::{ name, path::{PathKind, PathSegment}, ty::method_resolution::implements_trait, - AsName, AstId, Const, Crate, DefWithBody, Either, Enum, Function, HirDatabase, HirFileId, - MacroDef, Module, ModuleDef, Name, Path, PerNs, Resolution, Resolver, Static, Struct, Trait, - Ty, + AsName, AstId, Const, Crate, DefWithBody, Either, Enum, Function, HasBody, HirDatabase, + HirFileId, MacroDef, Module, ModuleDef, Name, Path, PerNs, Resolution, Resolver, Static, + Struct, Trait, Ty, }; /// Locates the module by `FileId`. Picks topmost module in the file. diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs index 8129904261a..9ba146299eb 100644 --- a/crates/ra_hir/src/ty/infer.rs +++ b/crates/ra_hir/src/ty/infer.rs @@ -50,8 +50,8 @@ use crate::{ }, ty::infer::diagnostics::InferenceDiagnostic, type_ref::{Mutability, TypeRef}, - AdtDef, ConstData, DefWithBody, FnData, Function, HirDatabase, ImplItem, ModuleDef, Name, Path, - StructField, + AdtDef, ConstData, DefWithBody, FnData, Function, HasBody, HirDatabase, ImplItem, ModuleDef, + Name, Path, StructField, }; mod unify; diff --git a/crates/ra_syntax/src/ptr.rs b/crates/ra_syntax/src/ptr.rs index 992034ef0ff..80e55d2aa40 100644 --- a/crates/ra_syntax/src/ptr.rs +++ b/crates/ra_syntax/src/ptr.rs @@ -15,9 +15,8 @@ impl SyntaxNodePtr { SyntaxNodePtr { range: node.text_range(), kind: node.kind() } } - pub fn to_node(self, root: &SyntaxNode) -> SyntaxNode { - assert!(root.parent().is_none()); - successors(Some(root.clone()), |node| { + pub fn to_node(self, parent: &SyntaxNode) -> SyntaxNode { + successors(Some(parent.clone()), |node| { node.children().find(|it| self.range.is_subrange(&it.text_range())) }) .find(|it| it.text_range() == self.range && it.kind() == self.kind)