From ace693585dcc9086c284a3953b868c7ec314cc80 Mon Sep 17 00:00:00 2001 From: Zak <19464008+zaksabeast@users.noreply.github.com> Date: Thu, 30 Nov 2023 14:51:08 -0600 Subject: [PATCH] Improve validation errors for input values (#811, #693) Co-authored-by: Kai Ren --- juniper/CHANGELOG.md | 3 + juniper/src/executor_tests/enums.rs | 2 +- juniper/src/executor_tests/variables.rs | 7 +- juniper/src/types/utilities.rs | 150 ++++++++--- .../rules/arguments_of_correct_type.rs | 241 +++++++++--------- .../rules/default_values_of_correct_type.rs | 122 +++++---- tests/integration/tests/array.rs | 45 ++-- .../tests/codegen_input_object_derive.rs | 3 +- 8 files changed, 352 insertions(+), 221 deletions(-) diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md index 0962c04b..da0d0c94 100644 --- a/juniper/CHANGELOG.md +++ b/juniper/CHANGELOG.md @@ -78,6 +78,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi - Made `GraphQLRequest` fields public. ([#750]) - Relaxed [object safety] requirement for `GraphQLValue` and `GraphQLValueAsync` traits. ([ba1ed85b]) - Updated [GraphQL Playground] to 1.7.28 version. ([#1190]) +- Improve validation errors for input values. ([#811], [#693]) ## Fixed @@ -94,8 +95,10 @@ All user visible changes to `juniper` crate will be documented in this file. Thi [#456]: /../../issues/456 [#503]: /../../issues/503 [#528]: /../../issues/528 +[#693]: /../../issues/693 [#750]: /../../issues/750 [#798]: /../../issues/798 +[#811]: /../../pull/811 [#918]: /../../issues/918 [#965]: /../../pull/965 [#966]: /../../pull/966 diff --git a/juniper/src/executor_tests/enums.rs b/juniper/src/executor_tests/enums.rs index 34e10acd..1b1d730a 100644 --- a/juniper/src/executor_tests/enums.rs +++ b/juniper/src/executor_tests/enums.rs @@ -100,7 +100,7 @@ async fn does_not_accept_string_literals() { assert_eq!( error, ValidationError(vec![RuleError::new( - r#"Invalid value for argument "color", expected type "Color!""#, + r#"Invalid value for argument "color", reason: Invalid value ""RED"" for enum "Color""#, &[SourcePosition::new(18, 0, 18)], )]) ); diff --git a/juniper/src/executor_tests/variables.rs b/juniper/src/executor_tests/variables.rs index db5422b9..c0ad27a9 100644 --- a/juniper/src/executor_tests/variables.rs +++ b/juniper/src/executor_tests/variables.rs @@ -916,7 +916,8 @@ async fn does_not_allow_missing_required_field() { assert_eq!( error, ValidationError(vec![RuleError::new( - r#"Invalid value for argument "arg", expected type "ExampleInputObject!""#, + "Invalid value for argument \"arg\", \ + reason: \"ExampleInputObject\" is missing fields: \"b\"", &[SourcePosition::new(20, 0, 20)], )]), ); @@ -940,7 +941,9 @@ async fn does_not_allow_null_in_required_field() { assert_eq!( error, ValidationError(vec![RuleError::new( - r#"Invalid value for argument "arg", expected type "ExampleInputObject!""#, + "Invalid value for argument \"arg\", \ + reason: Error on \"ExampleInputObject\" field \"b\": \ + \"null\" specified for not nullable type \"Int!\"", &[SourcePosition::new(20, 0, 20)], )]), ); diff --git a/juniper/src/types/utilities.rs b/juniper/src/types/utilities.rs index e8128eea..a4f5c45b 100644 --- a/juniper/src/types/utilities.rs +++ b/juniper/src/types/utilities.rs @@ -1,48 +1,126 @@ +use std::{collections::HashSet, iter::Iterator}; + use crate::{ ast::InputValue, schema::{ - meta::{EnumMeta, InputObjectMeta, MetaType}, + meta::{Argument, EnumMeta, InputObjectMeta, MetaType}, model::{SchemaType, TypeType}, }, value::ScalarValue, }; -use std::collections::HashSet; -pub fn is_valid_literal_value( +/// Common error messages used in validation and execution of GraphQL operations +pub(crate) mod error { + use std::fmt::Display; + + pub(crate) fn non_null(arg_type: impl Display) -> String { + format!("\"null\" specified for not nullable type \"{arg_type}\"") + } + + pub(crate) fn enum_value(arg_value: impl Display, arg_type: impl Display) -> String { + format!("Invalid value \"{arg_value}\" for enum \"{arg_type}\"") + } + + pub(crate) fn type_value(arg_value: impl Display, arg_type: impl Display) -> String { + format!("Invalid value \"{arg_value}\" for type \"{arg_type}\"") + } + + pub(crate) fn parser(arg_type: impl Display, msg: impl Display) -> String { + format!("Parser error for \"{arg_type}\": {msg}") + } + + pub(crate) fn not_input_object(arg_type: impl Display) -> String { + format!("\"{arg_type}\" is not an input object") + } + + pub(crate) fn field( + arg_type: impl Display, + field_name: impl Display, + error_message: impl Display, + ) -> String { + format!("Error on \"{arg_type}\" field \"{field_name}\": {error_message}") + } + + pub(crate) fn missing_fields(arg_type: impl Display, missing_fields: impl Display) -> String { + format!("\"{arg_type}\" is missing fields: {missing_fields}") + } + + pub(crate) fn unknown_field(arg_type: impl Display, field_name: impl Display) -> String { + format!("Field \"{field_name}\" does not exist on type \"{arg_type}\"") + } + + pub(crate) fn invalid_list_length( + arg_value: impl Display, + actual: usize, + expected: usize, + ) -> String { + format!("Expected list of length {expected}, but \"{arg_value}\" has length {actual}") + } +} + +/// Validates the specified field of a GraphQL object and returns an error message if the field is +/// invalid. +fn validate_object_field( + schema: &SchemaType, + object_type: &TypeType, + object_fields: &[Argument], + field_value: &InputValue, + field_key: &str, +) -> Option +where + S: ScalarValue, +{ + let field_type = object_fields + .iter() + .filter(|f| f.name == field_key) + .map(|f| schema.make_type(&f.arg_type)) + .next(); + + if let Some(field_arg_type) = field_type { + let error_message = validate_literal_value(schema, &field_arg_type, field_value); + + error_message.map(|m| error::field(object_type, field_key, m)) + } else { + Some(error::unknown_field(object_type, field_key)) + } +} + +/// Validates the specified GraphQL literal and returns an error message if the it's invalid. +pub fn validate_literal_value( schema: &SchemaType, arg_type: &TypeType, arg_value: &InputValue, -) -> bool +) -> Option where S: ScalarValue, { match *arg_type { TypeType::NonNull(ref inner) => { if arg_value.is_null() { - false + Some(error::non_null(arg_type)) } else { - is_valid_literal_value(schema, inner, arg_value) + validate_literal_value(schema, inner, arg_value) } } TypeType::List(ref inner, expected_size) => match *arg_value { - InputValue::Null | InputValue::Variable(_) => true, + InputValue::Null | InputValue::Variable(_) => None, InputValue::List(ref items) => { if let Some(expected) = expected_size { if items.len() != expected { - return false; + return Some(error::invalid_list_length(arg_value, items.len(), expected)); } } items .iter() - .all(|i| is_valid_literal_value(schema, inner, &i.item)) + .find_map(|i| validate_literal_value(schema, inner, &i.item)) } ref v => { if let Some(expected) = expected_size { if expected != 1 { - return false; + return Some(error::invalid_list_length(arg_value, 1, expected)); } } - is_valid_literal_value(schema, inner, v) + validate_literal_value(schema, inner, v) } }, TypeType::Concrete(t) => { @@ -51,19 +129,23 @@ where if let (&InputValue::Scalar(_), Some(&MetaType::Enum(EnumMeta { .. }))) = (arg_value, arg_type.to_concrete()) { - return false; + return Some(error::enum_value(arg_value, arg_type)); } match *arg_value { - InputValue::Null | InputValue::Variable(_) => true, + InputValue::Null | InputValue::Variable(_) => None, ref v @ InputValue::Scalar(_) | ref v @ InputValue::Enum(_) => { if let Some(parse_fn) = t.input_value_parse_fn() { - parse_fn(v).is_ok() + if parse_fn(v).is_ok() { + None + } else { + Some(error::type_value(arg_value, arg_type)) + } } else { - false + Some(error::parser(arg_type, "no parser present")) } } - InputValue::List(_) => false, + InputValue::List(_) => Some("Input lists are not literals".to_owned()), InputValue::Object(ref obj) => { if let MetaType::InputObject(InputObjectMeta { ref input_fields, .. @@ -77,23 +159,33 @@ where }) .collect::>(); - let all_types_ok = obj.iter().all(|(key, value)| { + let error_message = obj.iter().find_map(|(key, value)| { remaining_required_fields.remove(&key.item); - if let Some(ref arg_type) = input_fields - .iter() - .filter(|f| f.name == key.item) - .map(|f| schema.make_type(&f.arg_type)) - .next() - { - is_valid_literal_value(schema, arg_type, &value.item) - } else { - false - } + validate_object_field( + schema, + arg_type, + input_fields, + &value.item, + &key.item, + ) }); - all_types_ok && remaining_required_fields.is_empty() + if error_message.is_some() { + return error_message; + } + + if remaining_required_fields.is_empty() { + None + } else { + let missing_fields = remaining_required_fields + .into_iter() + .map(|s| format!("\"{}\"", &**s)) + .collect::>() + .join(", "); + Some(error::missing_fields(arg_type, missing_fields)) + } } else { - false + Some(error::not_input_object(arg_type)) } } } diff --git a/juniper/src/validation/rules/arguments_of_correct_type.rs b/juniper/src/validation/rules/arguments_of_correct_type.rs index 424b8027..4524cf8d 100644 --- a/juniper/src/validation/rules/arguments_of_correct_type.rs +++ b/juniper/src/validation/rules/arguments_of_correct_type.rs @@ -4,7 +4,7 @@ use crate::{ ast::{Directive, Field, InputValue}, parser::Spanning, schema::meta::Argument, - types::utilities::is_valid_literal_value, + types::utilities::validate_literal_value, validation::{ValidatorContext, Visitor}, value::ScalarValue, }; @@ -58,18 +58,15 @@ where { let meta_type = ctx.schema.make_type(&argument_meta.arg_type); - if !is_valid_literal_value(ctx.schema, &meta_type, &arg_value.item) { - ctx.report_error( - &error_message(arg_name.item, &argument_meta.arg_type), - &[arg_value.span.start], - ); + if let Some(err) = validate_literal_value(ctx.schema, &meta_type, &arg_value.item) { + ctx.report_error(&error_message(arg_name.item, err), &[arg_value.span.start]); } } } } -fn error_message(arg_name: impl fmt::Display, type_name: impl fmt::Display) -> String { - format!("Invalid value for argument \"{arg_name}\", expected type \"{type_name}\"",) +fn error_message(arg_name: impl fmt::Display, msg: impl fmt::Display) -> String { + format!("Invalid value for argument \"{arg_name}\", reason: {msg}") } #[cfg(test)] @@ -78,6 +75,7 @@ mod tests { use crate::{ parser::SourcePosition, + types::utilities::error, validation::{expect_fails_rule, expect_passes_rule, RuleError}, value::DefaultScalarValue, }; @@ -92,7 +90,7 @@ mod tests { intArgField(intArg: null) } } - "#, + "#, ); } @@ -106,7 +104,7 @@ mod tests { stringListArgField(stringListArg: null) } } - "#, + "#, ); } @@ -120,9 +118,9 @@ mod tests { nonNullIntArgField(nonNullIntArg: null) } } - "#, + "#, &[RuleError::new( - &error_message("nonNullIntArg", "Int!"), + &error_message("nonNullIntArg", error::non_null("Int!")), &[SourcePosition::new(97, 3, 50)], )], ); @@ -138,9 +136,9 @@ mod tests { nonNullStringListArgField(nonNullStringListArg: null) } } - "#, + "#, &[RuleError::new( - &error_message("nonNullStringListArg", "[String!]!"), + &error_message("nonNullStringListArg", error::non_null("[String!]!")), &[SourcePosition::new(111, 3, 64)], )], ); @@ -156,7 +154,7 @@ mod tests { intArgField(intArg: 2) } } - "#, + "#, ); } @@ -170,7 +168,7 @@ mod tests { booleanArgField(booleanArg: true) } } - "#, + "#, ); } @@ -184,7 +182,7 @@ mod tests { stringArgField(stringArg: "foo") } } - "#, + "#, ); } @@ -198,7 +196,7 @@ mod tests { floatArgField(floatArg: 1.1) } } - "#, + "#, ); } @@ -212,7 +210,7 @@ mod tests { floatArgField(floatArg: 1) } } - "#, + "#, ); } @@ -226,7 +224,7 @@ mod tests { idArgField(idArg: 1) } } - "#, + "#, ); } @@ -240,7 +238,7 @@ mod tests { idArgField(idArg: "someIdString") } } - "#, + "#, ); } @@ -254,7 +252,7 @@ mod tests { doesKnowCommand(dogCommand: SIT) } } - "#, + "#, ); } @@ -268,9 +266,9 @@ mod tests { stringArgField(stringArg: 1) } } - "#, + "#, &[RuleError::new( - &error_message("stringArg", "String"), + &error_message("stringArg", error::type_value("1", "String")), &[SourcePosition::new(89, 3, 42)], )], ); @@ -286,9 +284,9 @@ mod tests { stringArgField(stringArg: 1.0) } } - "#, + "#, &[RuleError::new( - &error_message("stringArg", "String"), + &error_message("stringArg", error::type_value("1", "String")), &[SourcePosition::new(89, 3, 42)], )], ); @@ -304,9 +302,9 @@ mod tests { stringArgField(stringArg: true) } } - "#, + "#, &[RuleError::new( - &error_message("stringArg", "String"), + &error_message("stringArg", error::type_value("true", "String")), &[SourcePosition::new(89, 3, 42)], )], ); @@ -322,9 +320,9 @@ mod tests { stringArgField(stringArg: BAR) } } - "#, + "#, &[RuleError::new( - &error_message("stringArg", "String"), + &error_message("stringArg", error::type_value("BAR", "String")), &[SourcePosition::new(89, 3, 42)], )], ); @@ -340,9 +338,9 @@ mod tests { intArgField(intArg: "3") } } - "#, + "#, &[RuleError::new( - &error_message("intArg", "Int"), + &error_message("intArg", error::type_value("\"3\"", "Int")), &[SourcePosition::new(83, 3, 36)], )], ); @@ -358,9 +356,9 @@ mod tests { intArgField(intArg: FOO) } } - "#, + "#, &[RuleError::new( - &error_message("intArg", "Int"), + &error_message("intArg", error::type_value("FOO", "Int")), &[SourcePosition::new(83, 3, 36)], )], ); @@ -376,9 +374,9 @@ mod tests { intArgField(intArg: 3.0) } } - "#, + "#, &[RuleError::new( - &error_message("intArg", "Int"), + &error_message("intArg", error::type_value("3", "Int")), &[SourcePosition::new(83, 3, 36)], )], ); @@ -394,9 +392,9 @@ mod tests { intArgField(intArg: 3.333) } } - "#, + "#, &[RuleError::new( - &error_message("intArg", "Int"), + &error_message("intArg", error::type_value("3.333", "Int")), &[SourcePosition::new(83, 3, 36)], )], ); @@ -412,9 +410,9 @@ mod tests { floatArgField(floatArg: "3.333") } } - "#, + "#, &[RuleError::new( - &error_message("floatArg", "Float"), + &error_message("floatArg", error::type_value("\"3.333\"", "Float")), &[SourcePosition::new(87, 3, 40)], )], ); @@ -430,9 +428,9 @@ mod tests { floatArgField(floatArg: true) } } - "#, + "#, &[RuleError::new( - &error_message("floatArg", "Float"), + &error_message("floatArg", error::type_value("true", "Float")), &[SourcePosition::new(87, 3, 40)], )], ); @@ -448,9 +446,9 @@ mod tests { floatArgField(floatArg: FOO) } } - "#, + "#, &[RuleError::new( - &error_message("floatArg", "Float"), + &error_message("floatArg", error::type_value("FOO", "Float")), &[SourcePosition::new(87, 3, 40)], )], ); @@ -466,9 +464,9 @@ mod tests { booleanArgField(booleanArg: 2) } } - "#, + "#, &[RuleError::new( - &error_message("booleanArg", "Boolean"), + &error_message("booleanArg", error::type_value("2", "Boolean")), &[SourcePosition::new(91, 3, 44)], )], ); @@ -484,9 +482,9 @@ mod tests { booleanArgField(booleanArg: 1.0) } } - "#, + "#, &[RuleError::new( - &error_message("booleanArg", "Boolean"), + &error_message("booleanArg", error::type_value("1", "Boolean")), &[SourcePosition::new(91, 3, 44)], )], ); @@ -502,9 +500,9 @@ mod tests { booleanArgField(booleanArg: "true") } } - "#, + "#, &[RuleError::new( - &error_message("booleanArg", "Boolean"), + &error_message("booleanArg", error::type_value("\"true\"", "Boolean")), &[SourcePosition::new(91, 3, 44)], )], ); @@ -520,9 +518,9 @@ mod tests { booleanArgField(booleanArg: TRUE) } } - "#, + "#, &[RuleError::new( - &error_message("booleanArg", "Boolean"), + &error_message("booleanArg", error::type_value("TRUE", "Boolean")), &[SourcePosition::new(91, 3, 44)], )], ); @@ -538,9 +536,9 @@ mod tests { idArgField(idArg: 1.0) } } - "#, + "#, &[RuleError::new( - &error_message("idArg", "ID"), + &error_message("idArg", error::type_value("1", "ID")), &[SourcePosition::new(81, 3, 34)], )], ); @@ -556,9 +554,9 @@ mod tests { idArgField(idArg: true) } } - "#, + "#, &[RuleError::new( - &error_message("idArg", "ID"), + &error_message("idArg", error::type_value("true", "ID")), &[SourcePosition::new(81, 3, 34)], )], ); @@ -574,9 +572,9 @@ mod tests { idArgField(idArg: SOMETHING) } } - "#, + "#, &[RuleError::new( - &error_message("idArg", "ID"), + &error_message("idArg", error::type_value("SOMETHING", "ID")), &[SourcePosition::new(81, 3, 34)], )], ); @@ -592,9 +590,9 @@ mod tests { doesKnowCommand(dogCommand: 2) } } - "#, + "#, &[RuleError::new( - &error_message("dogCommand", "DogCommand"), + &error_message("dogCommand", error::enum_value("2", "DogCommand")), &[SourcePosition::new(79, 3, 44)], )], ); @@ -610,9 +608,9 @@ mod tests { doesKnowCommand(dogCommand: 1.0) } } - "#, + "#, &[RuleError::new( - &error_message("dogCommand", "DogCommand"), + &error_message("dogCommand", error::enum_value("1", "DogCommand")), &[SourcePosition::new(79, 3, 44)], )], ); @@ -628,9 +626,9 @@ mod tests { doesKnowCommand(dogCommand: "SIT") } } - "#, + "#, &[RuleError::new( - &error_message("dogCommand", "DogCommand"), + &error_message("dogCommand", error::enum_value("\"SIT\"", "DogCommand")), &[SourcePosition::new(79, 3, 44)], )], ); @@ -646,9 +644,9 @@ mod tests { doesKnowCommand(dogCommand: true) } } - "#, + "#, &[RuleError::new( - &error_message("dogCommand", "DogCommand"), + &error_message("dogCommand", error::enum_value("true", "DogCommand")), &[SourcePosition::new(79, 3, 44)], )], ); @@ -664,9 +662,9 @@ mod tests { doesKnowCommand(dogCommand: JUGGLE) } } - "#, + "#, &[RuleError::new( - &error_message("dogCommand", "DogCommand"), + &error_message("dogCommand", error::type_value("JUGGLE", "DogCommand")), &[SourcePosition::new(79, 3, 44)], )], ); @@ -682,9 +680,9 @@ mod tests { doesKnowCommand(dogCommand: sit) } } - "#, + "#, &[RuleError::new( - &error_message("dogCommand", "DogCommand"), + &error_message("dogCommand", error::type_value("sit", "DogCommand")), &[SourcePosition::new(79, 3, 44)], )], ); @@ -700,7 +698,7 @@ mod tests { stringListArgField(stringListArg: ["one", "two"]) } } - "#, + "#, ); } @@ -714,7 +712,7 @@ mod tests { stringListArgField(stringListArg: []) } } - "#, + "#, ); } @@ -728,7 +726,7 @@ mod tests { stringListArgField(stringListArg: "one") } } - "#, + "#, ); } @@ -742,9 +740,9 @@ mod tests { stringListArgField(stringListArg: ["one", 2]) } } - "#, + "#, &[RuleError::new( - &error_message("stringListArg", "[String]"), + &error_message("stringListArg", error::type_value("2", "String")), &[SourcePosition::new(97, 3, 50)], )], ); @@ -760,9 +758,9 @@ mod tests { stringListArgField(stringListArg: 1) } } - "#, + "#, &[RuleError::new( - &error_message("stringListArg", "[String]"), + &error_message("stringListArg", error::type_value("1", "String")), &[SourcePosition::new(97, 3, 50)], )], ); @@ -778,7 +776,7 @@ mod tests { isHousetrained(atOtherHomes: true) } } - "#, + "#, ); } @@ -792,7 +790,7 @@ mod tests { isHousetrained } } - "#, + "#, ); } @@ -806,7 +804,7 @@ mod tests { multipleReqs(req1: 1, req2: 2) } } - "#, + "#, ); } @@ -820,7 +818,7 @@ mod tests { multipleReqs(req2: 2, req1: 1) } } - "#, + "#, ); } @@ -834,7 +832,7 @@ mod tests { multipleOpts } } - "#, + "#, ); } @@ -848,7 +846,7 @@ mod tests { multipleOpts(opt1: 1) } } - "#, + "#, ); } @@ -862,7 +860,7 @@ mod tests { multipleOpts(opt2: 1) } } - "#, + "#, ); } @@ -876,7 +874,7 @@ mod tests { multipleOptAndReq(req1: 3, req2: 4) } } - "#, + "#, ); } @@ -890,7 +888,7 @@ mod tests { multipleOptAndReq(req1: 3, req2: 4, opt1: 5) } } - "#, + "#, ); } @@ -904,7 +902,7 @@ mod tests { multipleOptAndReq(req1: 3, req2: 4, opt1: 5, opt2: 6) } } - "#, + "#, ); } @@ -918,14 +916,14 @@ mod tests { multipleReqs(req2: "two", req1: "one") } } - "#, + "#, &[ RuleError::new( - &error_message("req2", "Int!"), + &error_message("req2", error::type_value("\"two\"", "Int")), &[SourcePosition::new(82, 3, 35)], ), RuleError::new( - &error_message("req1", "Int!"), + &error_message("req1", error::type_value("\"one\"", "Int")), &[SourcePosition::new(95, 3, 48)], ), ], @@ -942,9 +940,9 @@ mod tests { multipleReqs(req1: "one") } } - "#, + "#, &[RuleError::new( - &error_message("req1", "Int!"), + &error_message("req1", error::type_value("\"one\"", "Int")), &[SourcePosition::new(82, 3, 35)], )], ); @@ -960,7 +958,7 @@ mod tests { complexArgField } } - "#, + "#, ); } @@ -974,7 +972,7 @@ mod tests { complexArgField(complexArg: { requiredField: true }) } } - "#, + "#, ); } @@ -988,7 +986,7 @@ mod tests { complexArgField(complexArg: { requiredField: false }) } } - "#, + "#, ); } @@ -1002,7 +1000,7 @@ mod tests { complexArgField(complexArg: { requiredField: true, intField: 4 }) } } - "#, + "#, ); } @@ -1022,7 +1020,7 @@ mod tests { }) } } - "#, + "#, ); } @@ -1042,7 +1040,7 @@ mod tests { }) } } - "#, + "#, ); } @@ -1056,9 +1054,12 @@ mod tests { complexArgField(complexArg: { intField: 4 }) } } - "#, + "#, &[RuleError::new( - &error_message("complexArg", "ComplexInput"), + &error_message( + "complexArg", + error::missing_fields("ComplexInput", "\"requiredField\""), + ), &[SourcePosition::new(91, 3, 44)], )], ); @@ -1077,9 +1078,16 @@ mod tests { }) } } - "#, + "#, &[RuleError::new( - &error_message("complexArg", "ComplexInput"), + &error_message( + "complexArg", + error::field( + "ComplexInput", + "stringListField", + error::type_value("2", "String"), + ), + ), &[SourcePosition::new(91, 3, 44)], )], ); @@ -1098,9 +1106,12 @@ mod tests { }) } } - "#, + "#, &[RuleError::new( - &error_message("complexArg", "ComplexInput"), + &error_message( + "complexArg", + error::unknown_field("ComplexInput", "unknownField"), + ), &[SourcePosition::new(91, 3, 44)], )], ); @@ -1119,7 +1130,7 @@ mod tests { name } } - "#, + "#, ); } @@ -1128,20 +1139,20 @@ mod tests { expect_fails_rule::<_, _, DefaultScalarValue>( factory, r#" - { - dog @include(if: "yes") { - name @skip(if: ENUM) - } - } - "#, + { + dog @include(if: "yes") { + name @skip(if: ENUM) + } + } + "#, &[ RuleError::new( - &error_message("if", "Boolean!"), - &[SourcePosition::new(38, 2, 27)], + &error_message("if", error::type_value("\"yes\"", "Boolean")), + &[SourcePosition::new(46, 2, 31)], ), RuleError::new( - &error_message("if", "Boolean!"), - &[SourcePosition::new(74, 3, 27)], + &error_message("if", error::type_value("ENUM", "Boolean")), + &[SourcePosition::new(86, 3, 31)], ), ], ); diff --git a/juniper/src/validation/rules/default_values_of_correct_type.rs b/juniper/src/validation/rules/default_values_of_correct_type.rs index a1a648ff..bc345897 100644 --- a/juniper/src/validation/rules/default_values_of_correct_type.rs +++ b/juniper/src/validation/rules/default_values_of_correct_type.rs @@ -3,7 +3,7 @@ use std::fmt; use crate::{ ast::VariableDefinition, parser::Spanning, - types::utilities::is_valid_literal_value, + types::utilities::validate_literal_value, validation::{ValidatorContext, Visitor}, value::ScalarValue, }; @@ -36,9 +36,9 @@ where } else { let meta_type = ctx.schema.make_type(&var_def.var_type.item); - if !is_valid_literal_value(ctx.schema, &meta_type, var_value) { + if let Some(err) = validate_literal_value(ctx.schema, &meta_type, var_value) { ctx.report_error( - &type_error_message(var_name.item, &var_def.var_type.item), + &type_error_message(var_name.item, &var_def.var_type.item, err), &[span.start], ); } @@ -47,8 +47,15 @@ where } } -fn type_error_message(arg_name: impl fmt::Display, type_name: impl fmt::Display) -> String { - format!("Invalid default value for argument \"{arg_name}\", expected type \"{type_name}\"") +fn type_error_message( + arg_name: impl fmt::Display, + type_name: impl fmt::Display, + reason: impl fmt::Display, +) -> String { + format!( + "Invalid default value for argument \"{arg_name}\", expected type \"{type_name}\", \ + reason: {reason}", + ) } fn non_null_error_message(arg_name: impl fmt::Display, type_name: impl fmt::Display) -> String { @@ -64,6 +71,7 @@ mod tests { use crate::{ parser::SourcePosition, + types::utilities::error, validation::{expect_fails_rule, expect_passes_rule, RuleError}, value::DefaultScalarValue, }; @@ -73,10 +81,10 @@ mod tests { expect_passes_rule::<_, _, DefaultScalarValue>( factory, r#" - query NullableValues($a: Int, $b: String, $c: ComplexInput) { - dog { name } - } - "#, + query NullableValues($a: Int, $b: String, $c: ComplexInput) { + dog { name } + } + "#, ); } @@ -85,10 +93,10 @@ mod tests { expect_passes_rule::<_, _, DefaultScalarValue>( factory, r#" - query RequiredValues($a: Int!, $b: String!) { - dog { name } - } - "#, + query RequiredValues($a: Int!, $b: String!) { + dog { name } + } + "#, ); } @@ -97,14 +105,14 @@ mod tests { expect_passes_rule::<_, _, DefaultScalarValue>( factory, r#" - query WithDefaultValues( - $a: Int = 1, - $b: String = "ok", - $c: ComplexInput = { requiredField: true, intField: 3 } - ) { - dog { name } - } - "#, + query WithDefaultValues( + $a: Int = 1, + $b: String = "ok", + $c: ComplexInput = { requiredField: true, intField: 3 } + ) { + dog { name } + } + "#, ); } @@ -113,18 +121,18 @@ mod tests { expect_fails_rule::<_, _, DefaultScalarValue>( factory, r#" - query UnreachableDefaultValues($a: Int! = 3, $b: String! = "default") { - dog { name } - } - "#, + query UnreachableDefaultValues($a: Int! = 3, $b: String! = "default") { + dog { name } + } + "#, &[ RuleError::new( &non_null_error_message("a", "Int!"), - &[SourcePosition::new(53, 1, 52)], + &[SourcePosition::new(55, 1, 54)], ), RuleError::new( &non_null_error_message("b", "String!"), - &[SourcePosition::new(70, 1, 69)], + &[SourcePosition::new(72, 1, 71)], ), ], ); @@ -135,26 +143,30 @@ mod tests { expect_fails_rule::<_, _, DefaultScalarValue>( factory, r#" - query InvalidDefaultValues( - $a: Int = "one", - $b: String = 4, - $c: ComplexInput = "notverycomplex" - ) { - dog { name } - } - "#, + query InvalidDefaultValues( + $a: Int = "one", + $b: String = 4, + $c: ComplexInput = "notverycomplex" + ) { + dog { name } + } + "#, &[ RuleError::new( - &type_error_message("a", "Int"), - &[SourcePosition::new(61, 2, 22)], + &type_error_message("a", "Int", error::type_value("\"one\"", "Int")), + &[SourcePosition::new(67, 2, 26)], ), RuleError::new( - &type_error_message("b", "String"), - &[SourcePosition::new(93, 3, 25)], + &type_error_message("b", "String", error::type_value("4", "String")), + &[SourcePosition::new(103, 3, 29)], ), RuleError::new( - &type_error_message("c", "ComplexInput"), - &[SourcePosition::new(127, 4, 31)], + &type_error_message( + "c", + "ComplexInput", + error::type_value("\"notverycomplex\"", "ComplexInput"), + ), + &[SourcePosition::new(141, 4, 35)], ), ], ); @@ -165,13 +177,17 @@ mod tests { expect_fails_rule::<_, _, DefaultScalarValue>( factory, r#" - query MissingRequiredField($a: ComplexInput = {intField: 3}) { - dog { name } - } - "#, + query MissingRequiredField($a: ComplexInput = {intField: 3}) { + dog { name } + } + "#, &[RuleError::new( - &type_error_message("a", "ComplexInput"), - &[SourcePosition::new(57, 1, 56)], + &type_error_message( + "a", + "ComplexInput", + error::missing_fields("ComplexInput", "\"requiredField\""), + ), + &[SourcePosition::new(59, 1, 58)], )], ); } @@ -181,13 +197,13 @@ mod tests { expect_fails_rule::<_, _, DefaultScalarValue>( factory, r#" - query InvalidItem($a: [String] = ["one", 2]) { - dog { name } - } - "#, + query InvalidItem($a: [String] = ["one", 2]) { + dog { name } + } + "#, &[RuleError::new( - &type_error_message("a", "[String]"), - &[SourcePosition::new(44, 1, 43)], + &type_error_message("a", "[String]", error::type_value("2", "String")), + &[SourcePosition::new(46, 1, 45)], )], ); } diff --git a/tests/integration/tests/array.rs b/tests/integration/tests/array.rs index 1686e519..4783be1d 100644 --- a/tests/integration/tests/array.rs +++ b/tests/integration/tests/array.rs @@ -87,11 +87,12 @@ mod as_input_field { let schema = RootNode::new(Query, EmptyMutation::new(), EmptySubscription::new()); let res = juniper::execute(query, None, &schema, &graphql_vars! {}, &()).await; - assert!(res.is_err()); - assert!(res - .unwrap_err() - .to_string() - .contains(r#"Invalid value for argument "input", expected type "Input!""#)); + assert!(res.is_err(), "result succeeded: {res:#?}"); + assert_eq!( + res.unwrap_err().to_string(), + "Invalid value for argument \"input\", reason: Error on \"Input\" field \"two\": \ + Expected list of length 2, but \"[true, true, false]\" has length 3. At 2:30\n", + ); } #[tokio::test] @@ -105,11 +106,12 @@ mod as_input_field { let schema = RootNode::new(Query, EmptyMutation::new(), EmptySubscription::new()); let res = juniper::execute(query, None, &schema, &graphql_vars! {}, &()).await; - assert!(res.is_err()); - assert!(res - .unwrap_err() - .to_string() - .contains(r#"Invalid value for argument "input", expected type "Input!""#)); + assert!(res.is_err(), "result succeeded: {res:#?}"); + assert_eq!( + res.unwrap_err().to_string(), + "Invalid value for argument \"input\", reason: Error on \"Input\" field \"two\": \ + Expected list of length 2, but \"true\" has length 1. At 2:30\n", + ); } #[tokio::test] @@ -178,11 +180,12 @@ mod as_input_argument { let schema = RootNode::new(Query, EmptyMutation::new(), EmptySubscription::new()); let res = juniper::execute(query, None, &schema, &graphql_vars! {}, &()).await; - assert!(res.is_err()); - assert!(res - .unwrap_err() - .to_string() - .contains(r#"Invalid value for argument "input", expected type "[Boolean!]!""#)); + assert!(res.is_err(), "result succeeded: {res:#?}"); + assert_eq!( + res.unwrap_err().to_string(), + "Invalid value for argument \"input\", reason: Expected list of length 2, \ + but \"[true, true, false]\" has length 3. At 2:30\n", + ); } #[tokio::test] @@ -196,11 +199,13 @@ mod as_input_argument { let schema = RootNode::new(Query, EmptyMutation::new(), EmptySubscription::new()); let res = juniper::execute(query, None, &schema, &graphql_vars! {}, &()).await; - assert!(res.is_err()); - assert!(res - .unwrap_err() - .to_string() - .contains(r#"Invalid value for argument "input", expected type "[Boolean!]!""#)); + assert!(res.is_err(), "result succeeded: {res:#?}"); + assert_eq!( + res.unwrap_err().to_string(), + "Invalid value for argument \"input\", reason: Expected list of length 2, \ + but \"true\" has length 1. At 2:30\n", + "invalid error returned", + ); } #[tokio::test] diff --git a/tests/integration/tests/codegen_input_object_derive.rs b/tests/integration/tests/codegen_input_object_derive.rs index fe9bac70..0cff21a4 100644 --- a/tests/integration/tests/codegen_input_object_derive.rs +++ b/tests/integration/tests/codegen_input_object_derive.rs @@ -189,7 +189,8 @@ mod default_value { assert_eq!( execute(DOC, None, &schema, &graphql_vars! {}, &()).await, Err(GraphQLError::ValidationError(vec![RuleError::new( - "Invalid value for argument \"point\", expected type \"Point2D!\"", + "Invalid value for argument \"point\", reason: Error on \"Point2D\" field \"y\": \ + \"null\" specified for not nullable type \"Float!\"", &[SourcePosition::new(11, 0, 11)], )])) );