Deprecate override in favour of triage (#2005)

qltysh/cloud#5925

Author: marschattha

Cargo.lock

@@ -10,15 +10,17 @@ use crate::planner::config_files::PluginConfigFile;
 use crate::Settings;
 use anyhow::{bail, Error, Result};
 use check_filters::CheckFilters;
+use console::style;
 use document_url_generator::DocumentUrlGenerator;
 use itertools::Itertools;
 use qlty_analysis::cache::{Cache, FilesystemCache, NullCache};
 use qlty_analysis::git::{compute_upstream, DiffLineFilter};
 use qlty_analysis::workspace_entries::TargetMode;
 use qlty_config::config::issue_transformer::IssueTransformer;
-use qlty_config::config::{DriverType, PluginDef};
+use qlty_config::config::{DriverType, Match, PluginDef, Set, Triage};
 use qlty_config::{QltyConfig, Workspace};
 use qlty_types::analysis::v1::ExecutionVerb;
+use qlty_types::{category_from_str, level_from_str};
 use rayon::prelude::*;
 use std::collections::HashMap;
 use std::time::Instant;
@@ -271,12 +273,48 @@ impl Planner {
         self.transformers
             .push(Box::new(IssueMuter::new(self.staging_area.clone())));
 
-        // keep overrides last
-        for issue_override in &self.config.overrides {
-            self.transformers.push(Box::new(issue_override.clone()));
+        // keep triage last
+        let triages = self.build_triages();
+        for issue_triage in &triages {
+            self.transformers.push(Box::new(issue_triage.clone()));
         }
     }
 
+    fn build_triages(&self) -> Vec<Triage> {
+        let mut triages = self.config.triage.clone();
+
+        if !self.config.overrides.is_empty() {
+            eprintln!(
+                "{} The `{}` field in qlty.toml is deprecated. Please use `{}` instead.",
+                style("WARNING:").bold().yellow(),
+                style("override").bold(),
+                style("triage").bold()
+            );
+
+            for issue_override in &self.config.overrides {
+                triages.push(Triage {
+                    set: Set {
+                        level: issue_override.level.as_ref().map(|l| level_from_str(l)),
+                        category: issue_override
+                            .category
+                            .as_ref()
+                            .map(|c| category_from_str(c)),
+                        mode: issue_override.mode,
+                        ..Default::default()
+                    },
+                    r#match: Match {
+                        plugins: issue_override.plugins.clone(),
+                        rules: issue_override.rules.clone(),
+                        file_patterns: issue_override.file_patterns.clone(),
+                        ..Default::default()
+                    },
+                });
+            }
+        }
+
+        triages
+    }
+
     fn build_plan(&mut self) -> Result<Plan> {
         let target_mode = self
             .target_mode

qlty-check/src/planner.rs

@@ -12,4 +12,7 @@ batch = true
 [[plugin]]
 name = "exists"
 version = "1.0.0"
+
+[[override]]
+plugin = "exists"
 mode = "monitor"

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

@@ -1 +1,2 @@
+WARNING: The `override` field in qlty.toml is deprecated. Please use `triage` instead.
 ✖ 1 issue

qlty-cli/tests/cmd/check/override.stderr

@@ -0,0 +1,4 @@
+.qlty/results
+.qlty/logs
+.qlty/out
+.qlty/sources

qlty-cli/tests/cmd/check/triage.in/.gitignore

@@ -0,0 +1,33 @@
+config_version = "0"
+
+[plugins.definitions.triaged]
+file_types = ["shell"]
+
+[plugins.definitions.triaged.drivers.lint]
+script = "false"
+success_codes = [0]
+output = "pass_fail"
+batch = true
+
+[[plugin]]
+name = "triaged"
+version = "1.0.0"
+
+[plugins.definitions.untriaged]
+file_types = ["shell"]
+
+[plugins.definitions.untriaged.drivers.lint]
+script = "false"
+success_codes = [0]
+output = "pass_fail"
+batch = true
+
+[[plugin]]
+name = "untriaged"
+version = "1.0.0"
+
+[[triage]]
+match.plugins = ["triaged"]
+set.mode = "comment"
+set.level = "low"
+set.category = "structure"

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

@@ -0,0 +1,2 @@
+#!/bin/sh
+echo "$foo"

qlty-cli/tests/cmd/check/triage.in/sample.sh

@@ -0,0 +1,15 @@
+#/usr/bin/env bash
+rm -fr ~/.cache/qlty/tools/ruby ~/.cache/qlty/tools/rubocop
+
+export RUNTIME="ruby"
+export RUNTIME_VERSION="3.2.2"
+
+runtime_directory="/Users/bhelmkamp/.cache/qlty/tools/$RUNTIME/$RUNTIME_VERSION"
+mkdir -p $runtime_directory
+cd $runtime_directory || exit
+
+download_filename="v20230330.tar.gz"
+download_url="https://github.com/rbenv/ruby-build/archive/refs/tags/$download_filename"
+wget "$download_url"
+tar --strip-components=1 -xpvzf $download_filename
+rm $download_filename

qlty-cli/tests/cmd/check/triage.in/sample2.sh

@@ -0,0 +1 @@
+✖ 2 issues

qlty-cli/tests/cmd/check/triage.stderr

@@ -0,0 +1,18 @@
+[
+  {
+    "tool": "untriaged",
+    "ruleKey": "fail",
+    "message": "untriaged failed",
+    "level": "LEVEL_HIGH",
+    "category": "CATEGORY_LINT",
+    "mode": "MODE_BLOCK"
+  },
+  {
+    "tool": "triaged",
+    "ruleKey": "fail",
+    "message": "triaged failed",
+    "level": "LEVEL_LOW",
+    "category": "CATEGORY_STRUCTURE",
+    "mode": "MODE_COMMENT"
+  }
+]

qlty-cli/tests/cmd/check/triage.stdout

@@ -0,0 +1,3 @@
+bin.name = "qlty"
+args = ["check", "--all", "--no-cache", "--json"]
+status.code = 1

qlty-cli/tests/cmd/check/triage.toml

@@ -11,6 +11,7 @@ mod plugin;
 mod release;
 pub mod smells;
 mod source;
+pub mod triage;
 
 pub use self::ignore::{Ignore, ALL_WILDCARD};
 pub use self::overrides::Override;
@@ -29,6 +30,7 @@ pub use plugin::{
 };
 pub use release::ReleaseDef;
 pub use source::SourceDef;
+pub use triage::{Match, Set, Triage};
 
 use crate::config::plugin::EnabledRuntimes;
 pub use crate::config::plugin::PluginsConfig;
@@ -56,6 +58,10 @@ pub struct QltyConfig {
     #[serde(rename = "override")] // Since `override` is a reserved keyword
     pub overrides: Vec<Override>,
 
+    #[serde(default)]
+    #[serde(rename = "triage")]
+    pub triage: Vec<Triage>,
+
     #[serde(default)]
     pub file_types: HashMap<String, FileType>,
 

qlty-config/src/config.rs

@@ -1,14 +1,9 @@
-use super::ignore::is_rule_issue_match;
 use super::IssueMode;
-use crate::config::issue_transformer::IssueTransformer;
-use globset::{Glob, GlobSet, GlobSetBuilder};
-use qlty_types::category_from_str;
-use qlty_types::{analysis::v1::Issue, level_from_str};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::sync::RwLock;
 
-#[derive(Debug, Serialize, Deserialize, Default, JsonSchema)]
+// DEPRECATED -- Use Triage instead
+#[derive(Debug, Serialize, Deserialize, Default, Clone, JsonSchema)]
 pub struct Override {
     #[serde(default)]
     pub level: Option<String>,
@@ -27,87 +22,4 @@ pub struct Override {
 
     #[serde(default)]
     pub mode: Option<IssueMode>,
-
-    #[serde(skip)]
-    glob_set: RwLock<Option<GlobSet>>,
-}
-
-impl Clone for Override {
-    fn clone(&self) -> Self {
-        Self {
-            level: self.level.clone(),
-            category: self.category.clone(),
-            plugins: self.plugins.clone(),
-            rules: self.rules.clone(),
-            file_patterns: self.file_patterns.clone(),
-            mode: self.mode,
-            glob_set: RwLock::new(None),
-        }
-    }
-}
-
-impl IssueTransformer for Override {
-    fn initialize(&self) {
-        let mut globset_builder = GlobSetBuilder::new();
-
-        for glob in &self.file_patterns {
-            globset_builder.add(Glob::new(glob).unwrap());
-        }
-
-        let mut glob_set = self.glob_set.write().unwrap();
-        *glob_set = Some(globset_builder.build().unwrap());
-    }
-
-    fn transform(&self, issue: Issue) -> Option<Issue> {
-        if self.applies_to_issue(&issue) {
-            let mut new_issue = issue.clone();
-
-            if let Some(level) = &self.level {
-                new_issue.level = level_from_str(level).into();
-            }
-
-            if let Some(category) = &self.category {
-                new_issue.category = category_from_str(category).into();
-            }
-
-            if let Some(mode) = &self.mode {
-                new_issue.mode = *mode as i32;
-            }
-
-            Some(new_issue)
-        } else {
-            Some(issue)
-        }
-    }
-
-    fn clone_box(&self) -> Box<dyn IssueTransformer> {
-        Box::new(self.clone())
-    }
-}
-
-impl Override {
-    fn applies_to_issue(&self, issue: &Issue) -> bool {
-        self.plugin_applies_to_issue(issue)
-            && is_rule_issue_match(&self.rules, issue)
-            && self.glob_applies_to_issue(issue)
-    }
-
-    fn plugin_applies_to_issue(&self, issue: &Issue) -> bool {
-        self.plugins.is_empty() || self.plugins.contains(&issue.tool.to_string())
-    }
-
-    fn glob_applies_to_issue(&self, issue: &Issue) -> bool {
-        if self.file_patterns.is_empty() {
-            return true;
-        }
-
-        let glob_set = self.glob_set.read().unwrap();
-
-        if let Some(path) = issue.path() {
-            glob_set.as_ref().unwrap().is_match(path)
-        } else {
-            // TODO: Issues without a path are not filterable
-            false
-        }
-    }
 }