Skip to content

Commit d33bd71

Browse files
sbdchdsbdchd
authored
linter: simplify violation creation (#614)
1 parent 4b9d86f commit d33bd71

30 files changed

+162
-161
lines changed

‎crates/squawk_linter/src/ignore.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ pub(crate) fn find_ignores(ctx: &mut Linter, file: &SyntaxNode) {
102102
Rule::UnusedIgnore,
103103
format!("unknown name {trimmed}"),
104104
range,
105-
None,
106105
));
107106
}
108107

‎crates/squawk_linter/src/lib.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -219,16 +219,6 @@ impl fmt::Display for Rule {
219219
}
220220
}
221221

222-
#[derive(Debug, Clone, PartialEq, Eq)]
223-
pub struct Violation {
224-
// TODO: should this be String instead?
225-
pub code: Rule,
226-
pub message: String,
227-
pub text_range: TextRange,
228-
pub help: Option<String>,
229-
pub fix: Option<Fix>,
230-
}
231-
232222
#[derive(Debug, Clone, PartialEq, Eq)]
233223
pub struct Fix {
234224
pub title: String,
@@ -258,14 +248,19 @@ impl Edit {
258248
}
259249
}
260250

251+
#[derive(Debug, Clone, PartialEq, Eq)]
252+
pub struct Violation {
253+
// TODO: should this be String instead?
254+
pub code: Rule,
255+
pub message: String,
256+
pub text_range: TextRange,
257+
pub help: Option<String>,
258+
pub fix: Option<Fix>,
259+
}
260+
261261
impl Violation {
262262
#[must_use]
263-
pub fn for_node(
264-
code: Rule,
265-
message: String,
266-
node: &SyntaxNode,
267-
help: impl Into<Option<String>>,
268-
) -> Self {
263+
pub fn for_node(code: Rule, message: String, node: &SyntaxNode) -> Self {
269264
let range = node.text_range();
270265

271266
let start = node
@@ -280,31 +275,30 @@ impl Violation {
280275
code,
281276
text_range: TextRange::new(start, range.end()),
282277
message,
283-
help: help.into(),
278+
help: None,
284279
fix: None,
285280
}
286281
}
287282

288283
#[must_use]
289-
pub fn for_range(
290-
code: Rule,
291-
message: String,
292-
text_range: TextRange,
293-
help: impl Into<Option<String>>,
294-
) -> Self {
284+
pub fn for_range(code: Rule, message: String, text_range: TextRange) -> Self {
295285
Self {
296286
code,
297287
text_range,
298288
message,
299-
help: help.into(),
289+
help: None,
300290
fix: None,
301291
}
302292
}
303293

304-
fn with_fix(mut self, fix: Option<Fix>) -> Violation {
294+
fn fix(mut self, fix: Option<Fix>) -> Violation {
305295
self.fix = fix;
306296
self
307297
}
298+
fn help(mut self, help: impl Into<String>) -> Violation {
299+
self.help = Some(help.into());
300+
self
301+
}
308302
}
309303

310304
#[derive(Default)]

‎crates/squawk_linter/src/rules/adding_field_with_default.rs‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,24 @@ pub(crate) fn adding_field_with_default(ctx: &mut Linter, parse: &Parse<SourceFi
7373
if is_const_expr(&expr) || is_non_volatile(&expr) {
7474
continue;
7575
}
76-
ctx.report(Violation::for_node(
77-
Rule::AddingFieldWithDefault,
78-
message.into(),
79-
expr.syntax(),
80-
help.to_string(),
81-
))
76+
ctx.report(
77+
Violation::for_node(
78+
Rule::AddingFieldWithDefault,
79+
message.into(),
80+
expr.syntax(),
81+
)
82+
.help(help),
83+
)
8284
}
8385
ast::Constraint::GeneratedConstraint(generated) => {
84-
ctx.report(Violation::for_node(
85-
Rule::AddingFieldWithDefault,
86-
message.into(),
87-
generated.syntax(),
88-
help.to_string(),
89-
));
86+
ctx.report(
87+
Violation::for_node(
88+
Rule::AddingFieldWithDefault,
89+
message.into(),
90+
generated.syntax(),
91+
)
92+
.help(help),
93+
);
9094
}
9195
_ => (),
9296
}

‎crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,14 @@ pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse<Sour
2525
ast::Constraint::ForeignKeyConstraint(_)
2626
| ast::Constraint::ReferencesConstraint(_)
2727
) {
28-
ctx.report(Violation::for_node(
29-
Rule::AddingForeignKeyConstraint,
30-
message.into(),
31-
constraint.syntax(),
32-
help.to_string(),
33-
))
28+
ctx.report(
29+
Violation::for_node(
30+
Rule::AddingForeignKeyConstraint,
31+
message.into(),
32+
constraint.syntax(),
33+
)
34+
.help(help),
35+
)
3436
}
3537
}
3638
}
@@ -41,12 +43,14 @@ pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse<Sour
4143
ast::Constraint::ForeignKeyConstraint(_)
4244
| ast::Constraint::ReferencesConstraint(_)
4345
) {
44-
ctx.report(Violation::for_node(
45-
Rule::AddingForeignKeyConstraint,
46-
message.into(),
47-
constraint.syntax(),
48-
help.to_string(),
49-
))
46+
ctx.report(
47+
Violation::for_node(
48+
Rule::AddingForeignKeyConstraint,
49+
message.into(),
50+
constraint.syntax(),
51+
)
52+
.help(help),
53+
)
5054
}
5155
}
5256
}

‎crates/squawk_linter/src/rules/adding_not_null_field.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ pub(crate) fn adding_not_null_field(ctx: &mut Linter, parse: &Parse<SourceFile>)
2121
"Setting a column `NOT NULL` blocks reads while the table is scanned."
2222
.into(),
2323
option.syntax(),
24-
"Make the field nullable and use a `CHECK` constraint instead."
25-
.to_string(),
26-
));
24+
).help("Make the field nullable and use a `CHECK` constraint instead."));
2725
}
2826
}
2927
}

‎crates/squawk_linter/src/rules/adding_primary_key_constraint.rs‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ pub(crate) fn adding_primary_key_constraint(ctx: &mut Linter, parse: &Parse<Sour
1818
add_constraint.constraint()
1919
{
2020
if primary_key_constraint.using_index().is_none() {
21-
ctx.report(Violation::for_node(
22-
Rule::AddingSerialPrimaryKeyField,
23-
message.to_string(),
24-
primary_key_constraint.syntax(),
25-
help.to_string(),
26-
));
21+
ctx.report(
22+
Violation::for_node(
23+
Rule::AddingSerialPrimaryKeyField,
24+
message.to_string(),
25+
primary_key_constraint.syntax(),
26+
)
27+
.help(help),
28+
);
2729
}
2830
}
2931
}
@@ -33,12 +35,14 @@ pub(crate) fn adding_primary_key_constraint(ctx: &mut Linter, parse: &Parse<Sour
3335
constraint
3436
{
3537
if primary_key_constraint.using_index().is_none() {
36-
ctx.report(Violation::for_node(
37-
Rule::AddingSerialPrimaryKeyField,
38-
message.to_string(),
39-
primary_key_constraint.syntax(),
40-
help.to_string(),
41-
));
38+
ctx.report(
39+
Violation::for_node(
40+
Rule::AddingSerialPrimaryKeyField,
41+
message.to_string(),
42+
primary_key_constraint.syntax(),
43+
)
44+
.help(help),
45+
);
4246
}
4347
}
4448
}

‎crates/squawk_linter/src/rules/adding_required_field.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ pub(crate) fn adding_required_field(ctx: &mut Linter, parse: &Parse<SourceFile>)
1919
Rule::AddingRequiredField,
2020
"Adding a new column that is `NOT NULL` and has no default value to an existing table effectively makes it required.".into(),
2121
add_column.syntax(),
22-
"Make the field nullable or add a non-VOLATILE DEFAULT".to_string(),
23-
));
22+
).help("Make the field nullable or add a non-VOLATILE DEFAULT"));
2423
}
2524
}
2625
}

‎crates/squawk_linter/src/rules/ban_alter_domain_with_add_constraint.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ pub(crate) fn ban_alter_domain_with_add_constraint(ctx: &mut Linter, parse: &Par
1616
Rule::BanAlterDomainWithAddConstraint,
1717
"Domains with constraints have poor support for online migrations. Use table and column constraints instead.".into(),
1818
add_constraint.syntax(),
19-
None,
2019
))
2120
}
2221
}

‎crates/squawk_linter/src/rules/ban_char_field.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ fn check_path_type(ctx: &mut Linter, path_type: ast::PathType) {
2424
Rule::BanCharField,
2525
"Using `character` is likely a mistake and should almost always be replaced by `text` or `varchar`.".into(),
2626
path_type.syntax(),
27-
None,
2827
));
2928
}
3029
}
@@ -36,7 +35,6 @@ fn check_char_type(ctx: &mut Linter, char_type: ast::CharType) {
3635
Rule::BanCharField,
3736
"Using `character` is likey a mistake and should almost always be replaced by `text` or `varchar`.".into(),
3837
char_type.syntax(),
39-
None,
4038
));
4139
}
4240
}

‎crates/squawk_linter/src/rules/ban_concurrent_index_creation_in_transaction.rs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ pub(crate) fn ban_concurrent_index_creation_in_transaction(
2626
Rule::BanConcurrentIndexCreationInTransaction,
2727
"While regular index creation can happen inside a transaction, this is not allowed when the `CONCURRENTLY` option is used.".into(),
2828
concurrently.text_range(),
29-
"Build the index outside any transactions.".to_string(),
30-
));
29+
).help("Build the index outside any transactions."));
3130
}
3231
}
3332
}
@@ -45,7 +44,10 @@ pub(crate) fn ban_concurrent_index_creation_in_transaction(
4544
mod test {
4645
use insta::assert_debug_snapshot;
4746

48-
use crate::{Rule, test_utils::{lint, lint_with_assume_in_transaction}};
47+
use crate::{
48+
Rule,
49+
test_utils::{lint, lint_with_assume_in_transaction},
50+
};
4951

5052
#[test]
5153
fn ban_concurrent_index_creation_in_transaction_err() {
@@ -77,7 +79,8 @@ mod test {
7779
CREATE UNIQUE INDEX CONCURRENTLY "field_name_idx" ON "table_name" ("field_name");
7880
ALTER TABLE "table_name" ADD CONSTRAINT "field_name_id" UNIQUE USING INDEX "field_name_idx";
7981
"#;
80-
let errors = lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
82+
let errors =
83+
lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
8184
assert_ne!(errors.len(), 0);
8285
assert_debug_snapshot!(errors);
8386
}
@@ -88,7 +91,8 @@ mod test {
8891
-- run index creation in a standalone migration
8992
CREATE UNIQUE INDEX CONCURRENTLY "field_name_idx" ON "table_name" ("field_name");
9093
"#;
91-
let errors = lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
94+
let errors =
95+
lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
9296
assert_eq!(errors.len(), 0);
9397
}
9498

@@ -101,7 +105,8 @@ mod test {
101105
BEGIN;
102106
ALTER TABLE "table_name" ADD CONSTRAINT "field_name_id" UNIQUE USING INDEX "field_name_idx";
103107
"#;
104-
let errors = lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
108+
let errors =
109+
lint_with_assume_in_transaction(sql, Rule::BanConcurrentIndexCreationInTransaction);
105110
assert_eq!(errors.len(), 0);
106111
}
107112
}

0 commit comments

Comments
 (0)