Add --mode arbitrary|project to smithy diff

This commit adds a --mode option to Smithy diff to support the
arbitrary mode (the current mode used to compare two models), and
the project mode used to compare the current project against another
project or model. When running in project mode, the imports and
sources of the current model are used to load the "new" model
(the current project), but they aren't used to load the "old" model.
If the --old argument points to a directory that contains a
smithy-build.json file, its imports and sources are used when loading
the old model, though its dependencies are ignored and it's loaded
using the resolved classpath of the new model. This ensures that the
models are comparable and won't cause comparison issues when trying
to compare things like traits across class loaders.

smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java

@@ -19,6 +19,7 @@
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 
 import java.io.FileWriter;
 import java.io.IOException;
@@ -120,4 +121,45 @@ public void requiresOldAndNewForArbitraryMode() {
         assertThat(result.getExitCode(), is(1));
         assertThat(result.getOutput(), containsString("Missing required --old argument"));
     }
+
+    @Test
+    public void doesNotAllowNewWithProjectMode() {
+        RunResult result = IntegUtils.run(Paths.get("."), ListUtils.of("diff",
+                                                                       "--mode", "project",
+                                                                       "--new", "x",
+                                                                       "--old", "y"));
+
+        assertThat(result.getExitCode(), is(1));
+        assertThat(result.getOutput(), containsString("--new cannot be used with this diff mode"));
+    }
+
+    @Test
+    public void projectModeUsesConfigOfOldModel() {
+        IntegUtils.withProject("diff-example-conflict-with-simple", outer -> {
+            IntegUtils.withProject("simple-config-sources", dir -> {
+                RunResult result = IntegUtils.run(dir, ListUtils.of(
+                        "diff",
+                        "--mode",
+                        "project",
+                        "--old",
+                        outer.toString()));
+                assertThat(result.getOutput(), containsString("ChangedShapeType"));
+                assertThat(result.getExitCode(), equalTo(1));
+            });
+        });
+    }
+
+    @Test
+    public void projectModeCanDiffAgainstSingleFile() {
+        // Diff against itself (the only model file of the project), so there should be no differences.
+        IntegUtils.withProject("simple-config-sources", dir -> {
+            RunResult result = IntegUtils.run(dir, ListUtils.of(
+                    "diff",
+                    "--mode",
+                    "project",
+                    "--old",
+                    dir.resolve("model").resolve("main.smithy").toString()));
+            assertThat(result.getExitCode(), equalTo(0));
+        });
+    }
 }

smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/diff-example-conflict-with-simple/model/main.smithy

@@ -0,0 +1,9 @@
+$version: "2.0"
+namespace smithy.example
+
+// See the DiffCommandTest#projectModeUsesConfigOfOldModel integration test.
+//
+// This is to cause a diff event when compared against simple-config-sources, ensuring that the config file of this
+// project is used and not the config file of the "new" model. If the new model's config was also loaded, the
+// integration test would create a shape conflict error after loading.
+integer MyString

smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/diff-example-conflict-with-simple/smithy-build.json

@@ -0,0 +1,4 @@
+{
+    "version": "1.0",
+    "sources": ["model"]
+}

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ConfigOptions.java

@@ -35,6 +35,7 @@ final class ConfigOptions implements ArgumentReceiver {
     private static final Logger LOGGER = Logger.getLogger(ConfigOptions.class.getName());
     private final List<String> config = new ArrayList<>();
     private boolean noConfig = false;
+    private Path root;
 
     @Override
     public void registerHelp(HelpPrinter printer) {
@@ -65,12 +66,18 @@ public boolean testOption(String name) {
         }
     }
 
+    void root(Path root) {
+        this.root = root;
+    }
+
     List<String> config() {
         List<String> config = this.config;
 
         // Don't find the default config if --no-config is passed.
         if (config.isEmpty() && !noConfig) {
-            Path defaultConfig = Paths.get("smithy-build.json").toAbsolutePath();
+            Path defaultConfig = root != null
+                    ? root.resolve("smithy-build.json").toAbsolutePath()
+                    : Paths.get("smithy-build.json").toAbsolutePath();
             if (Files.exists(defaultConfig)) {
                 LOGGER.fine("Detected smithy-build.json at " + defaultConfig);
                 config = Collections.singletonList(defaultConfig.toString());

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java

@@ -15,14 +15,16 @@
 
 package software.amazon.smithy.cli.commands;
 
+import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.function.Consumer;
-import java.util.logging.Logger;
 import software.amazon.smithy.build.model.SmithyBuildConfig;
 import software.amazon.smithy.cli.ArgumentReceiver;
 import software.amazon.smithy.cli.Arguments;
 import software.amazon.smithy.cli.CliError;
+import software.amazon.smithy.cli.ColorFormatter;
 import software.amazon.smithy.cli.ColorTheme;
 import software.amazon.smithy.cli.Command;
 import software.amazon.smithy.cli.HelpPrinter;
@@ -35,7 +37,6 @@
 
 final class DiffCommand implements Command {
 
-    private static final Logger LOGGER = Logger.getLogger(DiffCommand.class.getName());
     private final String parentCommandName;
     private final DependencyResolver.Factory dependencyResolverFactory;
 
@@ -51,7 +52,7 @@ public String getName() {
 
     @Override
     public String getSummary() {
-        return "Compares two Smithy models and reports differences.";
+        return "Compares two Smithy models and detects differences.";
     }
 
     @Override
@@ -63,12 +64,46 @@ public int execute(Arguments arguments, Env env) {
         arguments.getReceiver(BuildOptions.class).noPositionalArguments(true);
 
         CommandAction action = HelpActionWrapper.fromCommand(
-                this, parentCommandName, new ClasspathAction(dependencyResolverFactory, this::runWithClassLoader));
+            this,
+            parentCommandName,
+            this::getDocumentation,
+            new ClasspathAction(dependencyResolverFactory, this::runWithClassLoader)
+        );
 
         return action.apply(arguments, env);
     }
 
+    private String getDocumentation(ColorFormatter colors) {
+        String ls = System.lineSeparator();
+        String content =
+                "The `diff` command supports different modes through the `--mode` option:"
+                + ls
+                + ls
+                + "`--mode arbitrary`:"
+                + ls
+                + "Compares two arbitrary models. This mode requires that `--old` and `--new` are specified. "
+                + "When run within a project directory that contains a `smithy-build.json` config, any dependencies "
+                + "defined in the config file are used when loading both the old and new models; however, `imports` "
+                + "and `sources` defined in the config file are not used. This is the default mode when no `--mode` "
+                + "is specified."
+                + ls
+                + ls
+                + "`--mode project`:"
+                + ls
+                + "Compares the current state of a project against another project. `--old` is required and points "
+                + "to the location of another Smithy model or the root directory of another project. `--new` is not "
+                + "allowed in this mode because the new model is the project in the current working directory. The "
+                + "old model does not use any `sources` or `imports` defined by the current project, though it is "
+                + "loaded using any dependencies defined by the current project. If the `--old` argument points to "
+                + "a directory that contains a `smithy-build.json` file, any `imports` or `sources` defined in that "
+                + "config file will be used when loading the old model, though the dependencies of the old model "
+                + "are ignored.";
+
+        return StyleHelper.markdownLiterals(content, colors);
+    }
+
     private static final class Options implements ArgumentReceiver {
+        private DiffMode diffMode = DiffMode.ARBITRARY;
         private String oldModel;
         private String newModel;
 
@@ -79,12 +114,21 @@ public boolean testOption(String name) {
 
         @Override
         public Consumer<String> testParameter(String name) {
-            if (name.equals("--old")) {
-                return m -> oldModel = m;
-            } else if (name.equals("--new")) {
-                return n -> newModel = n;
-            } else {
-                return null;
+            switch (name) {
+                case "--old":
+                    return m -> oldModel = m;
+                case "--new":
+                    return n -> newModel = n;
+                case "--mode":
+                    return m -> {
+                        try {
+                            diffMode = DiffMode.valueOf(m.toUpperCase(Locale.ENGLISH));
+                        } catch (IllegalArgumentException e) {
+                            throw new CliError("Invalid --diff mode provided: " + m);
+                        }
+                    };
+                default:
+                    return null;
             }
         }
 
@@ -94,6 +138,8 @@ public void registerHelp(HelpPrinter printer) {
                           "Path to an old Smithy model file or directory that contains model files.");
             printer.param("--new", null, "NEW_MODEL",
                           "Path to the new Smithy model file or directory that contains model files.");
+            printer.param("--mode", null, "DIFF_MODE",
+                          "The diff mode to use: 'arbitrary' (default mode), 'project'.");
         }
     }
 
@@ -104,64 +150,102 @@ int runWithClassLoader(SmithyBuildConfig config, Arguments arguments, Env env) {
             throw new CliError("Unexpected arguments: " + arguments.getPositional());
         }
 
-        // TODO: Add checks here for `mode` to change the DiffMode. Defaults to arbitrary.
-        return DiffMode.ARBITRARY.diff(config, arguments, options, env);
+        return options.diffMode.diff(config, arguments, options, env);
     }
 
     private enum DiffMode {
         ARBITRARY {
             @Override
             int diff(SmithyBuildConfig config, Arguments arguments, Options options, Env env) {
-                String oldModelOpt = options.oldModel;
-                if (oldModelOpt == null) {
+                if (options.oldModel == null) {
                     throw new CliError("Missing required --old argument");
                 }
 
-                String newModelOpt = options.newModel;
-                if (newModelOpt == null) {
+                if (options.newModel == null) {
                     throw new CliError("Missing required --new argument");
                 }
 
-                LOGGER.fine(() -> String.format("Setting old models to: %s; new models to: %s",
-                                                oldModelOpt, newModelOpt));
-
-                ModelBuilder modelBuilder = new ModelBuilder()
-                        .config(config)
-                        .arguments(arguments)
-                        .env(env)
-                        .validationPrinter(env.stderr())
-                        // Don't use imports or sources from the model config file.
-                        .disableConfigModels(true)
-                        // Only report issues that fail the build.
-                        .validationMode(Validator.Mode.DISABLE)
-                        .severity(Severity.DANGER);
+                ModelBuilder modelBuilder = createModelBuilder(config, arguments, env);
+                // Don't use imports or sources from the model config file.
+                modelBuilder.disableConfigModels(true);
 
                 // Use the ModelBuilder template to build the old model.
                 Model oldModel = modelBuilder
-                        .models(Collections.singletonList(oldModelOpt))
+                        .models(Collections.singletonList(options.oldModel))
                         .titleLabel("OLD", ColorTheme.DIFF_EVENT_TITLE)
                         .build();
 
-                // Use the same ModelBuilder template to build the new model.
-                Model newModel = modelBuilder
-                        .models(Collections.singletonList(newModelOpt))
-                        .titleLabel("NEW", ColorTheme.DIFF_EVENT_TITLE)
-                        .build();
+                // Use the same ModelBuilder template to build the new model, being careful to use the original config.
+                Model newModel = createNewModel(modelBuilder, Collections.singletonList(options.newModel), config);
+                runDiff(modelBuilder, env, oldModel, newModel);
+                return 0;
+            }
+        },
+
+        PROJECT {
+            @Override
+            int diff(SmithyBuildConfig config, Arguments arguments, Options options, Env env) {
+                if (options.oldModel == null) {
+                    throw new CliError("Missing required --old argument");
+                }
+
+                if (options.newModel != null) {
+                    throw new CliError("--new cannot be used with this diff mode");
+                }
+
+                ModelBuilder modelBuilder = createModelBuilder(config, arguments, env);
+
+                // Use the ModelBuilder template to build the old model.
+                // The old model is built as if we're rooted in the given directory, allowing ConfigOptions to look
+                // for a smithy-build.json file, which can then use imports and sources.
+                ConfigOptions oldConfig = new ConfigOptions();
+                oldConfig.root(Paths.get(options.oldModel));
 
-                // Diff the models and report on the events, failing if necessary.
-                // We *do* use dependencies in smithy-build.json (if present) to find custom diff evaluators.
-                ClassLoader classLoader = env.classLoader();
-                List<ValidationEvent> events = ModelDiff.compare(classLoader, oldModel, newModel);
-                modelBuilder
-                        .titleLabel("DIFF", ColorTheme.DIFF_TITLE)
-                        .validatedResult(new ValidatedResult<>(newModel, events))
-                        .severity(null) // reset so it takes on standard option settings.
+                Model oldModel = modelBuilder
+                        .models(Collections.emptyList())
+                        .config(oldConfig.createSmithyBuildConfig())
+                        .titleLabel("OLD", ColorTheme.DIFF_EVENT_TITLE)
                         .build();
 
+                // Use the same ModelBuilder template to build the new model, being careful to use the original config.
+                Model newModel = createNewModel(modelBuilder, Collections.emptyList(), config);
+                runDiff(modelBuilder, env, oldModel, newModel);
                 return 0;
             }
         };
 
+        // Create a ModelBuilder template to load the old, then new, then diff both.
+        protected ModelBuilder createModelBuilder(SmithyBuildConfig config, Arguments arguments, Env env) {
+            return new ModelBuilder()
+                    .config(config)
+                    .arguments(arguments)
+                    .env(env)
+                    .validationPrinter(env.stderr())
+                    // Only report issues that fail the build.
+                    .validationMode(Validator.Mode.QUIET_CORE_ONLY)
+                    .severity(Severity.DANGER);
+        }
+
+        // Creating a new model is the same for each diff mode.
+        protected Model createNewModel(ModelBuilder builder, List<String> models, SmithyBuildConfig config) {
+            return builder
+                    .models(models)
+                    .titleLabel("NEW", ColorTheme.DIFF_EVENT_TITLE)
+                    .config(config)
+                    .build();
+        }
+
+        // Running the diff is the same for each diff mode.
+        protected void runDiff(ModelBuilder builder, Env env, Model oldModel, Model newModel) {
+            ClassLoader classLoader = env.classLoader();
+            List<ValidationEvent> events = ModelDiff.compare(classLoader, oldModel, newModel);
+            builder
+                    .titleLabel("DIFF", ColorTheme.DIFF_TITLE)
+                    .validatedResult(new ValidatedResult<>(newModel, events))
+                    .severity(null) // reset so it takes on standard option settings.
+                    .build();
+        }
+
         abstract int diff(SmithyBuildConfig config, Arguments arguments, Options options, Env env);
     }
 }

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ModelBuilder.java

@@ -139,7 +139,7 @@ public Model build() {
         if (validatedResult == null) {
             ModelAssembler assembler = createModelAssembler(classLoader);
 
-            if (validationMode == Validator.Mode.DISABLE) {
+            if (validationMode == Validator.Mode.QUIET_CORE_ONLY) {
                 assembler.disableValidation();
             }
 

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java

@@ -18,7 +18,6 @@
 import java.io.UncheckedIOException;
 import java.nio.file.Paths;
 import java.util.Collection;
-import java.util.regex.Pattern;
 import software.amazon.smithy.cli.ColorBuffer;
 import software.amazon.smithy.cli.ColorFormatter;
 import software.amazon.smithy.cli.ColorTheme;
@@ -28,12 +27,10 @@
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.ValidationEventFormatter;
 import software.amazon.smithy.utils.SmithyBuilder;
-import software.amazon.smithy.utils.StringUtils;
 
 final class PrettyAnsiValidationFormatter implements ValidationEventFormatter {
 
     private static final int LINE_LENGTH = 80;
-    private static final Pattern TICK_PATTERN = Pattern.compile("`(.*?)`");
     private final SourceContextLoader sourceContextLoader;
     private final ColorFormatter colors;
     private final String rootPath = Paths.get("").normalize().toAbsolutePath().toString();
@@ -140,13 +137,7 @@ private void printTitle(ColorBuffer writer, ValidationEvent event, Style border,
 
     // Converts Markdown style ticks to use color highlights if colors are enabled.
     private void writeMessage(ColorBuffer writer, String message) {
-        String content = StringUtils.wrap(message, LINE_LENGTH, System.lineSeparator(), false);
-
-        if (colors.isColorEnabled()) {
-            content = TICK_PATTERN.matcher(content).replaceAll(colors.style("$1", ColorTheme.LITERAL));
-        }
-
-        writer.append(content);
+        writer.append(StyleHelper.formatMessage(message, LINE_LENGTH, colors));
     }
 
     private String getHumanReadableFilename(String filename) {

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/SelectCommand.java

@@ -164,7 +164,7 @@ private int runWithClassLoader(SmithyBuildConfig config, Arguments arguments, En
                 .env(env)
                 .models(arguments.getPositional())
                 .validationPrinter(env.stderr())
-                .validationMode(Validator.Mode.DISABLE)
+                .validationMode(Validator.Mode.QUIET_CORE_ONLY)
                 .severity(Severity.DANGER)
                 .build();
 

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/StyleHelper.java

@@ -0,0 +1,46 @@
+/*
+ * 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.commands;
+
+import java.util.regex.Pattern;
+import software.amazon.smithy.cli.ColorFormatter;
+import software.amazon.smithy.cli.ColorTheme;
+import software.amazon.smithy.utils.StringUtils;
+
+final class StyleHelper {
+
+    private static final Pattern TICK_PATTERN = Pattern.compile("`(.*?)`");
+
+    private StyleHelper() {}
+
+    // Converts Markdown style ticks to use color highlights if colors are enabled.
+    static String formatMessage(String message, int lineLength, ColorFormatter colors) {
+        String content = StringUtils.wrap(message, lineLength, System.lineSeparator(), false);
+
+        if (colors.isColorEnabled()) {
+            content = markdownLiterals(content, colors);
+        }
+
+        return content;
+    }
+
+    static String markdownLiterals(String content, ColorFormatter colors) {
+        if (colors.isColorEnabled()) {
+            content = TICK_PATTERN.matcher(content).replaceAll(colors.style("$1", ColorTheme.LITERAL));
+        }
+        return content;
+    }
+}

smithy-cli/src/main/java/software/amazon/smithy/cli/commands/Validator.java

@@ -32,7 +32,7 @@
 final class Validator {
 
     enum Mode {
-        QUIET, DISABLE, ENABLE;
+        QUIET, QUIET_CORE_ONLY, ENABLE;
 
         static Mode from(StandardOptions standardOptions) {
             return standardOptions.quiet() ? Mode.QUIET : Mode.ENABLE;