Add support for validating different IDL versions

Various IDL features are only supported in versions > 1.0, including mixins, the "!" suffix for structure members, and other features soon to come. This commit adds version-specific validation to the IDL loading process to ensure that these features are only used in the appropriate versions. Given we're moving to Smithy IDL 2.0 instead of 1.1, I started using 2.0 in test cases. I'll send a followup commit that updates everything from 1.0 or 1.1 to 2.0. I kept that out of this commit to make it easier to review.
smithy-lang · mtdowling · Sep 24, 2021 · Sep 22, 2021 · Sep 22, 2021 · b0235faf54224fc5cf0b83f6cce787d6a1e7ffa1
commit b0235faf54224fc5cf0b83f6cce787d6a1e7ffa1
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AbstractMutableModelFile.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AbstractMutableModelFile.java
@@ -44,7 +44,7 @@
  */
 abstract class AbstractMutableModelFile implements ModelFile {
 
-    protected final TraitContainer traitContainer;
+    protected TraitContainer.VersionAwareTraitContainer traitContainer;
 
     private final Set<ShapeId> allShapeIds = new HashSet<>();
     private final Map<ShapeId, AbstractShapeBuilder<?, ?>> shapes = new LinkedHashMap<>();
@@ -59,7 +59,8 @@ abstract class AbstractMutableModelFile implements ModelFile {
      */
     AbstractMutableModelFile(TraitFactory traitFactory) {
         this.traitFactory = Objects.requireNonNull(traitFactory, "traitFactory must not be null");
-        traitContainer = new TraitContainer.TraitHashMap(traitFactory, events);
+        TraitContainer traitStore = new TraitContainer.TraitHashMap(traitFactory, events);
+        traitContainer = new TraitContainer.VersionAwareTraitContainer(traitStore);
     }
 
     /**
@@ -128,6 +129,24 @@ final void onTrait(ShapeId target, Trait trait) {
         traitContainer.onTrait(target, trait);
     }
 
+    /**
+     * Sets the version of the model file being loaded.
+     *
+     * @param version Version to set.
+     */
+    final void setVersion(Version version) {
+        traitContainer.setVersion(version);
+    }
+
+    /**
+     * Gets the currently defined version.
+     *
+     * @return Returns the defined version.
+     */
+    final Version getVersion() {
+        return traitContainer.getVersion();
+    }
+
     @Override
     public final List<ValidationEvent> events() {
         return events;
@@ -248,7 +267,7 @@ private <S extends Shape, B extends AbstractShapeBuilder<? extends B, S>> Option
             }
             return Optional.of(builder.build());
         } catch (SourceException e) {
-            events.add(ValidationEvent.fromSourceException(e).toBuilder().shapeId(builder.getId()).build());
+            events.add(ValidationEvent.fromSourceException(e, "", builder.getId()));
             resolvedTraits.clearTraitsForShape(builder.getId());
             return Optional.empty();
         }

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java
@@ -87,8 +87,9 @@ enum AstModelLoader {
     private static final Set<String> SERVICE_PROPERTIES = SetUtils.of(
             TYPE, "version", "operations", "resources", "rename", TRAITS);
 
-    ModelFile load(TraitFactory traitFactory, ObjectNode model) {
+    ModelFile load(Version modelVersion, TraitFactory traitFactory, ObjectNode model) {
         FullyResolvedModelFile modelFile = new FullyResolvedModelFile(traitFactory);
+        modelFile.setVersion(modelVersion);
         LoaderUtils.checkForAdditionalProperties(model, null, TOP_LEVEL_PROPERTIES, modelFile.events());
         loadMetadata(model, modelFile);
         loadShapes(model, modelFile);

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java
@@ -109,7 +109,6 @@ final class IdlModelParser extends SimpleParser {
 
     final ForwardReferenceModelFile modelFile;
     private final String filename;
-    private String definedVersion;
     private TraitEntry pendingDocumentationComment;
 
     // A pending trait that also doesn't yet have a resolved trait shape ID.
@@ -168,10 +167,16 @@ public void ws() {
 
     @Override
     public ModelSyntaxException syntax(String message) {
-        String formatted = format(
-                "Parse error at line %d, column %d near `%s`: %s",
-                line(), column(), peekDebugMessage(), message);
-        return new ModelSyntaxException(formatted, filename, line(), column());
+        return syntax(null, message);
+    }
+
+    ModelSyntaxException syntax(ShapeId shapeId, String message) {
+        return ModelSyntaxException.builder()
+                .message(format("Parse error at line %d, column %d near `%s`: %s",
+                                line(), column(), peekDebugMessage(), message))
+                .sourceLocation(filename, line(), column())
+                .shapeId(shapeId)
+                .build();
     }
 
     private void parseControlSection() {
@@ -184,7 +189,7 @@ private void parseControlSection() {
             ws();
 
             // Validation here for better error location.
-            if (key.equals("version") && definedVersion != null) {
+            if (key.equals("version") && modelFile.getVersion() != Version.UNKNOWN) {
                 throw syntax("Cannot define multiple versions in the same file");
             }
 
@@ -212,11 +217,13 @@ private void onVersion(Node value) {
         }
 
         String parsedVersion = value.expectStringNode().getValue();
-        if (!LoaderUtils.isVersionSupported(parsedVersion)) {
+        Version resolvedVersion = Version.fromString(parsedVersion);
+
+        if (resolvedVersion == null) {
             throw syntax("Unsupported Smithy version number: " + parsedVersion);
         }
 
-        definedVersion = parsedVersion;
+        modelFile.setVersion(resolvedVersion);
     }
 
     private void parseMetadataSection() {
@@ -422,7 +429,7 @@ private void parseShape(List<TraitEntry> traits) {
                 break;
             default:
                 // Unreachable.
-                throw syntax("Unexpected shape type: " + shapeType);
+                throw syntax(id, "Unexpected shape type: " + shapeType);
         }
 
         addTraits(id, traits);
@@ -477,7 +484,7 @@ private void parseMembers(ShapeId id, Set<String> requiredMembers, boolean struc
         }
 
         if (!remaining.isEmpty()) {
-            throw syntax("Missing required members of shape `" + id + "`: ["
+            throw syntax(id, "Missing required members of shape `" + id + "`: ["
                          + ValidationUtils.tickedList(remaining) + ']');
         }
 
@@ -498,15 +505,15 @@ private void parseMember(
 
         if (defined.contains(memberName)) {
             // This is a duplicate member name.
-            throw syntax("Duplicate member of " + parent + ": '" + memberName + '\'');
+            throw syntax(parent, "Duplicate member of " + parent + ": '" + memberName + '\'');
         }
 
         defined.add(memberName);
         remaining.remove(memberName);
 
         // Only enforce "allowedMembers" if it isn't empty.
         if (!required.isEmpty() && !required.contains(memberName)) {
-            throw syntax("Unexpected member of " + parent + ": '" + memberName + '\'');
+            throw syntax(parent, "Unexpected member of " + parent + ": '" + memberName + '\'');
         }
 
         ws();
@@ -517,6 +524,12 @@ private void parseMember(
         String target = ParserUtils.parseShapeId(this);
 
         if (structureMember && peek() == '!') {
+            if (!modelFile.getVersion().supportsRequiredSugar()) {
+                throw syntax(memberId, String.format(
+                        "The '!' suffix can only be used on structure members when using Smithy 2.0 or later, but "
+                        + "you're using version `%s`. Make `$version: \"2\"` the first line of this file.",
+                        modelFile.getVersion()));
+            }
             // Create a synthetic Node to specify the location.
             Node requiredTrait = new ObjectNode(Collections.emptyMap(), currentLocation());
             expect('!');
@@ -553,13 +566,20 @@ private void parseStructuredShape(
         // "Member `foo.baz#Foo$Baz` cannot be added to software.amazon.smithy.model.shapes.OperationShape$Builder"
         modelFile.onShape(builder.id(id).source(location));
 
-        // Parse optional "with" statements to add mixins.
         ws();
+
+        // Parse optional "with" statements to add mixins, but only if it's supported by the version.
         if (peek() == 'w') {
             expect('w');
             expect('i');
             expect('t');
             expect('h');
+
+            if (!modelFile.getVersion().supportsMixins()) {
+                throw syntax(id, "Mixins can only be used with Smithy version 2 or later. "
+                                 + "Attempted to use mixins with version `" + modelFile.getVersion() + "`.");
+            }
+
             ws();
             do {
                 String target = ParserUtils.parseShapeId(this);

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderUtils.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderUtils.java
@@ -54,18 +54,6 @@ static void checkForAdditionalProperties(
         }
     }
 
-    /**
-     * Checks if the given version string is supported.
-     *
-     * @param versionString Version string to check (e.g., 1, 1.0).
-     * @return Returns true if this is a supported model version.
-     */
-    static boolean isVersionSupported(String versionString) {
-        return versionString.equals("1")
-               || versionString.equals("1.0")
-               || versionString.equals("1.1");
-    }
-
     /**
      * Create a {@link ValidationEvent} for a shape conflict.
      *

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java
@@ -87,13 +87,14 @@ static ModelFile load(
     // This loader supports version 1.0 and 1.1. Support for 0.5 and 0.4 was removed in 0.10.
     static ModelFile loadParsedNode(TraitFactory traitFactory, Node node) {
         ObjectNode model = node.expectObjectNode("Smithy documents must be an object. Found {type}.");
-        StringNode version = model.expectStringMember(SMITHY);
+        StringNode versionNode = model.expectStringMember(SMITHY);
+        Version version = Version.fromString(versionNode.getValue());
 
-        if (LoaderUtils.isVersionSupported(version.getValue())) {
-            return AstModelLoader.INSTANCE.load(traitFactory, model);
-        } else {
-            throw new ModelSyntaxException("Unsupported Smithy version number: " + version.getValue(), version);
+        if (version != null) {
+            return AstModelLoader.INSTANCE.load(version, traitFactory, model);
         }
+
+        throw new ModelSyntaxException("Unsupported Smithy version number: " + versionNode.getValue(), versionNode);
     }
 
     // Allows importing JAR files by discovering models inside of a JAR file.

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelSyntaxException.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelSyntaxException.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License").
  * You may not use this file except in compliance with the License.
@@ -18,20 +18,76 @@
 import software.amazon.smithy.model.FromSourceLocation;
 import software.amazon.smithy.model.SourceException;
 import software.amazon.smithy.model.SourceLocation;
+import software.amazon.smithy.model.shapes.ShapeId;
+import software.amazon.smithy.model.shapes.ToShapeId;
+import software.amazon.smithy.utils.SmithyBuilder;
 
 /**
  * Thrown when the syntax of the IDL is invalid.
  */
-public class ModelSyntaxException extends SourceException {
+public class ModelSyntaxException extends SourceException implements ToShapeId {
+    private final ShapeId shapeId;
+
     public ModelSyntaxException(String message, int line, int column) {
-        this(message, SourceLocation.NONE.getFilename(), line, column);
+        this(builder().message(message).sourceLocation(line, column));
     }
 
     public ModelSyntaxException(String message, String filename, int line, int column) {
-        this(message, new SourceLocation(filename, line, column));
+        this(builder().message(message).sourceLocation(filename, line, column));
     }
 
     public ModelSyntaxException(String message, FromSourceLocation sourceLocation) {
-        super(message, sourceLocation);
+        this(builder().message(message).sourceLocation(sourceLocation.getSourceLocation()));
+    }
+
+    private ModelSyntaxException(Builder builder) {
+        super(builder.message, builder.sourceLocation);
+        this.shapeId = builder.shapeId;
+    }
+
+    static Builder builder() {
+        return new Builder();
+    }
+
+    @Override
+    public ShapeId toShapeId() {
+        return shapeId;
+    }
+
+    static final class Builder implements SmithyBuilder<ModelSyntaxException> {
+        private SourceLocation sourceLocation = SourceLocation.NONE;
+        private ShapeId shapeId = null;
+        private String message;
+
+        private Builder() {}
+
+        @Override
+        public ModelSyntaxException build() {
+            SmithyBuilder.requiredState("message", message);
+            return new ModelSyntaxException(this);
+        }
+
+        Builder shapeId(ShapeId shapeId) {
+            this.shapeId = shapeId;
+            return this;
+        }
+
+        Builder message(String message) {
+            this.message = message;
+            return this;
+        }
+
+        Builder sourceLocation(FromSourceLocation sourceLocation) {
+            this.sourceLocation = sourceLocation.getSourceLocation();
+            return this;
+        }
+
+        Builder sourceLocation(String filename, int line, int column) {
+            return sourceLocation(new SourceLocation(filename, line, column));
+        }
+
+        Builder sourceLocation(int line, int column) {
+            return sourceLocation(SourceLocation.NONE.getFilename(), line, column);
+        }
     }
 }
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/TraitContainer.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/TraitContainer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License").
  * You may not use this file except in compliance with the License.
@@ -246,4 +246,69 @@ private Trait createTrait(ShapeId target, ShapeId traitId, Node traitValue) {
             }
         }
     }
+
+    /**
+     * Performs version-specific validation on traits as they are added.
+     *
+     * <p>For example, this class will throw a {@link ModelSyntaxException} if
+     * the mixin trait is used in Smithy IDL 1.0.
+     */
+    final class VersionAwareTraitContainer implements TraitContainer {
+        private final TraitContainer delegate;
+        private Version version = Version.UNKNOWN;
+
+        VersionAwareTraitContainer(TraitContainer delegate) {
+            this.delegate = delegate;
+        }
+
+        /**
+         * Sets the version being tracked.
+         *
+         * @param version Version to set.
+         */
+        void setVersion(Version version) {
+            this.version = version;
+        }
+
+        /**
+         * Gets the currently configured version.
+         *
+         * @return Returns the configured version.
+         */
+        Version getVersion() {
+            return version;
+        }
+
+        @Override
+        public Map<ShapeId, Map<ShapeId, Trait>> traits() {
+            return delegate.traits();
+        }
+
+        @Override
+        public Map<ShapeId, Trait> getTraitsForShape(ShapeId shape) {
+            return delegate.getTraitsForShape(shape);
+        }
+
+        @Override
+        public void clearTraitsForShape(ShapeId shape) {
+            delegate.clearTraitsForShape(shape);
+        }
+
+        @Override
+        public Map<ShapeId, Map<ShapeId, Trait>> getTraitsAppliedToPrelude() {
+            return delegate.getTraitsAppliedToPrelude();
+        }
+
+        @Override
+        public void onTrait(ShapeId target, Trait value) {
+            version.validateVersionedTrait(target, value.toShapeId(), value.toNode());
+            delegate.onTrait(target, value);
+        }
+
+        @Override
+        public void onTrait(ShapeId target, ShapeId traitId, Node value) {
+            version.validateVersionedTrait(target, traitId, value);
+            delegate.onTrait(target, traitId, value);
+        }
+    }
 }