Change default behavior for missing output (#1466)

Prior to this PR, if a linter exited with a success exit code but did
not produce any output, we would hand an empty string to the parser. In
substantially all situations, this will result in a confusing EOF error
from the parser, because it will try to e.g. parse an empty string as
JSON.

This PR changes the default -- If a linter exits with a success exit
code, but does not produce output, we will treat that as a Lint Error
and skip the parser phase entirely.

This behavior can be overridden by two alternative behaviors: `parse` or
`no_issues`. `parse` will invoke the parser with an empty string, and
`no_issues` will treat the empty output as if it produced output along
the lines of `issues = []`

(Previously, we had a `missing_output_as_error` boolean. The new
`output_missing` enum replaces that.)

qlty-check/src/executor/invocation_result.rs

@@ -7,7 +7,7 @@ use anyhow::{Context, Result};
 use chrono::prelude::*;
 use qlty_analysis::utils::fs::path_to_string;
 use qlty_config::{
-    config::{OutputDestination, TargetType},
+    config::{OutputDestination, OutputMissing, TargetType},
     version::QLTY_VERSION,
 };
 use qlty_types::analysis::v1::{
@@ -273,20 +273,7 @@ impl InvocationResult {
         let tmpfile_path = self.invocation.tmpfile_path.as_ref().unwrap();
         let read_result = std::fs::read_to_string(tmpfile_path)
             .with_context(|| format!("Failed to read tmpfile contents from {}", tmpfile_path));
-
-        match read_result {
-            Ok(contents) => {
-                self.invocation.tmpfile_contents = Some(contents);
-            }
-            Err(e) => {
-                if self.plan.driver.missing_output_as_error {
-                    self.invocation.tmpfile_contents = Some("".to_string());
-                } else {
-                    self.invocation.parser_error = Some(e.to_string());
-                }
-            }
-        }
-
+        self.invocation.tmpfile_contents = read_result.ok();
         Ok(())
     }
 
@@ -320,37 +307,51 @@ impl InvocationResult {
     }
 
     fn handle_output_parsing(&mut self) -> Result<()> {
-        // If we have something to attempt to parse (which we don't if the tmpfile doesn't exist)
-        if !self.plan.uses_tmpfile() || self.invocation.tmpfile_contents.is_some() {
-            let output = if self.plan.uses_tmpfile() {
-                self.invocation.tmpfile_contents.as_ref().unwrap()
-            } else if self.plan.driver.output == OutputDestination::Stderr {
-                &self.invocation.stderr
-            } else {
-                &self.invocation.stdout
-            };
-
-            if output.is_empty() && self.plan.driver.missing_output_as_error {
-                self.invocation.exit_result =
-                    qlty_types::analysis::v1::ExitResult::UnknownError.into();
-                self.log_error_output();
-            } else {
-                let file_results = self.plan.driver.parse(output, &self.plan);
+        let output = if self.plan.uses_tmpfile() {
+            self.invocation
+                .tmpfile_contents
+                .as_ref()
+                .unwrap_or(&String::new())
+                .to_owned()
+        } else if self.plan.driver.output == OutputDestination::Stderr {
+            self.invocation.stderr.to_owned()
+        } else {
+            self.invocation.stdout.to_owned()
+        };
 
-                match file_results {
-                    Ok(file_results) => {
-                        self.file_results = Some(file_results);
-                    }
-                    Err(e) => {
-                        self.invocation.parser_error = Some(e.to_string());
-                    }
+        if output.is_empty() {
+            match self.plan.driver.output_missing {
+                OutputMissing::Error => {
+                    self.invocation.exit_result =
+                        qlty_types::analysis::v1::ExitResult::UnknownError.into();
+                    self.log_error_output();
                 }
+                OutputMissing::NoIssues => {
+                    self.invocation.exit_result =
+                        qlty_types::analysis::v1::ExitResult::NoIssues.into();
+                }
+                OutputMissing::Parse => self.parse_output(output),
             }
+        } else {
+            self.parse_output(output);
         }
 
         Ok(())
     }
 
+    fn parse_output(&mut self, output: String) {
+        let file_results = self.plan.driver.parse(&output, &self.plan);
+
+        match file_results {
+            Ok(file_results) => {
+                self.file_results = Some(file_results);
+            }
+            Err(e) => {
+                self.invocation.parser_error = Some(e.to_string());
+            }
+        }
+    }
+
     fn create_file_result_for_autofmts(&self) -> Result<Vec<FileResult>> {
         let mut file_results: Vec<FileResult> = Vec::new();
 

qlty-cli/tests/cmd/check/missing_output_as_error.in/.qlty/qlty.toml

@@ -8,7 +8,6 @@ prepare_script = "mkdir ${linter} && echo dir %2 > ${linter}/ls.cmd || echo dir
 script = "echo \"The plugin crashed for some reason\""
 success_codes = [0]
 output = "tmpfile"
-missing_output_as_error = true
 
 [[plugin]]
 name = "exists"

qlty-config/src/config.rs

@@ -23,8 +23,8 @@ pub use language::Language;
 pub use plugin::{
     CheckTrigger, DriverBatchBy, DriverDef, DriverType, EnabledPlugin, ExtraPackage,
     InvocationDirectoryDef, InvocationDirectoryType, IssueMode, OutputDestination, OutputFormat,
-    PackageFileCandidate, Platform, PluginDef, PluginEnvironment, PluginFetch, Runtime,
-    SuggestionMode, TargetDef, TargetType,
+    OutputMissing, PackageFileCandidate, Platform, PluginDef, PluginEnvironment, PluginFetch,
+    Runtime, SuggestionMode, TargetDef, TargetType,
 };
 pub use release::ReleaseDef;
 pub use source::SourceDef;

qlty-config/src/config/plugin.rs

@@ -40,6 +40,10 @@ pub struct DriverDef {
 
     #[serde(default)]
     pub output_format: OutputFormat,
+
+    #[serde(default)]
+    pub output_missing: OutputMissing,
+
     pub output_regex: Option<String>,
 
     #[serde(default)]
@@ -111,9 +115,6 @@ pub struct DriverDef {
 
     #[serde(default)]
     pub autoload_script: Option<String>,
-
-    #[serde(default)]
-    pub missing_output_as_error: bool,
 }
 
 fn default_driver_timeout() -> u64 {
@@ -124,6 +125,22 @@ fn default_max_batch() -> usize {
     64
 }
 
+#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash, Default, JsonSchema)]
+pub enum OutputMissing {
+    /// Raise an error if the output is missing
+    #[default]
+    #[serde(rename = "error")]
+    Error,
+
+    /// Interpret no output as no issues
+    #[serde(rename = "no_issues")]
+    NoIssues,
+
+    /// Hand the empty output to the parser for processing
+    #[serde(rename = "parse")]
+    Parse,
+}
+
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash, Default, JsonSchema)]
 pub enum DriverBatchBy {
     #[default]

qlty-plugins/plugins/linters/eslint/plugin.toml

@@ -75,4 +75,3 @@ batch = true
 cache_results = true
 max_batch = 40
 known_good_version = "4.19.1"
-missing_output_as_error = true

qlty-plugins/plugins/linters/kube-linter/plugin.toml

@@ -46,5 +46,4 @@ output = "stdout"
 output_format = "sarif"
 output_category = "vulnerability"
 output_level = "medium"
-missing_output_as_error = true
 target = { type = "literal", path = "." }

qlty-plugins/plugins/linters/radarlint/plugin.toml

@@ -15,6 +15,7 @@ script = "java -jar ${linter}/radarlint.jar ${target}"
 success_codes = [0]
 output = "stdout"
 output_format = "radarlint"
+output_missing = "parse"
 batch = true
 cache_results = true
 suggested = "targets"