Skip to content

Commit 08949f4

Browse files
gvozdvmozgubenfdking
gvozdvmozgu
authored and
benfdking
committed
refactor: remove legacy severity flags from SQLBaseError and streamline formatters
1 parent 54365f7 commit 08949f4

File tree

10 files changed

+21
-78
lines changed

10 files changed

+21
-78
lines changed

‎crates/cli-python/tests/json/hql_file.stdout

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"tests/lint/hql_file.hql":[{"range":{"start":{"line":1,"character":7},"end":{"line":1,"character":7}},"message":"Expected only single space before \"1\". Found \" \".","severity":"Error","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Files must end with a single trailing newline.","severity":"Error","source":"sqruff","code":"LT12"}]}
1+
{"tests/lint/hql_file.hql":[{"range":{"start":{"line":1,"character":7},"end":{"line":1,"character":7}},"message":"Expected only single space before \"1\". Found \" \".","severity":"Warning","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Files must end with a single trailing newline.","severity":"Warning","source":"sqruff","code":"LT12"}]}

‎crates/cli-python/tests/json/test_fail_whitespace_before_comma.stdout

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"tests/lint/test_fail_whitespace_before_comma.sql":[{"range":{"start":{"line":1,"character":8},"end":{"line":1,"character":8}},"message":"Column expression without alias. Use explicit `AS` clause.","severity":"Error","source":"sqruff","code":"AL03"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Column expression without alias. Use explicit `AS` clause.","severity":"Error","source":"sqruff","code":"AL03"},{"range":{"start":{"line":1,"character":9},"end":{"line":1,"character":9}},"message":"Unexpected whitespace before comma.","severity":"Error","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Expected single whitespace between \",\" and \"4\".","severity":"Error","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":12},"end":{"line":1,"character":12}},"message":"Files must end with a single trailing newline.","severity":"Error","source":"sqruff","code":"LT12"}]}
1+
{"tests/lint/test_fail_whitespace_before_comma.sql":[{"range":{"start":{"line":1,"character":8},"end":{"line":1,"character":8}},"message":"Column expression without alias. Use explicit `AS` clause.","severity":"Warning","source":"sqruff","code":"AL03"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Column expression without alias. Use explicit `AS` clause.","severity":"Warning","source":"sqruff","code":"AL03"},{"range":{"start":{"line":1,"character":9},"end":{"line":1,"character":9}},"message":"Unexpected whitespace before comma.","severity":"Warning","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Expected single whitespace between \",\" and \"4\".","severity":"Warning","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":12},"end":{"line":1,"character":12}},"message":"Files must end with a single trailing newline.","severity":"Warning","source":"sqruff","code":"LT12"}]}

‎crates/cli/tests/json/hql_file.stdout

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"tests/lint/hql_file.hql":[{"range":{"start":{"line":1,"character":7},"end":{"line":1,"character":7}},"message":"Expected only single space before \"1\". Found \" \".","severity":"Error","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Files must end with a single trailing newline.","severity":"Error","source":"sqruff","code":"LT12"}]}
1+
{"tests/lint/hql_file.hql":[{"range":{"start":{"line":1,"character":7},"end":{"line":1,"character":7}},"message":"Expected only single space before \"1\". Found \" \".","severity":"Warning","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Files must end with a single trailing newline.","severity":"Warning","source":"sqruff","code":"LT12"}]}

‎crates/cli/tests/json/test_fail_whitespace_before_comma.stdout

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"tests/lint/test_fail_whitespace_before_comma.sql":[{"range":{"start":{"line":1,"character":8},"end":{"line":1,"character":8}},"message":"Column expression without alias. Use explicit `AS` clause.","severity":"Error","source":"sqruff","code":"AL03"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Column expression without alias. Use explicit `AS` clause.","severity":"Error","source":"sqruff","code":"AL03"},{"range":{"start":{"line":1,"character":9},"end":{"line":1,"character":9}},"message":"Unexpected whitespace before comma.","severity":"Error","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Expected single whitespace between \",\" and \"4\".","severity":"Error","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":12},"end":{"line":1,"character":12}},"message":"Files must end with a single trailing newline.","severity":"Error","source":"sqruff","code":"LT12"}]}
1+
{"tests/lint/test_fail_whitespace_before_comma.sql":[{"range":{"start":{"line":1,"character":8},"end":{"line":1,"character":8}},"message":"Column expression without alias. Use explicit `AS` clause.","severity":"Warning","source":"sqruff","code":"AL03"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Column expression without alias. Use explicit `AS` clause.","severity":"Warning","source":"sqruff","code":"AL03"},{"range":{"start":{"line":1,"character":9},"end":{"line":1,"character":9}},"message":"Unexpected whitespace before comma.","severity":"Warning","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":11},"end":{"line":1,"character":11}},"message":"Expected single whitespace between \",\" and \"4\".","severity":"Warning","source":"sqruff","code":"LT01"},{"range":{"start":{"line":1,"character":12},"end":{"line":1,"character":12}},"message":"Files must end with a single trailing newline.","severity":"Warning","source":"sqruff","code":"LT12"}]}

‎crates/lib-core/src/errors.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,12 @@ pub trait SqlError: Display {
2020

2121
#[derive(Debug, PartialEq, Clone, Default)]
2222
pub struct SQLBaseError {
23-
pub fatal: bool,
24-
pub ignore: bool,
25-
pub warning: bool,
23+
pub fixable: bool,
2624
pub line_no: usize,
2725
pub line_pos: usize,
2826
pub description: String,
2927
pub rule: Option<ErrorStructRule>,
3028
pub source_slice: Range<usize>,
31-
pub fixable: bool,
3229
}
3330

3431
#[derive(Debug, PartialEq, Clone, Default)]
@@ -233,11 +230,9 @@ impl From<SQLParseError> for SQLBaseError {
233230
}
234231

235232
Self::default().config(|this| {
236-
this.fatal = true;
237233
this.line_no = line_no;
238234
this.line_pos = line_pos;
239235
this.description = value.description;
240-
this.fixable = false;
241236
})
242237
}
243238
}

‎crates/lib/src/cli/formatters.rs

+4-26
Original file line numberDiff line numberDiff line change
@@ -112,18 +112,10 @@ impl OutputStreamFormatter {
112112
fn format_file_violations(&self, fname: &str, mut violations: Vec<SQLBaseError>) -> String {
113113
let mut text_buffer = String::new();
114114

115-
let fails = violations
116-
.iter()
117-
.filter(|violation| !violation.ignore && !violation.warning)
118-
.count();
119-
let warns = violations
120-
.iter()
121-
.filter(|violation| violation.warning)
122-
.count();
123-
let show = fails + warns > 0;
115+
let show = !violations.is_empty();
124116

125117
if self.verbosity > 0 || show {
126-
let text = self.format_filename(fname, fails == 0);
118+
let text = self.format_filename(fname, !show);
127119
text_buffer.push_str(&text);
128120
text_buffer.push('\n');
129121
}
@@ -174,33 +166,19 @@ impl OutputStreamFormatter {
174166
max_line_length: usize,
175167
) -> String {
176168
let violation: SQLBaseError = violation.into();
177-
let desc = violation.desc();
178-
179-
let severity = if violation.ignore {
180-
"IGNORE: "
181-
} else if violation.warning {
182-
"WARNING: "
183-
} else {
184-
""
185-
};
169+
let mut desc = violation.desc().to_string();
186170

187171
let line_elem = format!("{:4}", violation.line_no);
188172
let pos_elem = format!("{:4}", violation.line_pos);
189173

190-
let mut desc = format!("{severity}{desc}");
191-
192174
if let Some(rule) = &violation.rule {
193175
let text = self.colorize(rule.name, LIGHT_GREY);
194176
let text = format!(" [{text}]");
195177
desc.push_str(&text);
196178
}
197179

198180
let split_desc = split_string_on_spaces(&desc, max_line_length - 25);
199-
let mut section_color = if violation.ignore || violation.warning {
200-
LIGHT_GREY
201-
} else {
202-
AnsiColor::Blue.on_default()
203-
};
181+
let mut section_color = AnsiColor::Blue.on_default();
204182

205183
let mut out_buff = String::new();
206184
for (idx, line) in enumerate(split_desc) {

‎crates/lib/src/cli/github_annotation_native_formatter.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,9 @@ impl Formatter for GithubAnnotationNativeFormatter {
6464
violation.rule.as_ref().unwrap().code,
6565
violation.description
6666
);
67+
6768
self.dispatch(&message);
68-
if !violation.ignore && !violation.warning {
69-
self.has_fail.store(true, Ordering::SeqCst);
70-
}
69+
self.has_fail.store(true, Ordering::SeqCst);
7170
}
7271
}
7372

‎crates/lib/src/cli/json.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ impl Formatter for JsonFormatter {
2828
let lock = self.violations.lock().unwrap();
2929
lock.values().any(|v| {
3030
v.iter()
31-
.any(|d| matches!(&d.severity, DiagnosticSeverity::Error))
31+
.any(|d| matches!(&d.severity, DiagnosticSeverity::Warning))
3232
})
3333
}
3434

‎crates/lib/src/cli/json_types.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,7 @@ impl From<SQLBaseError> for Diagnostic {
1212
end: Position::new(value.line_no as u32, value.line_pos as u32),
1313
},
1414
message: value.description,
15-
severity: if value.warning {
16-
DiagnosticSeverity::Warning
17-
} else {
18-
DiagnosticSeverity::Error
19-
},
15+
severity: DiagnosticSeverity::Warning,
2016
source: Some("sqruff".to_string()),
2117
code,
2218
// code: todo!(),

‎crates/lib/src/core/rules/noqa.rs

+8-33
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,12 @@ impl NoQADirective {
5858
for rule in rules {
5959
if !available_rules.contains(rule.as_str()) {
6060
return Err(SQLBaseError {
61-
fatal: true,
62-
ignore: false,
63-
warning: false,
61+
fixable: false,
6462
line_no: 0,
6563
line_pos: 0,
6664
description: format!("Rule {} not found in rule set", rule),
6765
rule: None,
6866
source_slice: Default::default(),
69-
fixable: false,
7067
});
7168
}
7269
}
@@ -126,16 +123,13 @@ impl NoQADirective {
126123
.collect();
127124
if rules.is_empty() {
128125
Err(SQLBaseError {
129-
fatal: true,
130-
ignore: false,
131-
warning: false,
126+
fixable: false,
132127
line_no,
133128
line_pos,
134129
description: "Malformed 'noqa' section. Expected 'noqa: <rule>[,...] | all'"
135130
.into(),
136131
rule: None,
137132
source_slice: Default::default(),
138-
fixable: false,
139133
})
140134
} else {
141135
Ok(Some(NoQADirective::RangeIgnoreRules(RangeIgnoreRules {
@@ -164,17 +158,14 @@ impl NoQADirective {
164158
.collect();
165159
if rules.is_empty() {
166160
Err(SQLBaseError {
167-
fatal: true,
168-
ignore: false,
169-
warning: false,
161+
fixable: false,
170162
line_no,
171163
line_pos,
172164
description:
173165
"Malformed 'noqa' section. Expected 'noqa: <rule>[,...]'"
174166
.to_string(),
175167
rule: None,
176168
source_slice: Default::default(),
177-
fixable: false,
178169
})
179170
} else {
180171
Ok(Some(NoQADirective::RangeIgnoreRules(RangeIgnoreRules {
@@ -190,17 +181,14 @@ impl NoQADirective {
190181
let rules = comment.split(",").map_into().collect::<HashSet<String>>();
191182
if rules.is_empty() {
192183
Err(SQLBaseError {
193-
fatal: true,
194-
ignore: false,
195-
warning: false,
184+
fixable: false,
196185
line_no,
197186
line_pos,
198187
description:
199188
"Malformed 'noqa' section. Expected 'noqa: <rule>[,...] | all'"
200189
.into(),
201190
rule: None,
202191
source_slice: Default::default(),
203-
fixable: false,
204192
})
205193
} else {
206194
return Ok(Some(NoQADirective::LineIgnoreRules(LineIgnoreRules {
@@ -212,32 +200,26 @@ impl NoQADirective {
212200
}
213201
} else {
214202
Err(SQLBaseError {
215-
fatal: true,
216-
ignore: false,
217-
warning: false,
203+
fixable: false,
218204
line_no,
219205
line_pos,
220206
description:
221207
"Malformed 'noqa' section. Expected 'noqa: <rule>[,...] | all'"
222208
.into(),
223209
rule: None,
224210
source_slice: Default::default(),
225-
fixable: false,
226211
})
227212
}
228213
} else {
229214
Err(SQLBaseError {
230-
fatal: true,
231-
ignore: false,
232-
warning: false,
215+
fixable: false,
233216
line_no,
234217
line_pos,
235218
description:
236219
"Malformed 'noqa' section. Expected 'noqa' or 'noqa: <rule>[,...]'"
237220
.to_string(),
238221
rule: None,
239222
source_slice: Default::default(),
240-
fixable: false,
241223
})
242224
}
243225
} else {
@@ -315,15 +297,12 @@ impl IgnoreMask {
315297
let (line_no, line_pos) = comment
316298
.get_position_marker()
317299
.ok_or(SQLBaseError {
318-
fatal: true,
319-
ignore: false,
320-
warning: false,
300+
fixable: false,
321301
line_no: 0,
322302
line_pos: 0,
323303
description: "Could not get position marker".to_string(),
324304
rule: None,
325305
source_slice: Default::default(),
326-
fixable: false,
327306
})?
328307
.source_position();
329308
NoQADirective::parse_from_comment(comment_content, line_no, line_pos)
@@ -480,9 +459,7 @@ mod tests {
480459
#[test]
481460
fn test_is_masked_single_line() {
482461
let error = SQLBaseError {
483-
fatal: false,
484-
ignore: false,
485-
warning: false,
462+
fixable: true,
486463
line_no: 2,
487464
line_pos: 11,
488465
description: "Implicit/explicit aliasing of columns.".to_string(),
@@ -491,7 +468,6 @@ mod tests {
491468
code: "AL02",
492469
}),
493470
source_slice: Default::default(),
494-
fixable: true,
495471
};
496472
let mask = IgnoreMask {
497473
ignore_list: vec![NoQADirective::LineIgnoreRules(LineIgnoreRules {
@@ -613,7 +589,6 @@ mod tests {
613589
assert!(result.is_err());
614590
let result_err = result.err().unwrap();
615591
assert_eq!(result_err.description, err);
616-
assert!(result_err.fatal);
617592
}
618593
}
619594
}

0 commit comments

Comments
 (0)