smithy-lang · mtdowling · Mar 31, 2023 · Mar 31, 2023
@@ -0,0 +1,53 @@
+/*
+ * Copyright 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.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package software.amazon.smithy.cli;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.not;
+
+import org.junit.jupiter.api.Test;
+import software.amazon.smithy.utils.ListUtils;
+
+public class AstCommandTest {
+    @Test
+    public void validatesModelSuccess() {
+        IntegUtils.run("model-with-warning", ListUtils.of("ast"), result -> {
+            assertThat(result.getExitCode(), equalTo(0));
+            assertThat(result.getOutput(), containsString("\"smithy\""));
+            assertThat(result.getOutput(), not(containsString("WARNING")));
+        });
+    }
+
+    @Test
+    public void convertsSyntacticallyCorrectModels() {
+        IntegUtils.run("invalid-model", ListUtils.of("ast"), result -> {
+            assertThat(result.getExitCode(), equalTo(0));
+            assertThat(result.getOutput(), containsString("\"smithy\""));
+            assertThat(result.getOutput(), not(containsString("WARNING")));
+        });
+    }
+
+    @Test
+    public void showsErrorsForSyntacticallyIncorrectModels() {
+        IntegUtils.run("model-with-syntax-error", ListUtils.of("ast"), result -> {
+            assertThat(result.getExitCode(), equalTo(1));
+            assertThat(result.getOutput(), containsString("ERROR"));
+            assertThat(result.getOutput(), containsString("bar // <- invalid syntax"));
+        });
+    }
+}
@@ -0,0 +1,6 @@
+$version: "2.0"
+namespace smithy.example
+
+structure Foo {
+    bar // <- invalid syntax
+}
@@ -0,0 +1,4 @@
+{
+    "version": "1.0",
+    "sources": ["model"]
+}
@@ -27,9 +27,9 @@
  * Command line arguments list to evaluate.
  *
  * <p>Arguments are parsed on demand. To finalize parsing arguments and
- * force validation of remaining arguments, call {@link #finishParsing()}.
+ * force validation of remaining arguments, call {@link #getPositional()}.
  * Note that because arguments are parsed on demand and whole set of
- * arguments isn't known before {@link #finishParsing()} is called, you
+ * arguments isn't known before {@link #getPositional()} is called, you
  * may need to delay configuring parts of the CLI or commands by adding
  * subscribers via {@link #onComplete(BiConsumer)}. These subscribers are
  * invoked when all arguments have been parsed.
@@ -39,6 +39,7 @@ public final class Arguments {
     private final String[] args;
     private final Map<Class<? extends ArgumentReceiver>, ArgumentReceiver> receivers = new LinkedHashMap<>();
     private final List<BiConsumer<Arguments, List<String>>> subscribers = new ArrayList<>();
+    private List<String> positional;
     private boolean inPositional = false;
     private int position = 0;
 
@@ -55,6 +56,28 @@ public <T extends ArgumentReceiver> void addReceiver(T receiver) {
         receivers.put(receiver.getClass(), receiver);
     }
 
+    /**
+     * Removes an argument receiver by class.
+     *
+     * @param receiverClass Class of receiver to remove.
+     * @return Returns the removed receiver if found, or null.
+     * @param <T> Kind of receiver to remove.
+     */
+    @SuppressWarnings("unchecked")
+    public <T extends ArgumentReceiver> T removeReceiver(Class<T> receiverClass) {
+        return (T) receivers.remove(receiverClass);
+    }
+
+    /**
+     * Check if this class contains a receiver.
+     *
+     * @param receiverClass Class of receiver to detect.
+     * @return Returns true if found.
+     */
+    public boolean hasReceiver(Class<? extends ArgumentReceiver> receiverClass) {
+        return receivers.containsKey(receiverClass);
+    }
+
     /**
      * Get a receiver by class.
      *
@@ -97,7 +120,7 @@ public boolean hasNext() {
     }
 
     /**
-     * Peeks at the next value in the arguments list without shifting it.
+     * Peeks at the next value in the argument list without shifting it.
      *
      * <p>Note that arguments consumed by a {@link ArgumentReceiver} are never
      * peeked.
@@ -134,7 +157,7 @@ public String peek() {
     }
 
     /**
-     * Shifts off the next value in the arguments list or returns null.
+     * Shifts off the next value in the argument list or returns null.
      *
      * @return Returns the next value or null.
      */
@@ -166,41 +189,41 @@ public String shiftFor(String parameter) {
     }
 
     /**
-     * Expects that all remaining arguments are positional, and returns them.
+     * Gets the positional arguments.
+     *
+     * <p>Expects that all remaining arguments are positional, and returns them.
      *
      * <p>If an argument is "--", then it's skipped and remaining arguments are
      * considered positional. If any argument is encountered that isn't valid
      * for a registered Receiver, then an error is raised. Otherwise, all remaining
      * arguments are returned in a list.
      *
-     * <p>Subscribers for different receivers are called when this method is called.
+     * <p>Subscribers for different receivers are called when this method is first called.
      *
      * @return Returns remaining arguments.
      * @throws CliError if the next argument starts with "--" but isn't "--".
      */
-    public List<String> finishParsing() {
-        List<String> positional = new ArrayList<>();
-
-        while (hasNext()) {
-            String next = shift();
-            if (next != null) {
-                if (!inPositional && next.startsWith("-")) {
-                    throw new CliError("Unexpected CLI argument: " + next);
-                } else {
-                    inPositional = true;
-                    positional.add(next);
+    public List<String> getPositional() {
+        if (positional == null) {
+            positional = new ArrayList<>();
+
+            while (hasNext()) {
+                String next = shift();
+                if (next != null) {
+                    if (!inPositional && next.startsWith("-")) {
+                        throw new CliError("Unexpected CLI argument: " + next);
+                    } else {
+                        inPositional = true;
+                        positional.add(next);
+                    }
                 }
             }
-        }
 
-        invokeSubscribers(positional);
+            for (BiConsumer<Arguments, List<String>> subscriber : subscribers) {
+                subscriber.accept(this, positional);
+            }
+        }
 
         return positional;
     }
-
-    private void invokeSubscribers(List<String> positional) {
-        for (BiConsumer<Arguments, List<String>> subscriber : subscribers) {
-            subscriber.accept(this, positional);
-        }
-    }
 }
@@ -22,6 +22,7 @@
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.shapes.ModelSerializer;
+import software.amazon.smithy.model.validation.Severity;
 
 final class AstCommand extends ClasspathCommand {
 
@@ -39,9 +40,26 @@ public String getSummary() {
         return "Reads Smithy models in and writes out a single JSON AST model.";
     }
 
+    @Override
+    protected void configureArgumentReceivers(Arguments arguments) {
+        super.configureArgumentReceivers(arguments);
+
+        // The AST command isn't meant for validation. Events are only shown when they fail the command.
+        arguments.removeReceiver(SeverityOption.class);
+    }
+
     @Override
     int runWithClassLoader(SmithyBuildConfig config, Arguments arguments, Env env, List<String> models) {
-        Model model = CommandUtils.buildModel(arguments, models, env, env.stderr(), ValidationFlag.QUIET, config);
+        Model model = new ModelBuilder()
+                .config(config)
+                .arguments(arguments)
+                .env(env)
+                .models(models)
+                .validationPrinter(env.stderr())
+                .validationMode(Validator.Mode.QUIET)
+                .severity(Severity.DANGER)
+                .build();
+
         ModelSerializer serializer = ModelSerializer.builder().build();
         env.stdout().println(Node.prettyPrintJson(serializer.serialize(model)));
         return 0;

@@ -89,8 +89,9 @@ public void registerHelp(HelpPrinter printer) {
     }
 
     @Override
-    protected void addAdditionalArgumentReceivers(List<ArgumentReceiver> receivers) {
-        receivers.add(new Options());
+    protected void configureArgumentReceivers(Arguments arguments) {
+        super.configureArgumentReceivers(arguments);
+        arguments.addReceiver(new Options());
     }
 
     @Override
@@ -99,11 +100,13 @@ int runWithClassLoader(SmithyBuildConfig config, Arguments arguments, Env env, L
         BuildOptions buildOptions = arguments.getReceiver(BuildOptions.class);
         StandardOptions standardOptions = arguments.getReceiver(StandardOptions.class);
         ClassLoader classLoader = env.classLoader();
-
-        // Build the model and fail if there are errors. Prints errors to stdout.
-        // Configure whether the build is quiet or not based on the --quiet option.
-        ValidationFlag flag = ValidationFlag.from(standardOptions);
-        Model model = CommandUtils.buildModel(arguments, models, env, env.stderr(), flag, config);
+        Model model = new ModelBuilder()
+                .config(config)
+                .arguments(arguments)
+                .env(env)
+                .models(models)
+                .validationPrinter(env.stderr())
+                .build();
 
         Supplier<ModelAssembler> modelAssemblerSupplier = () -> {
             ModelAssembler assembler = Model.assembler(classLoader);

@@ -17,108 +17,49 @@
 
 import java.util.function.Consumer;
 import software.amazon.smithy.cli.ArgumentReceiver;
-import software.amazon.smithy.cli.CliError;
 import software.amazon.smithy.cli.HelpPrinter;
-import software.amazon.smithy.cli.StandardOptions;
-import software.amazon.smithy.model.validation.Severity;
 
 /**
  * Arguments available to commands that load and build models.
  */
 final class BuildOptions implements ArgumentReceiver {
 
-    static final String SEVERITY = "--severity";
     static final String ALLOW_UNKNOWN_TRAITS = "--allow-unknown-traits";
     static final String MODELS = "<MODELS>";
 
-    private Severity severity;
-    private String discoverClasspath;
     private boolean allowUnknownTraits;
-    private boolean discover;
     private String output;
 
     @Override
     public void registerHelp(HelpPrinter printer) {
-        printer.param(SEVERITY, null, "SEVERITY", "Set the minimum reported validation severity (one of NOTE, "
-                                                  + "WARNING [default setting], DANGER, ERROR).");
         printer.option(ALLOW_UNKNOWN_TRAITS, null, "Ignore unknown traits when validating models");
-        /*
-        Hide these for now until we figure out a plan forward for these.
-        printer.option(DISCOVER, "-d", "Enable model discovery, merging in models found inside of jars");
-        printer.param(DISCOVER_CLASSPATH, null, "CLASSPATH",
-                            "Enable model discovery using a custom classpath for models");
-        */
         printer.param("--output", null, "OUTPUT_PATH",
                       "Where to write Smithy artifacts, caches, and other files (defaults to './build/smithy').");
         printer.positional(MODELS, "Model files and directories to load");
     }
 
     @Override
     public boolean testOption(String name) {
-        switch (name) {
-            case ALLOW_UNKNOWN_TRAITS:
-                allowUnknownTraits = true;
-                return true;
-            case "--discover":
-            case "-d":
-                discover = true;
-                return true;
-            default:
-                return false;
+        if (ALLOW_UNKNOWN_TRAITS.equals(name)) {
+            allowUnknownTraits = true;
+            return true;
         }
+        return false;
     }
 
     @Override
     public Consumer<String> testParameter(String name) {
-        switch (name) {
-            case "--output":
-                return value -> output = value;
-            case "--discover-classpath":
-                return value -> discoverClasspath = value;
-            case SEVERITY:
-                return value -> {
-                    severity(Severity.fromString(value).orElseThrow(() -> {
-                        return new CliError("Invalid severity level: " + value);
-                    }));
-                };
-            default:
-                return null;
+        if ("--output".equals(name)) {
+            return value -> output = value;
         }
-    }
-
-    String discoverClasspath() {
-        return discoverClasspath;
+        return null;
     }
 
     boolean allowUnknownTraits() {
         return allowUnknownTraits;
     }
 
-    boolean discover() {
-        return discover;
-    }
-
     String output() {
         return output;
     }
-
-    void severity(Severity severity) {
-        this.severity = severity;
-    }
-
-    /**
-     * Get the severity level, taking into account standard options that affect the default.
-     *
-     * @param options Standard options to query if no severity is explicitly set.
-     * @return Returns the resolved severity option.
-     */
-    Severity severity(StandardOptions options) {
-        if (severity != null) {
-            return severity;
-        } else if (options.quiet()) {
-            return Severity.DANGER;
-        } else {
-            return Severity.WARNING;
-        }
-    }
 }