Move the validation from ModelTransformer.java to RemoveShapes.java.

Author: joewyz

docs/source-2.0/spec/mixins.rst

@@ -300,7 +300,7 @@ Alternatively, the member can be redefined if it targets the same shape:
 
 .. warning::
 
-    Excluding copied mixin members explicitly through transforms will result in undefined behavior and an inconsistent model.
+    Removing mixed in members through transforms will result in an inconsistent model.
 
 Mixins are an implementation detail of the model
 ================================================

smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java

@@ -18,7 +18,6 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
-import java.util.logging.Logger;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.loader.ModelAssembler;
 import software.amazon.smithy.model.neighbor.UnreferencedShapes;
@@ -42,7 +41,6 @@
 public final class ModelTransformer {
     private final List<ModelTransformerPlugin> plugins;
 
-    private static final Logger LOGGER = Logger.getLogger(ModelTransformer.class.getName());
 
     private ModelTransformer(List<ModelTransformerPlugin> plugins) {
         this.plugins = ListUtils.copyOf(plugins);
@@ -119,32 +117,10 @@ public Model removeShapes(Model model, Collection<? extends Shape> shapes) {
         if (shapes.isEmpty()) {
             return model;
         }
-        validateMembersFromMixin(model, shapes);
+
         return new RemoveShapes(shapes, plugins).transform(this, model);
     }
 
-    private void validateMembersFromMixin(Model model, Collection<? extends Shape> shapesToExclude) {
-        for (Shape shape : shapesToExclude) {
-            // Only validate MemberShapes
-            if (!shape.isMemberShape()) {
-                continue;
-            }
-            MemberShape memberShape = shape.asMemberShape().get();
-            Optional<Shape> container = model.getShape(memberShape.getContainer());
-            if (container.isPresent()) {
-                for (ShapeId mixinId : container.get().getMixins()) {
-                    Shape mixinShape = model.expectShape(mixinId);
-                    if (mixinShape.getMemberNames().contains(shape.getId().getMember().get())) {
-                        LOGGER.warning(format("Excluding copied mixin member `%s` from mixin shape `%s` explicitly "
-                                + "in `%s` will result in undefined behavior and an inconsistent model!",
-                                memberShape.getMemberName(),
-                                mixinShape.getId(),
-                                container.get().getId().getName()));
-                    }
-                }
-            }
-        }
-    }
 
     /**
      * Removes shapes from the model that match the given predicate.

smithy-model/src/main/java/software/amazon/smithy/model/transform/RemoveShapes.java

@@ -7,9 +7,15 @@
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
+import java.util.logging.Logger;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.shapes.ShapeId;
+
+import static java.lang.String.format;
 
 /**
  * Removes shapes from a model while ensuring that relationships to/from
@@ -27,6 +33,8 @@ final class RemoveShapes {
     private final Collection<? extends Shape> toRemove;
     private final List<ModelTransformerPlugin> plugins;
 
+    private static final Logger LOGGER = Logger.getLogger(RemoveShapes.class.getName());
+
     RemoveShapes(Collection<? extends Shape> toRemove, List<ModelTransformerPlugin> plugins) {
         this.toRemove = toRemove;
         this.plugins = plugins;
@@ -37,6 +45,7 @@ Model transform(ModelTransformer transformer, Model model) {
 
         // Iteratively add each shape that needs to be removed from the index using multiple rounds.
         Set<Shape> removed = new HashSet<>(toRemove);
+        validateMembersFromMixin(model, removed);
         for (Shape removedShape : toRemove) {
             builder.removeShape(removedShape.getId());
             // We don't need to remove members from the builder since
@@ -52,4 +61,27 @@ Model transform(ModelTransformer transformer, Model model) {
 
         return result;
     }
+
+    private void validateMembersFromMixin(Model model, Collection<? extends Shape> shapesToExclude) {
+        for (Shape shape : shapesToExclude) {
+            // Only validate MemberShapes
+            if (!shape.isMemberShape()) {
+                continue;
+            }
+            MemberShape memberShape = shape.asMemberShape().get();
+            Optional<Shape> container = model.getShape(memberShape.getContainer());
+            if (container.isPresent()) {
+                for (ShapeId mixinId : container.get().getMixins()) {
+                    Shape mixinShape = model.expectShape(mixinId);
+                    if (mixinShape.getMemberNames().contains(shape.getId().getMember().get())) {
+                        LOGGER.warning(format("Removing mixed in member `%s` from mixin shape `%s` "
+                                        + "in `%s` will result in an inconsistent model.",
+                                memberShape.getMemberName(),
+                                mixinShape.getId(),
+                                container.get().getId().getName()));
+                    }
+                }
+            }
+        }
+    }
 }