diff --git a/Cargo.lock b/Cargo.lock index b429aae01e..752edddd69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1104,6 +1104,7 @@ dependencies = [ "chalk-ir", "chalk-solve", "ena", + "expect", "insta", "itertools", "log", diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index c82883d0c1..11a0ecb8b2 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/crates/ra_hir/src/diagnostics.rs @@ -1,4 +1,6 @@ //! FIXME: write short doc here pub use hir_def::diagnostics::UnresolvedModule; pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink}; -pub use hir_ty::diagnostics::{MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField}; +pub use hir_ty::diagnostics::{ + MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField, +}; diff --git a/crates/ra_hir_ty/Cargo.toml b/crates/ra_hir_ty/Cargo.toml index d6df48db20..ce257dc0bb 100644 --- a/crates/ra_hir_ty/Cargo.toml +++ b/crates/ra_hir_ty/Cargo.toml @@ -32,3 +32,4 @@ chalk-ir = { version = "0.15.0" } [dev-dependencies] insta = "0.16.0" +expect = { path = "../expect" } diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index 0289911de0..5b0dda634f 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -197,3 +197,33 @@ impl AstDiagnostic for MissingUnsafe { ast::Expr::cast(node).unwrap() } } + +#[derive(Debug)] +pub struct MismatchedArgCount { + pub file: HirFileId, + pub call_expr: AstPtr, + pub expected: usize, + pub found: usize, +} + +impl Diagnostic for MismatchedArgCount { + fn message(&self) -> String { + let s = if self.expected == 1 { "" } else { "s" }; + format!("Expected {} argument{}, found {}", self.expected, s, self.found) + } + fn source(&self) -> InFile { + InFile { file_id: self.file, value: self.call_expr.clone().into() } + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + +impl AstDiagnostic for MismatchedArgCount { + type AST = ast::CallExpr; + fn ast(&self, db: &dyn AstDatabase) -> Self::AST { + let root = db.parse_or_expand(self.source().file_id).unwrap(); + let node = self.source().value.to_node(&root); + ast::CallExpr::cast(node).unwrap() + } +} diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 7db928dded..6f34aaf170 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs @@ -9,9 +9,11 @@ use rustc_hash::FxHashSet; use crate::{ db::HirDatabase, - diagnostics::{MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields}, + diagnostics::{ + MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, + }, utils::variant_data, - ApplicationTy, InferenceResult, Ty, TypeCtor, + ApplicationTy, CallableDef, InferenceResult, Ty, TypeCtor, _match::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness}, }; @@ -24,7 +26,8 @@ pub use hir_def::{ ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp, }, - LocalFieldId, VariantId, + src::HasSource, + LocalFieldId, Lookup, VariantId, }; pub struct ExprValidator<'a, 'b: 'a> { @@ -56,8 +59,15 @@ impl<'a, 'b> ExprValidator<'a, 'b> { missed_fields, ); } - if let Expr::Match { expr, arms } = expr { - self.validate_match(id, *expr, arms, db, self.infer.clone()); + + match expr { + Expr::Match { expr, arms } => { + self.validate_match(id, *expr, arms, db, self.infer.clone()); + } + Expr::Call { .. } | Expr::MethodCall { .. } => { + self.validate_call(db, id, expr); + } + _ => {} } } for (id, pat) in body.pats.iter() { @@ -138,6 +148,68 @@ impl<'a, 'b> ExprValidator<'a, 'b> { } } + fn validate_call(&mut self, db: &dyn HirDatabase, call_id: ExprId, expr: &Expr) -> Option<()> { + // Check that the number of arguments matches the number of parameters. + + // FIXME: Due to shortcomings in the current type system implementation, only emit this + // diagnostic if there are no type mismatches in the containing function. + if self.infer.type_mismatches.iter().next().is_some() { + return Some(()); + } + + let is_method_call = matches!(expr, Expr::MethodCall { .. }); + let (callee, args) = match expr { + Expr::Call { callee, args } => { + let callee = &self.infer.type_of_expr[*callee]; + let (callable, _) = callee.as_callable()?; + let callee = match callable { + CallableDef::FunctionId(func) => func, + + // FIXME: Handle tuple struct/variant constructor calls. + _ => return None, + }; + + (callee, args.clone()) + } + Expr::MethodCall { receiver, args, .. } => { + let callee = self.infer.method_resolution(call_id)?; + let mut args = args.clone(); + args.insert(0, *receiver); + (callee, args) + } + _ => return None, + }; + + let loc = callee.lookup(db.upcast()); + let ast = loc.source(db.upcast()); + let params = ast.value.param_list()?; + + let mut param_count = params.params().count(); + let mut arg_count = args.len(); + + if params.self_param().is_some() { + param_count += 1; + } + + if arg_count != param_count { + let (_, source_map) = db.body_with_source_map(self.func.into()); + if let Ok(source_ptr) = source_map.expr_syntax(call_id) { + if is_method_call { + param_count -= 1; + arg_count -= 1; + } + self.sink.push(MismatchedArgCount { + file: source_ptr.file_id, + call_expr: source_ptr.value, + expected: param_count, + found: arg_count, + }); + } + } + + None + } + fn validate_match( &mut self, id: ExprId, @@ -312,3 +384,121 @@ pub fn record_pattern_missing_fields( } Some((variant_def, missed_fields, exhaustive)) } + +#[cfg(test)] +mod tests { + use expect::{expect, Expect}; + use ra_db::fixture::WithFixture; + + use crate::{diagnostics::MismatchedArgCount, test_db::TestDB}; + + fn check_diagnostic(ra_fixture: &str, expect: Expect) { + let msg = TestDB::with_single_file(ra_fixture).0.diagnostic::().0; + expect.assert_eq(&msg); + } + + fn check_no_diagnostic(ra_fixture: &str) { + let (s, diagnostic_count) = + TestDB::with_single_file(ra_fixture).0.diagnostic::(); + + assert_eq!(0, diagnostic_count, "expected no diagnostic, found one: {}", s); + } + + #[test] + fn simple_free_fn_zero() { + check_diagnostic( + r" + fn zero() {} + fn f() { zero(1); } + ", + expect![["\"zero(1)\": Expected 0 arguments, found 1\n"]], + ); + + check_no_diagnostic( + r" + fn zero() {} + fn f() { zero(); } + ", + ); + } + + #[test] + fn simple_free_fn_one() { + check_diagnostic( + r" + fn one(arg: u8) {} + fn f() { one(); } + ", + expect![["\"one()\": Expected 1 argument, found 0\n"]], + ); + + check_no_diagnostic( + r" + fn one(arg: u8) {} + fn f() { one(1); } + ", + ); + } + + #[test] + fn method_as_fn() { + check_diagnostic( + r" + struct S; + impl S { + fn method(&self) {} + } + + fn f() { + S::method(); + } + ", + expect![["\"S::method()\": Expected 1 argument, found 0\n"]], + ); + + check_no_diagnostic( + r" + struct S; + impl S { + fn method(&self) {} + } + + fn f() { + S::method(&S); + S.method(); + } + ", + ); + } + + #[test] + fn method_with_arg() { + check_diagnostic( + r" + struct S; + impl S { + fn method(&self, arg: u8) {} + } + + fn f() { + S.method(); + } + ", + expect![["\"S.method()\": Expected 1 argument, found 0\n"]], + ); + + check_no_diagnostic( + r" + struct S; + impl S { + fn method(&self, arg: u8) {} + } + + fn f() { + S::method(&S, 0); + S.method(1); + } + ", + ); + } +} diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 00f6bb1864..e69e9b4ec5 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -99,14 +99,6 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec fix, }) }) - .on::(|d| { - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: None, - }) - }) .on::(|d| { let node = d.ast(db); let replacement = format!("Ok({})", node.syntax());