Add validator for identifiers missing references

This commit introduces a new validator that emits warnings when it
detects a member that matches a resource identifier without the
relevant `@references` trait configuration.

.gitignore

@@ -14,6 +14,9 @@ docs/src
 *.iml
 *.iws
 
+# Fleet
+.fleet/
+
 # VSCode
 bin/
 

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

@@ -0,0 +1,135 @@
+/*
+ * 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.List;
+import java.util.Map;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.selector.PathFinder;
+import software.amazon.smithy.model.shapes.MemberShape;
+import software.amazon.smithy.model.shapes.ResourceShape;
+import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.shapes.ShapeId;
+import software.amazon.smithy.model.traits.ReferencesTrait;
+import software.amazon.smithy.model.validation.AbstractValidator;
+import software.amazon.smithy.model.validation.ValidationEvent;
+import software.amazon.smithy.model.validation.ValidationUtils;
+import software.amazon.smithy.utils.ListUtils;
+
+/**
+ * Validates if a member matches a resource identifier without the
+ * proper configuration of a `@references` trait.
+ */
+public final class MemberShouldReferenceResourceValidator extends AbstractValidator {
+    @Override
+    public List<ValidationEvent> validate(Model model) {
+        // There are usually far fewer resources than members, precompute the identifiers
+        // so various short circuits can be added.
+        List<String> identifierNames = getAllIdentifierNames(model);
+        // Short circuit validating all the members if we don't have any resources to test.
+        if (identifierNames.isEmpty()) {
+            return ListUtils.of();
+        }
+
+        // Check every member to see if it's a potential reference.
+        List<ValidationEvent> events = new ArrayList<>();
+        for (MemberShape member : model.getMemberShapes()) {
+            // Only the known identifier names can match for this, skip names that we don't know.
+            if (!identifierNames.contains(member.getMemberName())) {
+                continue;
+            }
+            // Only strings can be identifiers, so skip non-String targets.
+            if (!model.expectShape(member.getTarget()).isStringShape()) {
+                continue;
+            }
+
+            List<ShapeId> potentialReferences = computePotentialReferences(model, member);
+            if (!potentialReferences.isEmpty()) {
+                events.add(warning(member, format("This member appears to reference the following resources without "
+                        + "being included in a `@references` trait: [%s]",
+                        ValidationUtils.tickedList(potentialReferences))));
+            }
+        }
+
+        return events;
+    }
+
+    private List<String> getAllIdentifierNames(Model model) {
+        List<String> identifierNames = new ArrayList<>();
+        for (ResourceShape resource : model.getResourceShapes()) {
+            identifierNames.addAll(resource.getIdentifiers().keySet());
+        }
+        return identifierNames;
+    }
+
+    private List<ShapeId> computePotentialReferences(Model model, MemberShape member) {
+        // Exclude any resources already in `@references` on the member or container structure.
+        List<ShapeId> resourcesToIgnore = new ArrayList<>();
+        ignoreReferencedResources(member, resourcesToIgnore);
+        ignoreReferencedResources(model.expectShape(member.getContainer()), resourcesToIgnore);
+
+        // Check each resource in the model for something missed.
+        List<ShapeId> potentialResources = new ArrayList<>();
+        for (ResourceShape resource : model.getResourceShapes()) {
+            // We'll want to ignore some resources based on the member -> resource path.
+            computeResourcesToIgnore(model, member, resource, resourcesToIgnore);
+
+            // Exclude members bound to resource hierarchies from generating events,
+            // including for resources that are within the same hierarchy.
+            if (resourcesToIgnore.contains(resource.getId())) {
+                continue;
+            }
+
+            // This member matches the identifier for the resource we're checking, add it to a list.
+            if (isIdentifierMatch(resource, member)) {
+                potentialResources.add(resource.getId());
+            }
+        }
+
+        // Clean up any resources added through other paths that should be ignored.
+        potentialResources.removeAll(resourcesToIgnore);
+        return potentialResources;
+    }
+
+    private void computeResourcesToIgnore(Model model, MemberShape member, ResourceShape resource,
+            List<ShapeId> resourcesToIgnore) {
+        // Exclude actually bound members via searching with a PathFinder.
+        List<PathFinder.Path> resourceMemberPaths = PathFinder.create(model)
+                .search(resource, ListUtils.of(member));
+        if (!resourceMemberPaths.isEmpty()) {
+            // This member is already bound to a resource, so we don't need a references trait for it.
+            // In addition, we should not tell users to add a references trait for other resources that
+            // are children in that hierarchy - any parent resources or other children of those parents.
+            for (PathFinder.Path path : resourceMemberPaths) {
+                for (Shape pathShape : path.getShapes()) {
+                    if (pathShape.isResourceShape()) {
+                        ResourceShape resourceShape = (ResourceShape) pathShape;
+                        resourcesToIgnore.add(resourceShape.getId());
+                        resourcesToIgnore.addAll(resourceShape.getResources());
+                    }
+                }
+            }
+        }
+    }
+
+    private void ignoreReferencedResources(Shape shape, List<ShapeId> resourcesToIgnore) {
+        if (shape.hasTrait(ReferencesTrait.class)) {
+            for (ReferencesTrait.Reference reference : shape.expectTrait(ReferencesTrait.class)
+                    .getReferences()) {
+                resourcesToIgnore.add(reference.getResource());
+            }
+        }
+    }
+
+    private boolean isIdentifierMatch(ResourceShape resource, MemberShape member) {
+        Map<String, ShapeId> identifiers = resource.getIdentifiers();
+        return identifiers.containsKey(member.getMemberName())
+                && identifiers.get(member.getMemberName()).equals(member.getTarget());
+    }
+}

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

@@ -26,6 +26,7 @@ software.amazon.smithy.model.validation.validators.IdempotencyTokenIgnoredValida
 software.amazon.smithy.model.validation.validators.JsonNameValidator
 software.amazon.smithy.model.validation.validators.LengthTraitValidator
 software.amazon.smithy.model.validation.validators.MediaTypeValidator
+software.amazon.smithy.model.validation.validators.MemberShouldReferenceResourceValidator
 software.amazon.smithy.model.validation.validators.NoInlineDocumentSupportValidator
 software.amazon.smithy.model.validation.validators.OperationValidator
 software.amazon.smithy.model.validation.validators.PaginatedTraitValidator

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/references/missing-member-references.errors

@@ -0,0 +1 @@
+[WARNING] ns.foo#Operation1Input$fooId: This member appears to reference the following resources without being included in a `@references` trait: [`ns.foo#FooResource`] | MemberShouldReferenceResource

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/references/missing-member-references.smithy

@@ -0,0 +1,39 @@
+$version: "2.0"
+
+namespace ns.foo
+
+resource FooResource {
+    identifiers: {
+        fooId: String
+    }
+}
+
+operation Operation1 {
+    input := {
+        fooId: String
+    }
+}
+
+operation Operation2 {
+    input := @references([{
+        resource: FooResource
+    }]) {
+        fooId: String
+    }
+}
+
+resource BarResource {
+    identifiers: {
+        barId: BarId
+    }
+}
+
+operation Operation3 {
+    input := @references([{
+        resource: BarResource
+    }]) {
+        barId: BarId
+    }
+}
+
+string BarId

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/references/valid-implicit-references.json

@@ -19,9 +19,6 @@
             },
             "traits": {
                 "smithy.api#references": [
-                    {
-                        "resource": "ns.foo#MyResource"
-                    },
                     {
                         "resource": "ns.foo#MyResource"
                     },
@@ -48,6 +45,9 @@
                 "smithy.api#references": [
                     {
                         "resource": "ns.foo#MyResource"
+                    },
+                    {
+                        "resource": "ns.foo#MyResource2"
                     }
                 ]
             }

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

@@ -1 +1,2 @@
 [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
+[WARNING] smithy.example#GetFooInput$fooId: This member appears to reference the following resources without being included in a `@references` trait: [`smithy.example#Foo`] | MemberShouldReferenceResource

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/resource-operation-binding-validator.errors

@@ -1,2 +1,2 @@
-[ERROR] ns.foo#InvalidResourceOperation1: This operation does not form a valid instance operation when bound to resource `ns.foo#InvalidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `foo` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding
+[ERROR] ns.foo#InvalidResourceOperation1: This operation does not form a valid instance operation when bound to resource `ns.foo#InvalidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `bar` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding
 [ERROR] ns.foo#ValidResourceOperation2: This operation does not form a valid instance operation when bound to resource `ns.foo#ValidResource`. All of the identifiers of the resource were not implicitly or explicitly bound to the input of the operation. Expected the following identifier bindings: [required member named `foo` that targets `smithy.api#String`]. Found the following identifier bindings: [] | ResourceIdentifierBinding

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/resource-operation-binding-validator.json

@@ -83,7 +83,7 @@
         "ns.foo#InvalidResource": {
             "type": "resource",
             "identifiers": {
-                "foo": {
+                "bar": {
                     "target": "smithy.api#String"
                 }
             },
@@ -129,7 +129,7 @@
         "ns.foo#InvalidResourceOperation2Input": {
             "type": "structure",
             "members": {
-                "foo": {
+                "bar": {
                     "target": "smithy.api#String",
                     "traits": {
                         "smithy.api#required": {}