Skip to content

Deprecate override in favour of triage #2005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions qlty-check/src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ 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 rayon::prelude::*;
Expand Down Expand Up @@ -268,12 +269,45 @@ 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.clone(),
category: issue_override.category.clone(),
mode: issue_override.mode,
..Default::default()
},
_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
Expand Down
3 changes: 3 additions & 0 deletions qlty-cli/tests/cmd/check/override.in/.qlty/qlty.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ batch = true
[[plugin]]
name = "exists"
version = "1.0.0"

[[override]]
plugin = "exists"
mode = "monitor"
1 change: 1 addition & 0 deletions qlty-cli/tests/cmd/check/override.stderr
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
WARNING: The `override` field in qlty.toml is deprecated. Please use `triage` instead.
✖ 1 issue
4 changes: 4 additions & 0 deletions qlty-cli/tests/cmd/check/triage.in/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.qlty/results
.qlty/logs
.qlty/out
.qlty/sources
33 changes: 33 additions & 0 deletions qlty-cli/tests/cmd/check/triage.in/.qlty/qlty.toml
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 2 additions & 0 deletions qlty-cli/tests/cmd/check/triage.in/sample.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/bin/sh
echo "$foo"
15 changes: 15 additions & 0 deletions qlty-cli/tests/cmd/check/triage.in/sample2.sh
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions qlty-cli/tests/cmd/check/triage.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
✖ 2 issues
18 changes: 18 additions & 0 deletions qlty-cli/tests/cmd/check/triage.stdout
Original file line number Diff line number Diff line change
@@ -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"
}
]
3 changes: 3 additions & 0 deletions qlty-cli/tests/cmd/check/triage.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
bin.name = "qlty"
args = ["check", "--all", "--no-cache", "--json"]
status.code = 1
6 changes: 6 additions & 0 deletions qlty-config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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>,

Expand Down
92 changes: 2 additions & 90 deletions qlty-config/src/config/overrides.rs
Original file line number Diff line number Diff line change
@@ -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>,
Expand All @@ -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
}
}
}
Loading