Skip to content
Merged
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
108 changes: 105 additions & 3 deletions crates/pgls_suppressions/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'a> SuppressionsParser<'a> {
}

/// Will parse the suppressions at the start of the file.
/// As soon as anything is encountered that's not a `pgt-ignore-all`
/// As soon as anything is encountered that's not a `pgt-ignore-all` or `pgls-ignore-all`
/// suppression or an empty line, this will stop.
fn parse_file_suppressions(&mut self) {
while let Some((_, preview)) = self.lines.peek() {
Expand All @@ -65,7 +65,10 @@ impl<'a> SuppressionsParser<'a> {
continue;
}

if !preview.trim().starts_with("-- pgt-ignore-all") {
let trimmed = preview.trim();
if !trimmed.starts_with("-- pgt-ignore-all")
&& !trimmed.starts_with("-- pgls-ignore-all")
{
return;
}

Expand All @@ -82,7 +85,8 @@ impl<'a> SuppressionsParser<'a> {

fn parse_suppressions(&mut self) {
for (idx, line) in self.lines.by_ref() {
if !line.trim().starts_with("-- pgt-ignore") {
let trimmed = line.trim();
if !trimmed.starts_with("-- pgt-ignore") && !trimmed.starts_with("-- pgls-ignore") {
continue;
}

Expand Down Expand Up @@ -350,4 +354,102 @@ drop table posts;
String::from("This start suppression does not have a matching end.")
);
}

#[test]
fn test_parse_pgls_prefix_line_suppressions() {
let doc = r#"
SELECT 1;
-- pgls-ignore lint/safety/banDropColumn
SELECT 2;
"#;
let suppressions = SuppressionsParser::parse(doc);

// Should have a line suppression on line 2 (0-based index)
let suppression = suppressions
.line_suppressions
.get(&2)
.expect("no suppression found");

assert_eq!(suppression.kind, SuppressionKind::Line);
assert_eq!(
suppression.rule_specifier,
RuleSpecifier::Rule(
"lint".to_string(),
"safety".to_string(),
"banDropColumn".to_string()
)
);
}

#[test]
fn test_parse_pgls_prefix_file_suppressions() {
let doc = r#"
-- pgls-ignore-all lint
-- pgls-ignore-all typecheck

SELECT 1;
-- pgls-ignore-all lint/safety
"#;

let suppressions = SuppressionsParser::parse(doc);

assert_eq!(suppressions.diagnostics.len(), 1);
assert_eq!(suppressions.file_suppressions.len(), 2);

assert_eq!(
suppressions.file_suppressions[0].rule_specifier,
RuleSpecifier::Category("lint".to_string())
);
assert_eq!(
suppressions.file_suppressions[1].rule_specifier,
RuleSpecifier::Category("typecheck".to_string())
);
}

#[test]
fn test_parse_pgls_prefix_range_suppressions() {
let doc = r#"
-- pgls-ignore-start lint/safety/banDropTable
drop table users;
drop table auth;
drop table posts;
-- pgls-ignore-end lint/safety/banDropTable
"#;

let suppressions = SuppressionsParser::parse(doc);

assert_eq!(suppressions.range_suppressions.len(), 1);
assert_eq!(
suppressions.range_suppressions[0]
.start_suppression
.rule_specifier,
RuleSpecifier::Rule(
"lint".to_string(),
"safety".to_string(),
"banDropTable".to_string()
)
);
}

#[test]
fn test_parse_mixed_prefix_suppressions() {
let doc = r#"
-- pgt-ignore-all lint

SELECT 1;
-- pgls-ignore lint/safety/banDropColumn
SELECT 2;
-- pgt-ignore typecheck
"#;

let suppressions = SuppressionsParser::parse(doc);

assert_eq!(suppressions.file_suppressions.len(), 1);
assert_eq!(suppressions.line_suppressions.len(), 2);

assert_eq!(
suppressions.file_suppressions[0].rule_specifier,
RuleSpecifier::Category("lint".to_string())
);
}
}
68 changes: 61 additions & 7 deletions crates/pgls_suppressions/src/suppression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,17 @@ pub(crate) struct Suppression {

impl Suppression {
/// Creates a suppression from a suppression comment line.
/// The line start must match `-- pgt-ignore`, otherwise, this will panic.
/// The line start must match `-- pgt-ignore` or `-- pgls-ignore`, otherwise, this will panic.
/// Leading whitespace is ignored.
pub(crate) fn from_line(line: &str, offset: &TextSize) -> Result<Self, SuppressionDiagnostic> {
let start_trimmed = line.trim_ascii_start();
let leading_whitespace_offset = line.len() - start_trimmed.len();
let trimmed = start_trimmed.trim_ascii_end();

assert!(
start_trimmed.starts_with("-- pgt-ignore"),
"Only try parsing suppressions from lines starting with `-- pgt-ignore`."
start_trimmed.starts_with("-- pgt-ignore")
|| start_trimmed.starts_with("-- pgls-ignore"),
"Only try parsing suppressions from lines starting with `-- pgt-ignore` or `-- pgls-ignore`."
);

let full_offset = *offset + TextSize::new(leading_whitespace_offset.try_into().unwrap());
Expand All @@ -141,10 +142,10 @@ impl Suppression {

let _ = parts.next();
let kind = match parts.next().unwrap() {
"pgt-ignore-all" => SuppressionKind::File,
"pgt-ignore-start" => SuppressionKind::Start,
"pgt-ignore-end" => SuppressionKind::End,
"pgt-ignore" => SuppressionKind::Line,
"pgt-ignore-all" | "pgls-ignore-all" => SuppressionKind::File,
"pgt-ignore-start" | "pgls-ignore-start" => SuppressionKind::Start,
"pgt-ignore-end" | "pgls-ignore-end" => SuppressionKind::End,
"pgt-ignore" | "pgls-ignore" => SuppressionKind::Line,
k => {
return Err(SuppressionDiagnostic {
span,
Expand Down Expand Up @@ -452,4 +453,57 @@ mod tests {
];
assert!(spec.is_disabled(&disabled5));
}

#[test]
fn test_pgls_prefix_line_suppressions() {
let line = "-- pgls-ignore lint/safety/banDropColumn: explanation";
let offset = &TextSize::new(0);
let suppression = Suppression::from_line(line, offset).unwrap();

assert_eq!(suppression.kind, SuppressionKind::Line);
assert_eq!(
suppression.rule_specifier,
RuleSpecifier::Rule(
"lint".to_string(),
"safety".to_string(),
"banDropColumn".to_string()
)
);
assert_eq!(suppression.explanation.as_deref(), Some("explanation"));
}

#[test]
fn test_pgls_prefix_file_kind() {
let line = "-- pgls-ignore-all lint/safety: explanation";
let offset = &TextSize::new(0);
let suppression = Suppression::from_line(line, offset).unwrap();

assert_eq!(suppression.kind, SuppressionKind::File);
assert_eq!(
suppression.rule_specifier,
RuleSpecifier::Group("lint".to_string(), "safety".to_string())
);
assert_eq!(suppression.explanation.as_deref(), Some("explanation"));
}

#[test]
fn test_pgls_prefix_start_and_end_kind() {
let start_line = "-- pgls-ignore-start typecheck";
let end_line = "-- pgls-ignore-end typecheck";
let offset = &TextSize::new(0);

let start_suppression = Suppression::from_line(start_line, offset).unwrap();
assert_eq!(start_suppression.kind, SuppressionKind::Start);
assert_eq!(
start_suppression.rule_specifier,
RuleSpecifier::Category("typecheck".to_string())
);

let end_suppression = Suppression::from_line(end_line, offset).unwrap();
assert_eq!(end_suppression.kind, SuppressionKind::End);
assert_eq!(
end_suppression.rule_specifier,
RuleSpecifier::Category("typecheck".to_string())
);
}
}
38 changes: 19 additions & 19 deletions docs/guides/suppressions.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ You can suppress specific diagnostics or rules in your code using suppression co
To suppress a rule, add a comment above the line causing the diagnostic with the following format:

```sql
-- pgt-ignore lint/safety/banDropTable
-- pgls-ignore lint/safety/banDropTable
drop table users;
```

Expand All @@ -20,15 +20,15 @@ Where group and specific rule are optional.
So, to suppress the `lint/safety/banDropTable` diagnostic, all of these would work:

```sql
-- pgt-ignore lint
-- pgt-ignore lint/safety
-- pgt-ignore lint/safety/banDropTable
-- pgls-ignore lint
-- pgls-ignore lint/safety
-- pgls-ignore lint/safety/banDropTable
```

You can also add an explanation to the suppression by adding a `:` and the explanation text:

```sql
-- pgt-ignore lint/safety/banDropTable: My startup never had any users.
-- pgls-ignore lint/safety/banDropTable: My startup never had any users.
drop table users;
```

Expand All @@ -41,37 +41,37 @@ create table users (
-- ...
);

-- pgt-ignore-start typecheck: The `users` table will be created with this migration.
-- pgls-ignore-start typecheck: The `users` table will be created with this migration.
alter table users drop constraint users_pkey;

alter table users add primary key (user_id);
-- pgt-ignore-end typecheck
-- pgls-ignore-end typecheck
```

Every `pgt-ignore-start` needs a `pgt-ignore-end` suppression comment, and the suppressed rules must match exactly.
Every `pgls-ignore-start` needs a `pgls-ignore-end` suppression comment, and the suppressed rules must match exactly.

This _won't_ work, because the start tag suppresses a different diagnostic:

```sql
-- pgt-ignore-start lint/safety/banDropColumn
-- pgt-ignore-end lint/safety
-- pgls-ignore-start lint/safety/banDropColumn
-- pgls-ignore-end lint/safety
```

Nesting is allowed, so this works fine:

```sql
-- pgt-ignore-start typecheck: outer
-- pgt-ignore-start lint/safety: inner
-- pgt-ignore-end lint/safety: inner
-- pgt-ignore-end typecheck: outer
-- pgls-ignore-start typecheck: outer
-- pgls-ignore-start lint/safety: inner
-- pgls-ignore-end lint/safety: inner
-- pgls-ignore-end typecheck: outer
```

### Suppressing Rules for Entire Files

Instead of repeating the same suppression on multiple lines, you can suppress for an entire file.

```sql
-- pgt-ignore-all lint/safety/banDropTable
-- pgls-ignore-all lint/safety/banDropTable

drop table tasks;
drop table projects;
Expand All @@ -83,12 +83,12 @@ drop table users;
You can suppress multiple rules by adding multiple suppression comments above a statement:

```sql
-- pgt-ignore lint/safety/banDropColumn
-- pgt-ignore typecheck
-- pgls-ignore lint/safety/banDropColumn
-- pgls-ignore typecheck
alter table tasks drop column created_at;
```

## Notes

- Trying to suppress diagnostics that have already been disabled in your [configuration file](/#configuration) will show a warning.
- Trying to suppress diagnostics that don't haven't been raised will also show a warning.
- Trying to suppress diagnostics that have already been disabled in your [configuration file](/#configuration) will show a warning.
- Trying to suppress diagnostics that don't haven't been raised will also show a warning.