Improve resource property validation and errors

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

@@ -27,7 +27,6 @@
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.knowledge.IdentifierBindingIndex;
-import software.amazon.smithy.model.knowledge.OperationIndex;
 import software.amazon.smithy.model.knowledge.PropertyBindingIndex;
 import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.OperationShape;
@@ -56,22 +55,21 @@ public List<ValidationEvent> validate(Model model) {
     private List<ValidationEvent> validateResource(Model model, ResourceShape resource) {
         List<ValidationEvent> events = new ArrayList<>();
         Set<String> propertiesInOperations = new TreeSet<>();
-        OperationIndex operationIndex = OperationIndex.of(model);
         PropertyBindingIndex propertyBindingIndex = PropertyBindingIndex.of(model);
 
-        processLifecycleOperationProperties(model, resource, "put", resource.getPut(), operationIndex,
+        processLifecycleOperationProperties(model, resource, "put", resource.getPut(),
             propertyBindingIndex, propertiesInOperations, events);
-        processLifecycleOperationProperties(model, resource, "create", resource.getCreate(), operationIndex,
+        processLifecycleOperationProperties(model, resource, "create", resource.getCreate(),
             propertyBindingIndex, propertiesInOperations, events);
-        processLifecycleOperationProperties(model, resource, "read", resource.getRead(), operationIndex,
+        processLifecycleOperationProperties(model, resource, "read", resource.getRead(),
             propertyBindingIndex, propertiesInOperations, events);
-        processLifecycleOperationProperties(model, resource, "update", resource.getUpdate(), operationIndex,
+        processLifecycleOperationProperties(model, resource, "update", resource.getUpdate(),
             propertyBindingIndex, propertiesInOperations, events);
-        processLifecycleOperationProperties(model, resource, "delete", resource.getDelete(), operationIndex,
+        processLifecycleOperationProperties(model, resource, "delete", resource.getDelete(),
             propertyBindingIndex, propertiesInOperations, events);
         for (ShapeId operationId : resource.getOperations()) {
             processLifecycleOperationProperties(model, resource, operationId.getName(), Optional.of(operationId),
-                operationIndex, propertyBindingIndex, propertiesInOperations, events);
+                propertyBindingIndex, propertiesInOperations, events);
         }
 
         Set<String> definedProperties = new HashSet<>(resource.getProperties().keySet());
@@ -89,14 +87,13 @@ private void processLifecycleOperationProperties(
         ResourceShape resource,
         String name,
         Optional<ShapeId> operationShapeId,
-        OperationIndex operationIndex,
         PropertyBindingIndex propertyBindingIndex,
         Set<String> propertiesInOperations,
         List<ValidationEvent> events
     ) {
         operationShapeId.flatMap(model::getShape).flatMap(Shape::asOperationShape).ifPresent(operation -> {
             propertiesInOperations.addAll(getAllOperationProperties(propertyBindingIndex, operation));
-            validateOperationInputOutput(model, propertyBindingIndex, operationIndex, resource, operation,
+            validateOperationInputOutput(model, propertyBindingIndex, resource, operation,
                     name, events);
         });
     }
@@ -122,22 +119,18 @@ private List<String> getAllOperationProperties(
     private void validateOperationInputOutput(
             Model model,
             PropertyBindingIndex propertyBindingIndex,
-            OperationIndex operationIndex,
             ResourceShape resource,
             OperationShape operation,
             String lifecycleOperationName,
             List<ValidationEvent> events
     ) {
-        validateOperationInput(model, propertyBindingIndex, operationIndex, resource,
-                operation, lifecycleOperationName, events);
-        validateOperationOutput(model, propertyBindingIndex, operationIndex, resource,
-                operation, lifecycleOperationName, events);
+        validateOperationInput(model, propertyBindingIndex, resource, operation, lifecycleOperationName, events);
+        validateOperationOutput(model, propertyBindingIndex, resource, operation, lifecycleOperationName, events);
     }
 
     private void validateOperationOutput(
             Model model,
             PropertyBindingIndex propertyBindingIndex,
-            OperationIndex operationIndex,
             ResourceShape resource,
             OperationShape operation,
             String lifecycleOperationName,
@@ -162,7 +155,6 @@ private void validateOperationOutput(
     private void validateOperationInput(
             Model model,
             PropertyBindingIndex propertyBindingIndex,
-            OperationIndex operationIndex,
             ResourceShape resource,
             OperationShape operation,
             String lifecycleOperationName,
@@ -192,7 +184,7 @@ private void validateConflictingProperties(
         for (Map.Entry<String, Set<MemberShape>> entry : propertyToMemberMappings.entrySet()) {
             if (entry.getValue().size() > 1) {
                 events.add(error(shape, String.format(
-                        "This shape contains members with conflicting property names that resolve to '%s': %s",
+                        "This shape contains members with conflicting resource property names that resolve to '%s': %s",
                         entry.getKey(),
                         entry.getValue().stream().map(MemberShape::getMemberName)
                                 .collect(Collectors.joining(", ")))));
@@ -212,13 +204,14 @@ private void validateMember(
     ) {
         String propertyName = propertyBindingIndex.getPropertyName(member.getId()).get();
         propertyToMemberMappings.computeIfAbsent(propertyName, m -> new TreeSet<>()).add(member);
-
         if (properties.containsKey(propertyName)) {
             if (!properties.get(propertyName).equals(member.getTarget())) {
-                events.add(error(resource, String.format("The resource property `%s` has a conflicting"
-                        + " target shape `%s` on the `%s` operation which targets `%s`.",
-                        propertyName, member.getTarget().toString(), lifecycleOperationName,
-                        properties.get(propertyName).toString())));
+                ShapeId expectedTarget = properties.get(propertyName);
+                events.add(error(member, String.format(
+                        "This member must target `%s`. This member is used as part of the `%s` operation of the `%s` "
+                        + "resource and conflicts with its `%s` resource property.",
+                        expectedTarget, lifecycleOperationName,
+                        resource.getId(), propertyName)));
             }
         } else if (!identifierMembers.contains(member.getMemberName())) {
             events.add(error(member, String.format("Member `%s` does not target a property or identifier"

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/resource-properties/resource-properties-errors.errors

@@ -1,4 +1,4 @@
-[ERROR] smithy.example#GetForecastOutput: This shape contains members with conflicting property names that resolve to 'chanceOfRain': chanceOfRain, chancesOfRain | ResourceOperationInputOutput
-[ERROR] smithy.example#GetForecastInput: This shape contains members with conflicting property names that resolve to 'chanceOfRain': chancesOfRain, howLikelyToRain | ResourceOperationInputOutput
+[ERROR] smithy.example#GetForecastOutput: This shape contains members with conflicting resource property names that resolve to 'chanceOfRain': chanceOfRain, chancesOfRain | ResourceOperationInputOutput
+[ERROR] smithy.example#GetForecastInput: This shape contains members with conflicting resource property names that resolve to 'chanceOfRain': chancesOfRain, howLikelyToRain | ResourceOperationInputOutput
 [ERROR] smithy.example#GetForecastInput$memberIsNotProperty: Member `memberIsNotProperty` does not target a property or identifier for resource `smithy.example#Forecast` | ResourceOperationInputOutput
-[ERROR] smithy.example#Forecast: The resource property `booleanProperty` has a conflicting target shape `smithy.api#String` on the `read` operation which targets `smithy.api#Boolean`. | ResourceOperationInputOutput
+[ERROR] smithy.example#GetForecastInput$booleanProperty: This member must target `smithy.api#Boolean`. This member is used as part of the `read` operation of the `smithy.example#Forecast` resource and conflicts with its `booleanProperty` resource property. | ResourceOperationInputOutput