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

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,38 @@ 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
+}