diff --git a/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java b/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java index aee1e28fdb5..5708a784766 100644 --- a/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java +++ b/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)); + }); + } } diff --git a/smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/diff-example-conflict-with-simple/model/main.smithy b/smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/diff-example-conflict-with-simple/model/main.smithy new file mode 100644 index 00000000000..d40b478b22c --- /dev/null +++ b/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 diff --git a/smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/diff-example-conflict-with-simple/smithy-build.json b/smithy-cli/src/it/resources/software/amazon/smithy/cli/projects/diff-example-conflict-with-simple/smithy-build.json new file mode 100644 index 00000000000..db4a958216c --- /dev/null +++ b/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"] +} diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ConfigOptions.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ConfigOptions.java index 8a624353769..a517b7f8aa6 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ConfigOptions.java +++ b/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 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 config() { List 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()); diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java index 190f0ab8408..08a73137943 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java +++ b/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; @@ -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 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 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 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 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); } } diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ModelBuilder.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ModelBuilder.java index 1a986c780ce..b9020bee8c9 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ModelBuilder.java +++ b/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(); } diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java index 4241ee94954..728642db73b 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/PrettyAnsiValidationFormatter.java +++ b/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) { diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/SelectCommand.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/SelectCommand.java index e95f761c339..ae44f7729f1 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/SelectCommand.java +++ b/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(); diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/StyleHelper.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/StyleHelper.java new file mode 100644 index 00000000000..727d014ec59 --- /dev/null +++ b/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; + } +} diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/Validator.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/Validator.java index 686c3ba13bc..347cb5f4120 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/Validator.java +++ b/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;