Query/Schema validation for default values on object type field arguments not captured (ValueOfCorrectType rule)

[maloku]

From the documentation https://sangria-graphql.github.io/learn/#query-validation

Query validation can also be used for SDL validation. ...

Regarding documentation, I expected the following validation to fail, but it is passing:

// test can be added to sangria.validation.rules.ValuesOfCorrectTypeSpec
"Type field arguments default values" should {
    "fail when wrong field name" in {

      val Success(doc) = QueryParser.parse(
        """
          |input A {
          | f1: Int
          | f2: Float
          |}
          |""".stripMargin
      )
      val stubSchema = Schema.buildStubFromAst(doc)
      val Success(docUnderTest) = QueryParser.parse(
        """
          |type Test {
          | f(a: A = {wrong: 50}): String
          |}
          |""".stripMargin
      )
      val violations = validator(defaultRule.toList).validateQuery(stubSchema, docUnderTest)
      violations shouldNot be(empty) // but it is empty
      violations.head shouldBe a[UnknownFieldViolation] 

    }
  }

I have spent some time recently in the library code, and I think the issue is in TypeInfo.scala as it does not push in the inputTypeStack the input type, which leads ctx.typeInfo.inputType to be None and ignore the checking altogether.

The following change in TypeInfo.scala fixes this issue, and I did not get any other failing tests. It does check the InputValueDefinition with defaultValue and adds the type into the stack, which later can be found by the rule.

  def enter(node: ast.AstNode) = {
    ancestorStack.push(node)

    node match {
      //code omitted
      case ast.InputValueDefinition(_, valueType, Some(_), _, _, _, _) =>
        inputTypeStack.push(schema.getInputType(valueType))
    }
  }
  def leave(node: ast.AstNode) = {
    node match {
      //code omitted
      case ast.InputValueDefinition(_, _, Some(_), _, _, _, _) => 
        inputTypeStack.pop()
    }

    ancestorStack.pop()
  }

It is very hard to get around this issue (or at least I could not find one) since all the stacks in this class are private; extending this class does not help. Adding public methods for pop and push to the stacks could be helpful if one wants to add specific validation rules if the default implementation does not handle specific AstNodes.

I am new to the library, but if you think this would be enough, I can PR this fix and add public methods for pop and push.

[yanns]

Thanks a lot for the investigation, and the time spent to explain the issue.

I'm not very familiar with this part of the code. I don't know if this change would have some strange side-effects.
I can propose the following:

• you can open a PR
• I'll try the change on some internal applications

WDYT?

[maloku]

Thanks for the prompt response. I will open a PR as soon as I have it.