This commit updates the validation for resource identifier collisions
to warn instead of error. When an explicit and implicit binding for
the same resource identifier are found, the explicit binding is
preferred.
Many of the validators in Smithy and Smithy Diff today emit events for
various reasons. A single validator might event different events with
different severities, and there is currently no way to suppress just
one specific type of event for a given validator. For example, the
ChangedNullability SmithyDiff validator emits events for various reasons,
some of which different tools may choose to suppress. However, without
inspecting the message of the event, there's no way to know what you're
suppressing.
To give existing validators the ability to emit more granular validation
events without breaking current validators, this change introduces
event ID hierarchies and hierarchical suppression IDs using dots (.).
For example, ChangedNullability.RemovedRequiredTrait is now emitted, and
tools can check for "ChangedNullability" to match all events, or more
granularly only match "ChangedNullability.RemovedRequiredTrait". This
can also be used in suppressions so that existing Smithy validators can
be updated to emit more granular events.
Previously, the ABNF did not match the identifier parser.
This commit updates the ABNF to match current parsing behavior, and
updates the identifier parser to avoid out of bounds exceptions.
Also, a set of tests are added for ONLY parsing identifiers rather
than being mixed with namespace tests.
Smithy-Diff was flagging acceptable changes of removing `@default(null)` from
member when the trait had no effect because the target shape was already nullable.
NullableIndex was also giving the `@default(null)` trait precedence over the
`@required` trait which is incorrect. Required members are non-nullable, and
the `@default(null)` trait doesn't override this. It simply means that there is
no default value for the member (so coupled with the required trait, the member
is still always present).
This commit fixes an issue where documentation comments were dropped
for members that follow a member with a default value. Pending docs
are only cleared when a default isn't encountered or the container
is complete.
The suppress trait contained a restrictive pattern trait that was
not present in validator ID definitions. This meant that validation
events could be suppressed in metadata suppressions (where there was
no pattern validation), but not using the `@suppress` trait. Adding
the pattern from the `@suppress` trait to validation event IDs at
this point has the potential to break existing models, so that is
not a desirable alternative.
Closes#987
When loading a model with multiple members bound to the same resource identifier,
Smithy failed with an error in IdentifierBindingIndex due to Collectors.toMap:
Duplicate key X (attempted merging values y and z)
This change updates IdentifierBindingIndex to ignore collisions. However this
means that colliding resource identifier bindings would be ignored (which is
arguably worse than failing to load the model since it wouldn't ever flag
these issues, and retroactively enforcing things is always challenging).
To address this, I added validation to detect when members with the
resourceIdentifier trait conflict with members of the same binding name
or members that have the same resourceIdentifier trait.
We previously did not attempt to perform model interop transforms
if a model had errors. For example, if a model referred to an
unknown trait, the model wouldn't use the interop transformation.
Some use cases might choose to ignore unknown traits and other
errors by _not_ unwrapping the assembled model and instead
directly accessing the result from ValidatedResult. In these
cases, the model would have not gone through the interop transform,
resulting in an unexpect model (for example, if performing a diff
against a 1.0 and 2.0 model, the upgrade won't run and causes the
models to appear to be wildly different).
This commit adds the following test cases for 1.0 roundtrip
serialization for `@enum` and `@box` traits (refactor test
factories).
Also, added a string shape with the `@enum` trait for the 2.0
enum roundtrip test case.
Updated removing unnecessary defaults to fix `MemberShape`
cases with comments to explain each case.
Fixes: #1369
PrivateAccessValidator was not checking trait relationships which allowed private traits to be referenced in any namespace. The HttpConfiguration mixin in aws.protocols was not including private in its localTraits, which caused the private trait to be applied to any shape that mixed in HttpConfiguration. This issue wasn't apparent because trait relationships were not being validated for private access. The private access validation test was updated to check for this.
This could be a breaking change for any Smithy models that are using private traits in other namespaces, either directly or through a mixin that doesn't have private included in its localTraits.
Commits:
* Fix bug where `private` would be applied to every shape that mixes in `HttpConfiguration`
* Fix bug where private access wasn't enforced on trait applications
This fixes an issue where `applySomeShape @someTrait` was valid,
without any space between the apply and shape name. The fix makes
it so spacing is required, not line breaks as was the case in
the old ABNF, to be consistent with other shapes grammar
(see [SimpleShapeStatement](https://awslabs.github.io/smithy/2.0/spec/idl.html#grammar-token-smithy-SimpleShapeStatement))
The previous implementation could cause a recursive update error when
a shape ID is purged from the cache because it attempted to remove an
element from the ConcurrentHashMap inside of a computeIfAbsent call.
This updated approach is simpler and instead separates prelude shape
IDs into a ConcurrentHashMap that just stops caching after 500 entries
are in the cache (which does more than cover the shapes currently in
the prelude), and uses a synchronized LinkedHashMap to function as an
LRU cache for all other shape IDs. The performance of this approach
vs what we had previously is essentially the same.
This commit also fixes an issue where we would sometimes add
empty apply statements to the JSON AST because a mixin introduces
no traits. The AST parser now handles this without failing, and
the model serializer no longer writes out empty apply blocks.
This commit adds the ability to downgrade the in-memory semantic
model to 1.0 models, and adds support for serializing 1.0 JSON
AST models to ModelSerializer.
This commit makes several significant updates to Smithy IDL 2 that
improve lossless interoperability with Smithy IDL 1:
* Default traits can now coexist with required. This indicates that
a member should be serialized, but it is a protocol-specific
decision if and how this is enforced. This was a pattern that occurred
in Smithy 1.0 models when a member was required and targeted a shape
with a zero value.
* Default traits can be added to root-level shapes. Any structure member
that targets a shape marked with the default trait must repeat the
default on the member. This removes the action at a distance problem
observed in Smithy IDL 1 where a root level shape implicitly introduced
a default zero value, and to know if that's the case for any member, you
had to look through from the member to the target shape. This change
allows us to know if a root level shape was boxed in IDL 1.0 too -- if
it is a shape that had a zero value in IDL 1 and either doesn't have
a default or the default is not the zero value, then it was boxed in v1.
* When loading v2 models, we will now patch synthetic box traits onto shapes
that would have been considered boxed by existing smithy-model consumers.
This improves further interop with tooling that has not yet adopted
Smithy IDL 2 or that hasn't yet migrated to use the NullableIndex
abstraction.
* Added an optional nullability report to smithy-build that shows the
computed nullability semantics of each member in a model. This can be
used to better understand nullability semantics.
* Updated smithy-diff to not emit events when diffing a 1.0 model against
the 2.0 serialized version of the model. This means that changes to the
box trait are ignored unless the change impacts the nullability of the
shape. Special handling was added to detect breaking changes with the
default trait too (you can't change a default value of a root-level shape
for example, you can't change a default value of a shape to or from the
zero value of a type as this might break code generators, etc).
* Add new NullableIndex modes for testing if a member is nullable based on
the supported features of the generator. For example, some generators only
make members non-optional when the member is set to the zero value of a
type, so there is a NullableIndex check mode for that and other use cases.
* Added the @addedDefault trait which is used to indicate that a `@default`
trait was added to a member after it was initially released. This can be
used by tooling to make an appropriate determination if generating a
non-nullable type for the member is a backward compatible change. For
example, if a generator only uses default zero values to generate
non-nullable types, then the removal of the required trait and addition
of a default trait would be a breaking change for them, so they can use
addedDefault to ignore the default trait.
* When loading 1.0 models, rather than dropping the default trait from a
member when the range trait of a shape is invalid for its zero value, we
now instead emit only a warning for this specific case. This prevents
changing the type and also doesn't lose the range constraint in case
implementations are relying on this validation.
* Un-deprecated Primitive* shapes in the prelude and add the `@default`
trait to them. Any member that targets one of these primitive prelude
shapes must now also repeat the zero value of the target shape.
* Fixed an issue where synthetic box traits were being dropped when the
same model is loaded multiple times.
- Changed the selector of `@httpResponseCode` to not select structures
with the `@input` trait applied
- Updated the docs for `@httpResponseCode`
- Move `http-trait-conflicts` test to a `.smithy` file
Resolves#1105
Synthetic traits should not be parsed again. That will cause a
failure, for instance, if we import a 1.0 model that has @enum
traits. If we try to reparse them this will fail with an error
Unable to resolve trait `smithy.synthetic#enum`
* Cleanup after rebasing against main
* Update OpenAPI converter to be deterministic
* Fix the flattenMixins model transform to not use the removeShapes
transform when removing mixins. This causes issues when a mixin is
already flattened out of a shape.
* Fixed a naming issue for setting enum shape members.
* Don't try to flatten mixins in a shape if it has none.
* Clean up method for finding a required mixin member so it doesn't
build intermediate member shapes.
* Add test to ensure forward references are ordered.
* Mark a few public APIs as internal in the OpenAPI converters because
I think we'll refactor them heavily in the near future.
We actually do need synthetic box traits on members so that tooling that
attempts to work with 1.0 and 2.0 models can accurately know the intended
nullability semantics of a structure member after it has been loaded into
memory. This is particularly important for structure members marked as
required that target a prelude shape like Integer. Without a synthetic
box trait on the member, tooling can't differentiate between a member that
was considered nullable in v1 or non-nullable in v2 (the box trait on the
prelude shape would be honored in v1 but discarded in v2). The trait on
the member itself makes the 1.0 nullability explicit. So explicit in fact
that we can even check for this trait and use it in the NullableIndex
methods that understand 2.0 semantics. The box trait is only allowed in
1.0 models, so its present doesn't interfere with IDL 2.0 nullability
semantics. We will still keep the IDL 1.0 isNullable method around because
it will work even with models that weren't loaded through an assembler
(such models skip the model upgrade process and have no synthetic box trait).
If we don't know the version of a shape (like if a Model
or a bunch of pre-built shapes are added to an assembler),
then we should not attempt to upgrade them. If we do attempt
to upgrade them, then we cause issues with things like
projections in smithy-build and CLI, where we build a model,
then pass that built model into each project, thereby causing
the version to be unknown (it's detached from the original
source file that contained the version information). If we
do attempt to upgrade this kind of model again, then we can
get into a situation where the following model:
```
$version: "2.0"
namespace smithy.example
structure Foo {
bar: Bar
}
integer Bar
```
Is projected into the following, incorrect model:
```
{
"smithy": "2.0",
"shapes": {
"smithy.example#Foo" {
"type": "structure",
"members": {
"bar": {
"target": "smithy.example#Bar",
"traits": {
"smithy.api#default": 0
}
}
}
},
"smithy.example#Bar": {
"type": "integer"
}
}
}
```
Notice the incorrectly added `@default` trait. This would happen because
when using Smithy 1.0 semantics, `integer Bar` has a default zero value
because it isn't marked with the `@box` trait. No `@default` trait is added
to the "source" model projection because at that point we know the version
is 2.0. But when doing another projection, the loaded model is disconnected
from the original version and would cause this issue.
In addition to not doing upgrades on shapes with an unknown version, this
change also stops adding synthetic box traits to members. That's no longer
needed because we now keep a synthetic box trait on prelude shapes. When
we removed those traits previously, it meant it was impossible to determine
the 1.0 nullability of a shape unless we add the synthetic box trait to the
member. Given this change, we no longer need to validate the box trait
in the ModelUpgrade and can rather do the validation in the Version enum
directly.
Let's keep NullableIndex as-is rather than change it's default behavior
to look for v2 semantics. This ensures that no existing code will
change behavior unless they want it to. The isNullable method is
now marked as deprecated and continues to use v1 semantics, while
the other newly introduced methods use v2 semantics. Added various
test cases to ensure v1 and v2 compat.
The box trait is no longer supported in IDL 2.0, so it was removed from
the Smithy 2.0 prelude model. However, tooling that is built for Smithy
1.0 might look at shapes targeted by members for the box trait, and their
logic would change because the box trait was removed from common shapes
like Integer, Boolean, etc. To maintain backward compatibility, this
commit adds a box trait to these prelude shapes using the
prelude-1.0.smithy model that was previously only used to define
the `@box` trait for 1.0 models. It now uses the 1.0 model version so
that it can apply the box trait to prelude shapes. This change removes
the need for some custom logic in ModelUpgrader too.
There are some teams within Amazon that used operation properties in an
order other than input, output, errors (weird!). Rather than break them,
we'll just allow any order in the grammar and parser and use post-parsing
logic to ensure no duplicates.
For some reason Windows tests are failing with anything to do with newlines,
event though we attempt to normalize line endings. To unblock us, I just
removed anything that caused line ending issues for now.
Members of list, map, union, and structure are now explicitly
modeled rather than relying on a generic grammar for all members.
Two new constraints:
1. Map keys, if defined, must be defined before map values.
2. Members that are not elided must be on the same line.
`foo:\nBar` is no longer valid.
This commit tightens the grammar of the IDL to be a bit more
restrictive and less ambiguous. Of note:
* Several rules were updated to require spaces between productions.
For example `strutureFoo{}` is no longer technically valid.
`usesmithy.api#Foo` is no longer valid.
* control statements must be on a single line. For example,
`$version:\n"1.0"` is no longer valid. Control statements must
also be followed by a line break.
* metadata statements must be on a single line. `metadata foo\n="bar"`
is no longer valid. Metadata statements must be followed by a
line break.
* use statements must be on a single line too and followed by a
line break.
* Specify that newlines are allowed in strings
* Updated grammar rules to CamelCase to be compatible with ABNF
* Adopted RFC 7405 to specify case sensitive strings
* Default value assignment and enum value assignment must occur
on the same line.
* Operation input, output, and errors is now specified more
granularly and must be defined in this specific order.
* Operation input, output, and error definition must be followed by
a line break.
* Assigning a default value or an enum value must be followed by a line
break.
kstich