johnstonskj / rust-atelier

Rust native core model for the AWS Smithy IDL
MIT License
77 stars 11 forks source link

[BUG] model validate fails if lists or maps contain members with types from smithy.api namespace #24

Closed stevelr closed 3 years ago

stevelr commented 3 years ago

Describe the bug This map fails validation:

namespace org.example
map Thing {
  key: String,
  value: String
}

with the errors

Map key's type (org.example#Thing$key) cannot be resolved to a shape in this model Map value's type (org.example#Thing$value) cannot be resolved to a shape in this model

To Reproduce Paste the above into foo.smithy and run cargo-atelier validate -i foo.smithy

Expected behavior The model should validate without errors

Additional context I tried making the namespace explicit, as in key: smithy.api#String but got the same error.

The same error occurs for list members, and map key/value members, but does not occur for struct fields defined as String. Although the list and map validation functions intend to allow a type's namespace to be in PRELUDE_NAMEPACE, I believe they are using the type of the field (i.e., member, 'key, or 'value), which are defined in org.example's namespace, not the type of the field's value.

I believe the list bug is on this line: https://github.com/johnstonskj/rust-atelier/blob/52ad7c76c962796ac6862905a7b76ff8a648ccbc/atelier-core/src/action/validate/mod.rs#L84

Where list_or_set.member().id() should be replaced with list_or_set.member().target(),

and for Maps, In https://github.com/johnstonskj/rust-atelier/blob/52ad7c76c962796ac6862905a7b76ff8a648ccbc/atelier-core/src/action/validate/mod.rs#L90-L91

map.key().id() and map.value().id() should be replaced with map.key().target() and map.value().target() respectively.

If I make the changes above and rebuild, the validation passes as expected.

stevelr commented 3 years ago

PR #25 submitted with this fix