Add validator for service bound resoruce operations

This commit introduces a new validator that emits warnings when it
detects operations that are bound directly to a service when a resource
bound to that service may be a better match.

smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/ServiceBoundResourceOperationValidator.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.model.validation.validators;
+
+import static java.lang.String.format;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.knowledge.OperationIndex;
+import software.amazon.smithy.model.knowledge.TopDownIndex;
+import software.amazon.smithy.model.shapes.MemberShape;
+import software.amazon.smithy.model.shapes.OperationShape;
+import software.amazon.smithy.model.shapes.ResourceShape;
+import software.amazon.smithy.model.shapes.ServiceShape;
+import software.amazon.smithy.model.shapes.ShapeId;
+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.ValidationUtils;
+
+public final class ServiceBoundResourceOperationValidator extends AbstractValidator {
+ @Override
+ public List<ValidationEvent> validate(Model model) {
+ List<ValidationEvent> events = new ArrayList<>();
+ OperationIndex operationIndex = OperationIndex.of(model);
+ TopDownIndex topDownIndex = TopDownIndex.of(model);
+
+ // Check every service operation to see if it should be bound to a resource instead.
+ for (ServiceShape service : model.getServiceShapes()) {
+ // Store potential targets to emit one event per operation.
+ Map<OperationShape, Set<ShapeId>> potentiallyBetterBindings = new HashMap<>();
+ for (ShapeId operationId : service.getOperations()) {
+ OperationShape operation = model.expectShape(operationId, OperationShape.class);
+ // Check the resources of the containing service to test for input/output attachment.
+ for (ResourceShape resource : topDownIndex.getContainedResources(service)) {
+ // Check the operation members to see if they are implicit matches for resource identifiers.
+ for (MemberShape member : operationIndex.getInputMembers(operation).values()) {
+ if (isImplicitIdentifierBinding(member, resource)) {
+ potentiallyBetterBindings.computeIfAbsent(operation, k -> new HashSet<>())
+ .add(resource.getId());
+ }
+ }
+ for (MemberShape member : operationIndex.getOutputMembers(operation).values()) {
+ if (isImplicitIdentifierBinding(member, resource)) {
+ potentiallyBetterBindings.computeIfAbsent(operation, k -> new HashSet<>())
+ .add(resource.getId());
+ }
+ }
+ }
+ }
+
+ // Emit events per service that's present with a potentially bad binding.
+ for (Map.Entry<OperationShape, Set<ShapeId>> entry : potentiallyBetterBindings.entrySet()) {
+ events.add(warning(entry.getKey(), service, format(
+ "The `%s` operation is bound to the `%s` service but has members that match identifiers "
+ + "of the following resource shapes: [%s]. It may be more accurately bound to one "
+ + "of them than directly to the service.",
+ entry.getKey().getId(), service.getId(), ValidationUtils.tickedList(entry.getValue())),
+ service.getId().toString(), entry.getKey().getId().getName()));
+ }
+ }
+
+ return events;
+ }
+
+ private boolean isImplicitIdentifierBinding(MemberShape member, ResourceShape resource) {
+ return resource.getIdentifiers().containsKey(member.getMemberName())
+ && member.getTrait(RequiredTrait.class).isPresent()
+ && member.getTarget().equals(resource.getIdentifiers().get(member.getMemberName()));
+ }
+}

smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator

@@ -40,6 +40,7 @@ software.amazon.smithy.model.validation.validators.ResourceIdentifierValidator
 software.amazon.smithy.model.validation.validators.ResourceLifecycleValidator
 software.amazon.smithy.model.validation.validators.ResourceOperationInputOutputValidator
 software.amazon.smithy.model.validation.validators.ServiceAuthDefinitionsValidator
+software.amazon.smithy.model.validation.validators.ServiceBoundResourceOperationValidator
 software.amazon.smithy.model.validation.validators.ServiceValidator
 software.amazon.smithy.model.validation.validators.SetValidator
 software.amazon.smithy.model.validation.validators.ShapeIdConflictValidator

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/resource-identifiers/detects-misbound-operations.errors

@@ -0,0 +1 @@
+[WARNING] smithy.example#GetFoo: The `smithy.example#GetFoo` operation is bound to the `smithy.example#FooService` service but has members that match identifiers of the following resource shapes: [`smithy.example#Foo`]. It may be more accurately bound to one of them than directly to the service. | ServiceBoundResourceOperation.smithy.example#FooService.GetFoo

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/resource-identifiers/detects-misbound-operations.smithy

@@ -0,0 +1,53 @@
+$version: "2.0"
+
+namespace smithy.example
+
+service FooService {
+ version: "2020-07-02"
+ operations: [GetFoo]
+ resources: [Foo, BoundNoMatch]
+}
+
+resource Foo {
+ identifiers: {
+ fooId: String
+ }
+ create: CreateFoo
+}
+
+operation CreateFoo {
+ input := {
+ @required
+ something: String
+ }
+ output := {
+ @required
+ fooId: String
+
+ @required
+ something: String
+ }
+}
+
+operation GetFoo {
+ input := {
+ @required
+ fooId: String
+ }
+ output := {
+ @required
+ something: String
+ }
+}
+
+resource Unbound {
+ identifiers: {
+ id: String
+ }
+}
+
+resource BoundNoMatch {
+ identifiers: {
+ id: String
+ }
+}