More predictable handling of trait/validator classes model loader#1709
More predictable handling of trait/validator classes model loader#1709Baccata merged 8 commits intoseries/0.18from
Conversation
| // Loading the upstream model | ||
| val upstreamModel = Model | ||
| .assembler() | ||
| .assembler(validatorClassLoader) |
There was a problem hiding this comment.
This should make the classes (validators, trait services) visible to the assembler - but notably not making the models visible.
kubukoz
changed the title
|
ok I think the easiest way to test this will be to use a custom (non-alloy) trait class in a plugin/CLI test - as proven by polyvariant/smithy4s-bsp#10 this finally allows us to use those properly. the other thing would still be nice to test though, I'll need to think of a library that we can use as the example (or make a plugin test like that somehow) |
| @@ -0,0 +1,4 @@ | |||
| # check if the app runs successfully. | |||
| # if it doesn't compile, it could be because the transformation wasn't applied. | |||
There was a problem hiding this comment.
thought: it is WAY too easy to misconfigure a transformation, hence this extra care
There was a problem hiding this comment.
BTW we had no other tests that involved user-provided transformations/validators, apparently. There's no software.amazon.smithy.build.ProjectionTransformer or software.amazon.smithy.model.validation.Validator in the test resources. I think these tests suffice, but maybe we should have a more minimal one for both too?
2 participants
Workaround / fix for smithy-lang/smithy#2596. Even fixes #336.
tl;dr without passing the classloader to
assembler(), we can run into strange cases where the validator gets loaded and the trait services don't - meaning the validators won't behave like they're supposed to.PR Checklist (not all items are relevant to all PRs)