diff --git a/docs/source/1.0/guides/model-linters.rst b/docs/source/1.0/guides/model-linters.rst index c4f9419be7a..ad5b52824c5 100644 --- a/docs/source/1.0/guides/model-linters.rst +++ b/docs/source/1.0/guides/model-linters.rst @@ -591,6 +591,83 @@ Configuration represent time. +.. _MissingClientOptionalTrait: + +MissingClientOptionalTrait +========================== + +Allows services to control backward compatibility guarantees for +members marked as :ref:`@required ` and +:ref:`@default ` by requiring the application of the +:ref:`@clientOptional ` trait. + +Rationale + Different service providers have different backward compatibility + guarantees for :ref:`@required ` and + :ref:`@default ` structure members. Some + services wish to reserve the right to remove the ``@required`` trait at + any time, while others are able to strictly follow the backward-compatibility + guarantees of the ``@required`` trait. For example, it is considered + backward compatible to remove the ``@required`` trait from a member and + replace it with the ``@default`` trait. However, this isn't possible for + members that target structure or union shapes because they have no zero + value. The risk associated with such members may be unacceptable for some + services. + +Default severity + ``DANGER`` + +Configuration + .. list-table:: + :header-rows: 1 + :widths: 20 20 60 + + * - Property + - Type + - Description + * - onRequiredOrDefault + - ``boolean`` + - Requires that members marked with the ``@required`` or ``@default`` + trait are also marked with the ``@clientOptional`` trait. + * - onRequiredStructureOrUnion + - ``boolean`` + - Requires that ``@required`` members that target structure or union + shapes are also marked with the ``@clientOptional`` trait. + ``@required`` members that target structures and unions are risky + because there is no backward compatible way to replace the + ``@required`` trait with the ``@default`` trait if the member ever + needs to be made optional. + +The following example requires that ``@required`` members that target a structure or +union are marked with the ``@clientOptional`` trait. + +.. code-block:: smithy + + metadata validators = [ + { + name: "MissingClientOptionalTrait", + + // Limit validation to a specific set of namespaces. + namespaces: ["smithy.example"], + + configuration: { + onRequiredStructureOrUnion: true + } + } + ] + +This validation can be suppressed for any member that the service provider +decides is not at risk of ever needing to become optional in the future: + +.. code-block:: smithy + + structure Sprocket { + @required + @suppress(["MissingClientOptionalTrait"]) + owner: OwnerStructure + } + + ------------------------- Writing custom validators ------------------------- diff --git a/smithy-linters/src/main/java/software/amazon/smithy/linters/MissingClientOptionalTrait.java b/smithy-linters/src/main/java/software/amazon/smithy/linters/MissingClientOptionalTrait.java new file mode 100644 index 00000000000..bedfb151bd3 --- /dev/null +++ b/smithy-linters/src/main/java/software/amazon/smithy/linters/MissingClientOptionalTrait.java @@ -0,0 +1,119 @@ +/* + * Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.linters; + +import java.util.ArrayList; +import java.util.List; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.node.NodeMapper; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.traits.ClientOptionalTrait; +import software.amazon.smithy.model.traits.DefaultTrait; +import software.amazon.smithy.model.traits.RequiredTrait; +import software.amazon.smithy.model.validation.AbstractValidator; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.ValidatorService; + +/** + * Validates that the clientOptional trait is applied based on the rules + * defined in the validator. + */ +public final class MissingClientOptionalTrait extends AbstractValidator { + + /** + * MissingClientOptionalTrait configuration settings. + */ + public static final class Config { + private boolean onRequiredStructureOrUnion; + private boolean onRequiredOrDefault; + + /** + * Whether clientOptional is required for members marked as required that + * target structures or unions (shapes with no zero value, meaning it's + * impossible to later remove the required trait and replace it with the + * default trait). + * + * @return Returns true if required. + */ + public boolean onRequiredStructureOrUnion() { + return onRequiredStructureOrUnion; + } + + public void onRequiredStructureOrUnion(boolean onRequiredStructuresOrUnion) { + this.onRequiredStructureOrUnion = onRequiredStructuresOrUnion; + } + + /** + * Whether clientOptional is required for all members marked as required or default. + * + * @return Returns true if required. + */ + public boolean onRequiredOrDefault() { + return onRequiredOrDefault; + } + + public void onRequiredOrDefault(boolean onDefault) { + this.onRequiredOrDefault = onDefault; + } + } + + public static final class Provider extends ValidatorService.Provider { + public Provider() { + super(MissingClientOptionalTrait.class, configuration -> { + NodeMapper mapper = new NodeMapper(); + Config config = mapper.deserialize(configuration, MissingClientOptionalTrait.Config.class); + return new MissingClientOptionalTrait(config); + }); + } + } + + private final Config config; + + public MissingClientOptionalTrait(Config config) { + this.config = config; + } + + @Override + public List validate(Model model) { + List events = new ArrayList<>(); + for (MemberShape member : model.getMemberShapes()) { + if (member.hasTrait(ClientOptionalTrait.class)) { + continue; + } + if (member.hasTrait(DefaultTrait.class) && config.onRequiredOrDefault) { + events.add(danger(member, "@default members must also be marked with the @clientOptional trait")); + } + if (member.hasTrait(RequiredTrait.class)) { + if (config.onRequiredOrDefault) { + events.add(danger(member, "@required members must also be marked with the @clientOptional trait")); + } else if (config.onRequiredStructureOrUnion && isTargetingStructureOrUnion(model, member)) { + events.add(danger(member, "@required members that target a structure or union must be marked with " + + "the @clientOptional trait. Not using the @clientOptional trait here " + + "is risky because there is no backward compatible way to replace the " + + "@required trait with the @default trait if the member ever needs to " + + "be made optional.")); + } + } + } + return events; + } + + private boolean isTargetingStructureOrUnion(Model model, MemberShape member) { + Shape target = model.expectShape(member.getTarget()); + return target.isStructureShape() || target.isUnionShape(); + } +} diff --git a/smithy-linters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService b/smithy-linters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService index 4d768d18dc7..344efdf388e 100644 --- a/smithy-linters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService +++ b/smithy-linters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService @@ -2,6 +2,7 @@ software.amazon.smithy.linters.AbbreviationNameValidator$Provider software.amazon.smithy.linters.CamelCaseValidator$Provider software.amazon.smithy.linters.NoninclusiveTermsValidator$Provider software.amazon.smithy.linters.InputOutputStructureReuseValidator$Provider +software.amazon.smithy.linters.MissingClientOptionalTrait$Provider software.amazon.smithy.linters.MissingPaginatedTraitValidator$Provider software.amazon.smithy.linters.RepeatedShapeNameValidator$Provider software.amazon.smithy.linters.ReservedWordsValidator$Provider diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredOrDefault.errors b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredOrDefault.errors new file mode 100644 index 00000000000..b47079e05bf --- /dev/null +++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredOrDefault.errors @@ -0,0 +1,2 @@ +[DANGER] smithy.example#Foo$bar: @default members must also be marked with the @clientOptional trait | MissingClientOptionalTrait +[DANGER] smithy.example#Foo$baz: @required members must also be marked with the @clientOptional trait | MissingClientOptionalTrait diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredOrDefault.smithy b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredOrDefault.smithy new file mode 100644 index 00000000000..6f7037ce3ef --- /dev/null +++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredOrDefault.smithy @@ -0,0 +1,20 @@ +$version: "2" + +metadata validators = [ + { + name: "MissingClientOptionalTrait", + configuration: { + "onRequiredOrDefault": true + } + } +] + +namespace smithy.example + +structure Foo { + @default + bar: String, + + @required + baz: String +} diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredStructureOrUnion.errors b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredStructureOrUnion.errors new file mode 100644 index 00000000000..a940218424a --- /dev/null +++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredStructureOrUnion.errors @@ -0,0 +1,2 @@ +[DANGER] smithy.example#Foo$bam: @required members that target a structure or union must be marked with the @clientOptional trait. Not using the @clientOptional trait here is risky because there is no backward compatible way to replace the @required trait with the @default trait if the member ever needs to be made optional. | MissingClientOptionalTrait +[DANGER] smithy.example#Foo$boo: @required members that target a structure or union must be marked with the @clientOptional trait. Not using the @clientOptional trait here is risky because there is no backward compatible way to replace the @required trait with the @default trait if the member ever needs to be made optional. | MissingClientOptionalTrait diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredStructureOrUnion.smithy b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredStructureOrUnion.smithy new file mode 100644 index 00000000000..01511ebf1f7 --- /dev/null +++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/missing-client-optional-onRequiredStructureOrUnion.smithy @@ -0,0 +1,32 @@ +$version: "2" + +metadata validators = [ + { + name: "MissingClientOptionalTrait", + configuration: { + "onRequiredStructureOrUnion": true + } + } +] + +namespace smithy.example + +structure Foo { + @default + bar: String, + + @required + baz: String, + + @required + bam: Bam, + + @required + boo: Boo +} + +structure Bam {} + +union Boo { + test: String +}