Skip to content

Commit 78d5ed9

Browse files
sbdchdsbdchd
authored
cli: include param for default disabled rules (#1073)
We have `exclude` to disable rules and now we have `include` to enable rules. There's also a config option. cc @Flaiers
1 parent 892cec9 commit 78d5ed9

24 files changed

Lines changed: 287 additions & 148 deletions

‎README.md‎

Lines changed: 51 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -108,65 +108,78 @@ Found 6 issues in 1 file (checked 1 source file)
108108
### `squawk --help`
109109
110110
```
111-
squawk
112111
Find problems in your SQL
113112

114-
USAGE:
115-
squawk [FLAGS] [OPTIONS] [path]... [SUBCOMMAND]
113+
Usage: squawk [OPTIONS] [path]... [COMMAND]
116114

117-
FLAGS:
118-
--assume-in-transaction
119-
Assume that a transaction will wrap each SQL file when run by a migration tool
115+
Commands:
116+
server Run the language server
117+
upload-to-github Comment on a PR with Squawk's results
118+
help Print this message or the help of the given subcommand(s)
120119
121-
Use --no-assume-in-transaction to override this setting in any config file that exists
122-
-h, --help
123-
Prints help information
120+
Arguments:
121+
[path]...
122+
Paths or patterns to search
124123
125-
-V, --version
126-
Prints version information
124+
Options:
125+
--exclude-path <EXCLUDED_PATH>
126+
Paths to exclude
127127
128-
--verbose
129-
Enable debug logging output
128+
For example:
130129
130+
`--exclude-path=005_user_ids.sql --exclude-path=009_account_emails.sql`
131131
132-
OPTIONS:
133-
-c, --config <config-path>
134-
Path to the squawk config file (.squawk.toml)
132+
`--exclude-path='*user_ids.sql'`
135133
136-
--debug <format>
137-
Output debug info [possible values: Lex, Parse]
134+
-e, --exclude <rule>
135+
Exclude specific warnings
138136
139-
--exclude-path <excluded-path>...
140-
Paths to exclude
137+
For example: --exclude=require-concurrent-index-creation,ban-drop-database
141138
142-
For example: --exclude-path=005_user_ids.sql --exclude-path=009_account_emails.sql
139+
-i, --include <rule>
140+
Include opt-in rules that are disabled by default
143141
144-
--exclude-path='*user_ids.sql'
142+
Rules listed in --exclude take precedence over --include.
145143
146-
-e, --exclude <rule>...
147-
Exclude specific warnings
144+
For example: --include=require-table-schema
148145
149-
For example: --exclude=require-concurrent-index-creation,ban-drop-database
146+
--pg-version <PG_VERSION>
147+
Specify postgres version
150148
151-
--pg-version <pg-version>
152-
Specify postgres version
149+
For example: --pg-version=13.0
153150
154-
For example: --pg-version=13.0
155-
--reporter <reporter>
156-
Style of error reporting [possible values: Tty, Gcc, Json]
151+
--debug <format>
152+
Output debug format
157153
158-
--stdin-filepath <filepath>
159-
Path to use in reporting for stdin
154+
[possible values: lex, parse, ast]
160155
156+
--reporter <REPORTER>
157+
Style of error reporting
161158
162-
ARGS:
163-
<path>...
164-
Paths to search
159+
[possible values: tty, gcc, json, gitlab]
165160
161+
--stdin-filepath <filepath>
162+
Path to use in reporting for stdin
166163
167-
SUBCOMMANDS:
168-
help Prints this message or the help of the given subcommand(s)
169-
upload-to-github Comment on a PR with Squawk's results
164+
--verbose
165+
Enable debug logging output
166+
167+
-c, --config <CONFIG_PATH>
168+
Path to the squawk config file (.squawk.toml)
169+
170+
--assume-in-transaction
171+
Assume that a transaction will wrap each SQL file when run by a migration tool
172+
173+
Use --no-assume-in-transaction to override any config file that sets this
174+
175+
--no-error-on-unmatched-pattern
176+
Do not exit with an error when provided path patterns do not match any files
177+
178+
-h, --help
179+
Print help (see a summary with '-h')
180+
181+
-V, --version
182+
Print version
170183
```
171184
172185
## Rules

‎crates/squawk/src/cmd.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ impl Cmd {
5858
return Cmd::Lint(LintArgs {
5959
input,
6060
excluded_rules: conf.excluded_rules,
61+
included_rules: conf.included_rules,
6162
pg_version: conf.pg_version,
6263
assume_in_transaction: conf.assume_in_transaction,
6364
reporter: conf.reporter,

‎crates/squawk/src/config.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ pub struct ConfigFile {
2626
#[serde(default)]
2727
pub excluded_rules: Vec<Rule>,
2828
#[serde(default)]
29+
pub included_rules: Vec<Rule>,
30+
#[serde(default)]
2931
pub pg_version: Option<Version>,
3032
#[serde(default)]
3133
pub assume_in_transaction: Option<bool>,
@@ -56,6 +58,7 @@ impl ConfigFile {
5658
pub struct Config {
5759
pub excluded_paths: Vec<String>,
5860
pub excluded_rules: Vec<Rule>,
61+
pub included_rules: Vec<Rule>,
5962
pub pg_version: Option<Version>,
6063
pub assume_in_transaction: bool,
6164
pub upload_to_github: UploadToGitHubConfig,
@@ -86,6 +89,13 @@ impl Config {
8689
conf.excluded_rules.clone()
8790
};
8891

92+
// the --include flag completely overrides the configuration file.
93+
let included_rules = if let Some(included_rules) = opts.included_rules {
94+
included_rules
95+
} else {
96+
conf.included_rules.clone()
97+
};
98+
8999
// the --exclude-path flag completely overrides the configuration file.
90100
let excluded_paths = if let Some(excluded_paths) = opts.excluded_path {
91101
excluded_paths
@@ -115,6 +125,7 @@ impl Config {
115125

116126
info!("pg version: {pg_version:?}");
117127
info!("excluded rules: {:?}", &excluded_rules);
128+
info!("included rules: {:?}", &included_rules);
118129
info!("excluded paths: {:?}", &excluded_paths);
119130
info!("assume in a transaction: {assume_in_transaction:?}");
120131
info!("no error on unmatched pattern: {no_error_on_unmatched_pattern:?}");
@@ -138,6 +149,7 @@ impl Config {
138149
Config {
139150
excluded_paths,
140151
excluded_rules,
152+
included_rules,
141153
pg_version,
142154
assume_in_transaction,
143155
upload_to_github,
@@ -246,6 +258,16 @@ fail_on_violations = true
246258
assert_debug_snapshot!(ConfigFile::parse(Some(squawk_toml.path().to_path_buf())));
247259
}
248260
#[test]
261+
fn load_included_rules() {
262+
let squawk_toml = NamedTempFile::new().expect("generate tempFile");
263+
let file = r#"
264+
included_rules = ["require-table-schema"]
265+
266+
"#;
267+
fs::write(&squawk_toml, file).expect("Unable to write file");
268+
assert_debug_snapshot!(ConfigFile::parse(Some(squawk_toml.path().to_path_buf())));
269+
}
270+
#[test]
249271
fn load_excluded_rules_with_alias() {
250272
let squawk_toml = NamedTempFile::new().expect("generate tempFile");
251273
let file = r#"

‎crates/squawk/src/github.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ pub fn check_and_comment_on_pr(cfg: Config) -> Result<()> {
121121
let file_results = lint_files(&LintArgs {
122122
input: Input::Paths(found_paths),
123123
excluded_rules: cfg.excluded_rules,
124+
included_rules: cfg.included_rules,
124125
pg_version: cfg.pg_version,
125126
assume_in_transaction: cfg.assume_in_transaction,
126127
reporter: cfg.reporter,

‎crates/squawk/src/main.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,20 @@ struct Opts {
108108
global = true
109109
)]
110110
excluded_rules: Option<Vec<Rule>>,
111+
/// Include opt-in rules that are disabled by default
112+
///
113+
/// Rules listed in --exclude take precedence over --include.
114+
///
115+
/// For example:
116+
/// --include=require-table-schema
117+
#[arg(
118+
short = 'i',
119+
long = "include",
120+
value_name = "rule",
121+
value_delimiter = ',',
122+
global = true
123+
)]
124+
included_rules: Option<Vec<Rule>>,
111125
/// Specify postgres version
112126
///
113127
/// For example:

‎crates/squawk/src/reporter.rs‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@ use crate::{
2222
fn check_sql(
2323
sql: &str,
2424
path: &str,
25+
included_rules: &[Rule],
2526
excluded_rules: &[Rule],
2627
pg_version: Option<Version>,
2728
assume_in_transaction: bool,
2829
) -> CheckReport {
29-
let mut linter = Linter::without_rules(excluded_rules);
30+
let mut linter = Linter::with_rules(included_rules, excluded_rules);
3031
if let Some(pg_version) = pg_version {
3132
linter.settings.pg_version = pg_version;
3233
}
@@ -142,6 +143,7 @@ fn render_lint_error<W: std::io::Write>(
142143
pub(crate) struct LintArgs {
143144
pub(crate) input: Input,
144145
pub(crate) excluded_rules: Vec<Rule>,
146+
pub(crate) included_rules: Vec<Rule>,
145147
pub(crate) pg_version: Option<Version>,
146148
pub(crate) assume_in_transaction: bool,
147149
pub(crate) reporter: Reporter,
@@ -162,6 +164,7 @@ pub fn lint_files(args: &LintArgs) -> Result<Vec<CheckReport>> {
162164
let content = check_sql(
163165
&sql,
164166
&path,
167+
&args.included_rules,
165168
&args.excluded_rules,
166169
args.pg_version,
167170
args.assume_in_transaction,
@@ -176,6 +179,7 @@ pub fn lint_files(args: &LintArgs) -> Result<Vec<CheckReport>> {
176179
let content = check_sql(
177180
&sql,
178181
path.to_str().unwrap(),
182+
&args.included_rules,
179183
&args.excluded_rules,
180184
args.pg_version,
181185
args.assume_in_transaction,
@@ -491,7 +495,7 @@ mod test_check_files {
491495
select \;
492496
";
493497
let mut buff = Vec::new();
494-
let res = check_sql(sql, "test.sql", &[], None, false);
498+
let res = check_sql(sql, "test.sql", &[], &[], None, false);
495499
fmt_json(&mut buff, vec![res]).unwrap();
496500

497501
let val: Value = serde_json::from_slice(&buff).unwrap();
@@ -502,7 +506,7 @@ select \;
502506
fn skip_lint_on_syntax_error() {
503507
let error_sql = "ALTER TABLE foo ALTER CONSTRAINT bar RENAME TO quux;";
504508
let mut buff = vec![];
505-
let res = check_sql(error_sql, "test.sql", &[], None, false);
509+
let res = check_sql(error_sql, "test.sql", &[], &[], None, false);
506510
fmt_json(&mut buff, vec![res]).unwrap();
507511
assert_snapshot!(String::from_utf8_lossy(&buff), @r#"[{"file":"test.sql","line":0,"column":36,"level":"Error","message":"missing comma","help":null,"rule_name":"syntax-error","column_end":36,"line_end":0}]"#);
508512
}
@@ -529,7 +533,7 @@ SELECT 1;
529533

530534
let res = print_violations(
531535
&mut buff,
532-
vec![check_sql(sql, filename, &[], None, false)],
536+
vec![check_sql(sql, filename, &[], &[], None, false)],
533537
&Reporter::Gcc,
534538
false,
535539
);
@@ -562,7 +566,7 @@ SELECT 1;
562566

563567
let res = print_violations(
564568
&mut buff,
565-
vec![check_sql(sql, filename, &[], None, false)],
569+
vec![check_sql(sql, filename, &[], &[], None, false)],
566570
&Reporter::Tty,
567571
true,
568572
);
@@ -584,7 +588,7 @@ SELECT 1;
584588

585589
let res = print_violations(
586590
&mut buff,
587-
vec![check_sql(sql, filename, &[], None, false)],
591+
vec![check_sql(sql, filename, &[], &[], None, false)],
588592
&Reporter::Tty,
589593
false,
590594
);
@@ -600,7 +604,7 @@ SELECT 1;
600604

601605
let res = print_violations(
602606
&mut buff,
603-
vec![check_sql(sql, "main.sql", &[], None, false)],
607+
vec![check_sql(sql, "main.sql", &[], &[], None, false)],
604608
&Reporter::Tty,
605609
false,
606610
);
@@ -622,7 +626,7 @@ SELECT 1;
622626

623627
let res = print_violations(
624628
&mut buff,
625-
vec![check_sql(sql, filename, &[], None, false)],
629+
vec![check_sql(sql, filename, &[], &[], None, false)],
626630
&Reporter::Json,
627631
false,
628632
);
@@ -643,7 +647,7 @@ SELECT 1;
643647

644648
let res = print_violations(
645649
&mut buff,
646-
vec![check_sql(sql, filename, &[], None, false)],
650+
vec![check_sql(sql, filename, &[], &[], None, false)],
647651
&Reporter::Gitlab,
648652
false,
649653
);
@@ -662,6 +666,6 @@ ALTER TABLE "core_foo" ADD COLUMN "bar" integer NOT NULL;
662666
SELECT 1;
663667
"#;
664668
let filename = "main.sql";
665-
assert_debug_snapshot!(check_sql(sql, filename, &[], None, false));
669+
assert_debug_snapshot!(check_sql(sql, filename, &[], &[], None, false));
666670
}
667671
}

‎crates/squawk/src/snapshots/squawk__config__test_config__load_assume_in_transaction.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Ok(
77
ConfigFile {
88
excluded_paths: [],
99
excluded_rules: [],
10+
included_rules: [],
1011
pg_version: None,
1112
assume_in_transaction: Some(
1213
false,

‎crates/squawk/src/snapshots/squawk__config__test_config__load_cfg_full.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Ok(
1111
excluded_rules: [
1212
RequireConcurrentIndexCreation,
1313
],
14+
included_rules: [],
1415
pg_version: Some(
1516
Version {
1617
major: 19,

‎crates/squawk/src/snapshots/squawk__config__test_config__load_excluded_paths.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Ok(
99
"example.sql",
1010
],
1111
excluded_rules: [],
12+
included_rules: [],
1213
pg_version: None,
1314
assume_in_transaction: None,
1415
upload_to_github: UploadToGitHubConfig {

‎crates/squawk/src/snapshots/squawk__config__test_config__load_excluded_rules.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Ok(
99
excluded_rules: [
1010
RequireConcurrentIndexCreation,
1111
],
12+
included_rules: [],
1213
pg_version: None,
1314
assume_in_transaction: None,
1415
upload_to_github: UploadToGitHubConfig {

0 commit comments

Comments
 (0)