Validation discrepancy between classpath model and imported model

[jdisanti]

When using the smithy-gradle-plugin to build/validate a model, if that model is pulled in via the classpath by existing in a model/ directory, then the validation errors for that model are different than if that model is explicitly imported via the smithy-build.json file.

Here's an example minimal model that should produce a DANGER error (it doesn't like teapots, unless something changes due to #1386):

$version: "1.0"
namespace aws.protocoltests.misc

use aws.protocols#restJson1
use smithy.test#httpRequestTests
use smithy.test#httpResponseTests

@restJson1
service MiscService {
    operations: [ResponseCodeRequiredOperation],
}

@http(method: "GET", uri: "/responseCodeRequiredOperation", code: 418)
operation ResponseCodeRequiredOperation {
    input: EmptyStructure,
    output: ResponseCodeRequiredOutput,
}

structure EmptyStructure {}

@output
structure ResponseCodeRequiredOutput {
    @required
    @httpResponseCode
    responseCode: Integer,
}

This model, if pulled in by existing in the models/ directory, emits a WARN due to the IDL version being 1, but does not emit the DANGER message. If instead, it is placed outside the models/ directory and referenced directly from the imports section of smithy-build.json, then the DANGER message is emitted and the build fails.

[david-perez]

then the DANGER message is emitted and the build fails.

Worth mentioning too that the WARN is not emitted, only the DANGER message.

[mtdowling]

I wasn't able to reproduce this by moving a model between an explicit import vs placing it in the model/ directory, but that might be because I don't understand the full setup of how to get this to fail.

You mentioned the models/ directory, but the Gradle plugin actually looks for the model/ directory. Is it possible that is the root cause of the issue here (that is just didn't load your model)? One quick way to check this kind of thing is note the number of shapes that are validated (e.g., you might see something likeFAILURE: Validated 299 shapes (DANGER: 1, WARNING: 2, NOTE: 2) output from the plugin).

One other consideration that may or may not be relevant here: when generating a JAR from a Gradle project, the Smithy Gradle plugin will validate that the inputs it's given are correct as a first pass at validation. It also then does another pass at validating using only the generated JAR and your implementation/api dependencies to ensure that the generated JAR is valid and doesn't, say, refer to unknown shapes.

Worth mentioning too that the WARN is not emitted, only the DANGER message.

We removed the IDL 1.0 warning since it was so noisy, so that might explain this.

[jdisanti]

Sorry, models/ (plural) is a typo. We're using a model/ directory.

[mtdowling]

Ah ok, good to know. When trying to reproduce this with Smithy (1.26.4) and the Gradle plugin (0.6.0 or HEAD), I see the DANGER event when your example model is an imports model or found in the model/ directory. The output is different though, which I don't think is a good DX. For example, when loaded via model/, I get the formatted text like:

DANGER: aws.protocoltests.misc#ResponseCodeRequiredOperation (HttpResponseCodeSemantics)
     @ /Users/dowling/projects/smithy-test-1387/model/foo.smithy
     |
  13 | @http(method: "GET", uri: "/responseCodeRequiredOperation", code: 418)
     | ^
     = Expected an `http` code in the 2xx range, but found 418

When loaded via smithy-build.json/imports, I get:

[DANGER] aws.protocoltests.misc#ResponseCodeRequiredOperation: Expected an `http` code in the 2xx range, but found 418 | HttpResponseCodeSemantics /Users/dowling/projects/smithy-test-1387/foo.smithy:13:1
[WARNING] aws.protocoltests.misc#ResponseCodeRequiredOperation: The output of this operation should target a shape that starts with the operation's name, 'ResponseCodeRequiredOperation', but the targeted shape is `aws.protocoltests.misc#ResponseCodeRequiredOutput` | OperationInputOutputName /Users/dowling/projects/smithy-test-1387/foo.smithy:14:1
[WARNING] aws.protocoltests.misc#ResponseCodeRequiredOperation: This operation uses the `GET` method in the `http` trait, but is not marked with the readonly trait | HttpMethodSemantics /Users/dowling/projects/smithy-test-1387/foo.smithy:13:1

Which still fails the build for me.

Here's my setup:

~/projects/smithy-test-1387〉tree
.
├── build.gradle.kts
├── model
│   └── foo.smithy
├── settings.gradle.kts
└── smithy-build.json

1 directory, 4 files

~/projects/smithy-test-1387〉cat smithy-build.json
{
  "version": "1.0",
  "imports": [
    //"foo.smithy"
  ]
}

~/projects/smithy-test-1387〉cat settings.gradle.kts
rootProject.name = "test-1387"

pluginManagement {
    repositories {
        gradlePluginPortal()
        mavenCentral()
    }
}

~/projects/smithy-test-1387〉cat build.gradle.kts
plugins {
    id("software.amazon.smithy").version("0.6.0")
}

repositories {
    mavenCentral()
}

dependencies {
    implementation("software.amazon.smithy:smithy-model:[1.0, 2.0[")
    implementation("software.amazon.smithy:smithy-aws-traits:[1.0, 2.0[")
    implementation("software.amazon.smithy:smithy-protocol-test-traits:[1.0, 2.0[")
}

~/projects/smithy-test-1387〉cat model/foo.smithy
$version: "1.0"
namespace aws.protocoltests.misc

use aws.protocols#restJson1
use smithy.test#httpRequestTests
use smithy.test#httpResponseTests

@restJson1
service MiscService {
    operations: [ResponseCodeRequiredOperation],
}

@http(method: "GET", uri: "/responseCodeRequiredOperation", code: 418)
operation ResponseCodeRequiredOperation {
    input: EmptyStructure,
    output: ResponseCodeRequiredOutput,
}

structure EmptyStructure {}

@output
structure ResponseCodeRequiredOutput {
    @required
    @httpResponseCode
    responseCode: Integer,
}

Are there other settings you're passing to the plugin?

[jdisanti]

I looked through our build.gradle.kts files and didn't see any additional settings for the plugin. Perhaps the situation has improved since this was originally filed.

I don't have time right this moment, but I can try to repro it again in smithy-rs with its upgraded Smithy version. We could also see if things have changed by trying out the above with an August release of Smithy.

[jdisanti]

I'm not able to repro this anymore in smithy-rs with Smithy 1.26.2.

[mtdowling]

Ok thanks for checking. One thing that I did see when trying to repro this is that the output was weird and interleaving text across threads (due to the use of PrintStream instead of PrintWriter). This PR will fix that #1525. I’ll go ahead and resolve this, but let me know if you ever see it again.