diff --git a/docs/source/1.0/spec/waiters.rst b/docs/source/1.0/spec/waiters.rst index 586ba57d981..4685254cf0f 100644 --- a/docs/source/1.0/spec/waiters.rst +++ b/docs/source/1.0/spec/waiters.rst @@ -111,6 +111,11 @@ ABNF: .. seealso:: :ref:`waiter-best-practices` for additional best practices to follow when naming waiters. +Each waiter in the :ref:`closure of a service ` MUST have +a case-insensitively unique waiter name. This limitation helps make it +easier to both understand a service and to generate code for a service +without needing to consider duplicate waiter names across operations. + Waiter workflow =============== diff --git a/smithy-waiters/src/main/java/software/amazon/smithy/waiters/UniqueWaiterNamesValidator.java b/smithy-waiters/src/main/java/software/amazon/smithy/waiters/UniqueWaiterNamesValidator.java new file mode 100644 index 00000000000..6cbb7ec719b --- /dev/null +++ b/smithy-waiters/src/main/java/software/amazon/smithy/waiters/UniqueWaiterNamesValidator.java @@ -0,0 +1,91 @@ +/* + * Copyright 2020 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.waiters; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; +import java.util.stream.Collectors; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.TopDownIndex; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.validation.AbstractValidator; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.utils.SmithyInternalApi; + +/** + * Ensures that no two waiters use the same case-insensitive name in the + * closure of a service. + */ +@SmithyInternalApi +public final class UniqueWaiterNamesValidator extends AbstractValidator { + @Override + public List validate(Model model) { + return model.shapes(ServiceShape.class) + .flatMap(service -> validateService(model, service).stream()) + .collect(Collectors.toList()); + } + + private List validateService(Model model, ServiceShape service) { + Map> waiterNamesToOperations = computeWaiterNamesToOperations(model, service); + List events = new ArrayList<>(); + + for (Map.Entry> entry : waiterNamesToOperations.entrySet()) { + if (entry.getValue().size() > 1) { + for (OperationShape operation : entry.getValue()) { + Set conflicts = entry.getValue().stream() + .filter(o -> !o.equals(operation)) + .map(Shape::getId) + .collect(Collectors.toCollection(TreeSet::new)); + events.add(error(operation, operation.expectTrait(WaitableTrait.class), String.format( + "`%s` trait waiter name `%s` case-insensitively conflicts with waiters on other " + + "operations in the closure of service, `%s`: %s", + WaitableTrait.ID, + entry.getKey(), + service.getId(), + conflicts))); + } + } + } + + return events; + } + + private Map> computeWaiterNamesToOperations(Model model, ServiceShape service) { + TopDownIndex index = TopDownIndex.of(model); + Map> waiterNamesToOperations = new TreeMap<>(); + + for (OperationShape operation : index.getContainedOperations(service)) { + operation.getTrait(WaitableTrait.class).ifPresent(trait -> { + for (String name : trait.getWaiters().keySet()) { + Set operations = waiterNamesToOperations.computeIfAbsent( + name.toLowerCase(Locale.ENGLISH), n -> new HashSet<>()); + operations.add(operation); + } + }); + } + + return waiterNamesToOperations; + } +} diff --git a/smithy-waiters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator b/smithy-waiters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator index 6a6953d8013..0a38dd7223b 100644 --- a/smithy-waiters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator +++ b/smithy-waiters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator @@ -1 +1,2 @@ software.amazon.smithy.waiters.WaitableTraitValidator +software.amazon.smithy.waiters.UniqueWaiterNamesValidator diff --git a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/waiter-name-conflicts.errors b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/waiter-name-conflicts.errors new file mode 100644 index 00000000000..12786482c15 --- /dev/null +++ b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/waiter-name-conflicts.errors @@ -0,0 +1,4 @@ +[ERROR] smithy.example#A: `smithy.waiters#waitable` trait waiter name `a` case-insensitively conflicts with waiters on other operations in the closure of service, `smithy.example#InvalidService`: [smithy.example#B] | UniqueWaiterNames +[ERROR] smithy.example#A: `smithy.waiters#waitable` trait waiter name `b` case-insensitively conflicts with waiters on other operations in the closure of service, `smithy.example#InvalidService`: [smithy.example#B] | UniqueWaiterNames +[ERROR] smithy.example#B: `smithy.waiters#waitable` trait waiter name `a` case-insensitively conflicts with waiters on other operations in the closure of service, `smithy.example#InvalidService`: [smithy.example#A] | UniqueWaiterNames +[ERROR] smithy.example#B: `smithy.waiters#waitable` trait waiter name `b` case-insensitively conflicts with waiters on other operations in the closure of service, `smithy.example#InvalidService`: [smithy.example#A] | UniqueWaiterNames diff --git a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/waiter-name-conflicts.smithy b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/waiter-name-conflicts.smithy new file mode 100644 index 00000000000..d3ff7401045 --- /dev/null +++ b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/waiter-name-conflicts.smithy @@ -0,0 +1,82 @@ +namespace smithy.example + +use smithy.waiters#waitable + +service InvalidService { + version: "2020-11-30", + operations: [A, B], +} + +@waitable( + A: { + "documentation": "A", + "acceptors": [ + { + "state": "success", + "matcher": { + "output": { + "path": "foo == 'hi'", + "comparator": "booleanEquals", + "expected": "true" + } + } + } + ] + }, + B: { + "acceptors": [ + { + "state": "success", + "matcher": { + "output": { + "path": "foo == 'hey'", + "comparator": "booleanEquals", + "expected": "true" + } + } + } + ] + } +) +operation A { + output: AOutput, +} + +structure AOutput { + foo: String, +} + +@waitable( + A: { + "documentation": "A", + "acceptors": [ + { + "state": "success", + "matcher": { + "output": { + "path": "foo == 'hi'", + "comparator": "booleanEquals", + "expected": "true" + } + } + } + ] + }, + B: { + "acceptors": [ + { + "state": "success", + "matcher": { + "output": { + "path": "foo == 'hey'", + "comparator": "booleanEquals", + "expected": "true" + } + } + } + ] + } +) +operation B { + output: AOutput, +}