Fix a bug that prevented warning events to show up for diff CLI command

The severity is set to DANGER to load the old and new models to avoid
events of lesser severity to show up during loading. After this was
done the option was mutated and the attempt to restore the default
severity didn't work. Now, the validation option keeps a different
field to distinguish the value read from the CLI from overrides to be
able to tell them apart and properly reset these when needed.

Author: sugmanue

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

@@ -298,11 +298,9 @@ private static void discoverModelsWithClasspath(String rawClasspath, ModelAssemb
     }
 
     // Determine a default severity if one wasn't given, by inspecting if there is a --severity option.
-    private Severity resolveMinSeverity(StandardOptions standardOptions, ValidatorOptions validatorOption) {
-        if (defaultSeverity != null && validatorOption.severity() == null) {
-            validatorOption.severity(defaultSeverity);
-        }
-        return validatorOption.detectAndGetSeverity(standardOptions);
+    private void resolveMinSeverity(StandardOptions standardOptions, ValidatorOptions validatorOption) {
+        validatorOption.severityOverride(defaultSeverity);
+        validatorOption.detectAndSetSeverity(standardOptions);
     }
 
     static ModelAssembler createModelAssembler(ClassLoader classLoader) {

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

@@ -24,6 +24,7 @@ final class ValidatorOptions implements ArgumentReceiver {
     static final String SHOW_VALIDATORS = "--show-validators";
     static final String HIDE_VALIDATORS = "--hide-validators";
 
+    private Severity severityOverride;
     private Severity severity;
     private List<String> showValidators = Collections.emptyList();
     private List<String> hideValidators = Collections.emptyList();
@@ -107,21 +108,28 @@ void severity(Severity severity) {
         this.severity = severity;
     }
 
+    /**
+     * Set a severity override
+     *
+     * @param severity Severity to set.
+     */
+    void severityOverride(Severity severity) {
+        this.severityOverride = severity;
+    }
+
     /**
      * Set and get the severity level, taking into account standard options that affect the default.
      *
      * @param standardOptions  Standard options to query if no severity is explicitly set.
-     * @return Returns the resolved severity option.
      */
-    Severity detectAndGetSeverity(StandardOptions standardOptions) {
+    void detectAndSetSeverity(StandardOptions standardOptions) {
         if (severity == null) {
             if (standardOptions.quiet()) {
                 severity = Severity.DANGER;
             } else {
                 severity = Severity.WARNING;
             }
         }
-        return severity;
     }
 
     /**
@@ -160,6 +168,18 @@ void hideValidators(List<String> validators) {
         this.hideValidators = validators;
     }
 
+    /**
+     * Returns the current severity.
+     *
+     *  @return The validation severity
+     */
+    Severity getSeverity() {
+        if (severityOverride != null) {
+            return severityOverride;
+        }
+        return severity;
+    }
+
     /**
      * Check if the given validation event matches the show/hide settings.
      *
@@ -169,7 +189,8 @@ void hideValidators(List<String> validators) {
      * @return Return true if the event can be seen.
      */
     boolean isVisible(ValidationEvent event) {
-        if (event.getSeverity().ordinal() < severity.ordinal()) {
+
+        if (event.getSeverity().ordinal() < getSeverity().ordinal()) {
             return false;
         }
 

smithy-cli/src/test/java/software/amazon/smithy/cli/commands/DiffCommandTest.java

@@ -38,4 +38,33 @@ public void canOutputCsv() throws Exception {
         assertThat(lines[0], containsString("severity,id,shape,file,line,column,message,hint,suppressionReason"));
         assertThat(lines[1], containsString("\"ERROR\",\"ChangedShapeType\",\"smithy.example#Hello\""));
     }
+
+    @Test
+    public void showsWarningEventsByDefault() throws Exception {
+        Path oldModel = Paths.get(getClass().getResource("diff/old2.smithy").toURI());
+        Path newModel = Paths.get(getClass().getResource("diff/new2.smithy").toURI());
+        CliUtils.Result result = CliUtils.runSmithy("diff",
+                "--old", oldModel.toString(),
+                "--new", newModel.toString());
+
+        assertThat(result.code(), is(0));
+
+        String[] lines = result.stdout().split("(\\r\\n|\\r|\\n)");
+        assertThat(lines[1], containsString("WARNING"));
+        assertThat(lines[1], containsString("TraitBreakingChange.Add.smithy.api#pattern"));
+    }
+
+    @Test
+    public void doesNotShowWarningEventsWhenSeverityIsSetToDanger() throws Exception {
+        Path oldModel = Paths.get(getClass().getResource("diff/old2.smithy").toURI());
+        Path newModel = Paths.get(getClass().getResource("diff/new2.smithy").toURI());
+        CliUtils.Result result = CliUtils.runSmithy("diff",
+                // warning events won't be shown in the output
+                "--severity", "DANGER",
+                "--old", oldModel.toString(),
+                "--new", newModel.toString());
+
+        assertThat(result.code(), is(0));
+        assertThat(result.stdout(), is(""));
+    }
 }

smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/new2.smithy

@@ -0,0 +1,14 @@
+$version: "2.0"
+
+namespace smithy.example
+
+
+structure StructureOne {
+
+    @pattern("^.*$")
+    stringMember: String
+
+    @clientOptional
+    @required
+    intMember: Integer
+}

smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/diff/old2.smithy

@@ -0,0 +1,10 @@
+$version: "2.0"
+
+namespace smithy.example
+
+structure StructureOne {
+
+    stringMember: String
+
+    intMember: Integer
+}