Add validator for service bound resoruce operations

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

@@ -0,0 +1,83 @@
+/*
+ * 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;
+
+/**
+ * Validates if operations that are bound directly to a service may
+ * be more accurately bound to a resource bound to the same service.
+ */
+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
+    }
+}