Skip to content

Commit

Permalink
feat(graphql_analyze): add suppression action for the GraphQL linter (#…
Browse files Browse the repository at this point in the history
…4312)

Co-authored-by: Emanuele Stoppa <[email protected]>
  • Loading branch information
vohoanglong0107 and ematipico authored Oct 18, 2024
1 parent bc206bb commit 1c60340
Show file tree
Hide file tree
Showing 7 changed files with 335 additions and 10 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,22 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
#### New features

- Add [noUselessUndefined](https://biomejs.dev/linter/rules/no-useless-undefined/). Contributed by @unvalley
- Add support for parsing the defer attribute in import statements ([#4215](https://github.com/biomejs/biome/issues/4215)).

```js
import defer * as myModule from "my-module";
```

Contributed by @fireairforce

## v1.9.4 (2024-10-17)

### Analyzer

#### Bug fixes

- Implement [GraphQL suppression action](https://github.com/biomejs/biome/pull/4312). Contributed by @vohoanglong0107

- Improved the message for unused suppression comments. Contributed by @dyc3

- Fix [#4228](https://github.com/biomejs/biome/issues/4228), where the rule `a11y/noInteractiveElementToNoninteractiveRole` incorrectly reports a `role` for non-interactive elements. Contributed by @eryue0220
Expand Down
67 changes: 58 additions & 9 deletions crates/biome_graphql_analyze/src/suppression_action.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use biome_analyze::{ApplySuppression, SuppressionAction};
use biome_graphql_syntax::GraphqlLanguage;
use biome_rowan::{BatchMutation, SyntaxToken};
use biome_graphql_syntax::{GraphqlLanguage, GraphqlSyntaxToken};
use biome_rowan::{BatchMutation, TriviaPieceKind};

pub(crate) struct GraphqlSuppressionAction;

Expand All @@ -9,18 +9,67 @@ impl SuppressionAction for GraphqlSuppressionAction {

fn find_token_to_apply_suppression(
&self,
_original_token: SyntaxToken<Self::Language>,
token: GraphqlSyntaxToken,
) -> Option<ApplySuppression<Self::Language>> {
// TODO: property implement. Look for the JsSuppressionAction for an example
None
let mut apply_suppression = ApplySuppression {
token_has_trailing_comments: false,
token_to_apply_suppression: token.clone(),
should_insert_leading_newline: false,
};

// Find the token at the start of suppressed token's line
let mut current_token = token;
loop {
let trivia = current_token.leading_trivia();
if trivia.pieces().any(|trivia| trivia.kind().is_newline()) {
break;
} else if let Some(prev_token) = current_token.prev_token() {
current_token = prev_token
} else {
break;
}
}

apply_suppression.token_to_apply_suppression = current_token;
Some(apply_suppression)
}

fn apply_suppression(
&self,
_mutation: &mut BatchMutation<Self::Language>,
_apply_suppression: ApplySuppression<Self::Language>,
_suppression_text: &str,
mutation: &mut BatchMutation<Self::Language>,
apply_suppression: ApplySuppression<Self::Language>,
suppression_text: &str,
) {
unreachable!("find_token_to_apply_suppression return None")
let ApplySuppression {
token_to_apply_suppression,
..
} = apply_suppression;

let mut new_token = token_to_apply_suppression.clone();
let leading_whitespaces: Vec<_> = new_token
.leading_trivia()
.pieces()
.filter(|trivia| trivia.is_whitespace())
.collect();

let suppression_comment = format!("# {}: <explanation>", suppression_text);
let suppression_comment = suppression_comment.as_str();
let trivia = [
(TriviaPieceKind::SingleLineComment, suppression_comment),
(TriviaPieceKind::Newline, "\n"),
];
if leading_whitespaces.is_empty() {
new_token = new_token.with_leading_trivia(trivia);
}
// Token is indented
else {
let mut trivia = trivia.to_vec();

for w in leading_whitespaces.iter() {
trivia.push((TriviaPieceKind::Whitespace, w.text()));
}
new_token = new_token.with_leading_trivia(trivia);
}
mutation.replace_token_transfer_trivia(token_to_apply_suppression, new_token);
}
}
2 changes: 1 addition & 1 deletion crates/biome_graphql_analyze/tests/spec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ fn check_code_action(
assert_errors_are_absent(re_parse.tree().syntax(), re_parse.diagnostics(), path);
}

pub(crate) fn _run_suppression_test(input: &'static str, _: &str, _: &str, _: &str) {
pub(crate) fn run_suppression_test(input: &'static str, _: &str, _: &str, _: &str) {
register_leak_checker();

let input_file = Path::new(input);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
query test($v: String, $t: String, $v: String) {
id
}

query test {
users(first: 100, after: 10, filter: "test", first: 50) {
id
}
}

query test {
users {
id
name
email
name
}
}

query test {
users {
id
name
email
# biome-ignore lint/nursery/noDuplicatedFields: testing
email: somethingElse
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
---
source: crates/biome_graphql_analyze/tests/spec_tests.rs
expression: noDuplicatedFields.graphql
---
# Input
```graphql
query test($v: String, $t: String, $v: String) {
id
}
query test {
users(first: 100, after: 10, filter: "test", first: 50) {
id
}
}
query test {
users {
id
name
email
name
}
}
query test {
users {
id
name
email
# biome-ignore lint/nursery/noDuplicatedFields: testing
email: somethingElse
}
}
```

# Diagnostics
```
noDuplicatedFields.graphql:1:36 lint/nursery/noDuplicatedFields FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━
! Variable `v` defined multiple times.
> 1 │ query test($v: String, $t: String, $v: String) {
^^^^^^^^^^
2id
3}
i Remove the duplicated variable.
i Safe fix: Suppress rule lint/nursery/noDuplicatedFields
1 │ - query·test($v:·String,·$t:·String,·$v:·String)·{
1+biome-ignore·lint/nursery/noDuplicatedFields:·<explanation>
2 │ + query··test($v:·String,·$t:·String,·$v:·String)·{
2 3id
3 4}
```
```
noDuplicatedFields.graphql:6:48 lint/nursery/noDuplicatedFields FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━
! Argument `first` defined multiple times.
5 │ query test {
> 6users(first: 100, after: 10, filter: "test", first: 50) {
│ ^^^^^^^^^
7 │ id
8 │ }
i Remove the duplicated argument.
i Safe fix: Suppress rule lint/nursery/noDuplicatedFields
4 4
5 5query test {
6 │ - ··users(first:·100after:·10filter:·"test"first:·50)·{
6+ ··#·biome-ignore·lint/nursery/noDuplicatedFields:·<explanation>
7 │ + ··users(first:·100,·after:·10,·filter:·"test",·first:·50)·{
7 8id
8 9}
```
```
noDuplicatedFields.graphql:16:5 lint/nursery/noDuplicatedFields FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━
! Field `name` defined multiple times.
14 │ name
15 │ email
> 16 │ name
│ ^^^^
17 │ }
18 │ }
i Remove the duplicated field.
i Safe fix: Suppress rule lint/nursery/noDuplicatedFields
14 14 │ name
15 15 │ email
16 │ - ····name
16 │ + ····#·biome-ignore·lint/nursery/noDuplicatedFields:·<explanation>
17 │ + ····name
17 18 │ }
18 19 │ }
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
query {
member @deprecated {
id
}
}

query {
member @deprecated()
}

query {
member
@deprecated(abc: 123)
}

query {
# biome-ignore lint/nursery/useDeprecatedReason: testing
member @deprecated()
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
---
source: crates/biome_graphql_analyze/tests/spec_tests.rs
expression: useDeprecatedReason.graphql
---
# Input
```graphql
query {
member @deprecated {
id
}
}
query {
member @deprecated()
}
query {
member
@deprecated(abc: 123)
}
query {
# biome-ignore lint/nursery/useDeprecatedReason: testing
member @deprecated()
}
```

# Diagnostics
```
useDeprecatedReason.graphql:2:10 lint/nursery/useDeprecatedReason FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━
! The directive `@deprecated` should have a `reason` argument.
1 │ query {
> 2member @deprecated {
│ ^^^^^^^^^^^
3 │ id
4 │ }
i Add a `reason` argument to the directive.
i Safe fix: Suppress rule lint/nursery/useDeprecatedReason
1 1query {
2 │ - ··member·@deprecated·{
2 │ + ··#·biome-ignore·lint/nursery/useDeprecatedReason:·<explanation>
3 │ + ··member··@deprecated·{
3 4id
4 5}
```
```
useDeprecatedReason.graphql:8:10 lint/nursery/useDeprecatedReason FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━
! The directive `@deprecated` should have a `reason` argument.
7 │ query {
> 8member @deprecated()
^^^^^^^^^^^^^
9}
10 │
i Add a `reason` argument to the directive.
i Safe fix: Suppress rule lint/nursery/useDeprecatedReason
6 6 │
7 7 │ query {
8- ··member·@deprecated()
8+ ··#·biome-ignore·lint/nursery/useDeprecatedReason:·<explanation>
9 │ + ··member··@deprecated()
9 10 │ }
10 11 │
```
```
useDeprecatedReason.graphql:13:3 lint/nursery/useDeprecatedReason FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━
! The directive `@deprecated` should have a `reason` argument.
11 │ query {
12member
> 13 │ @deprecated(abc: 123)
^^^^^^^^^^^^^^^^^^^^^
14}
15 │
i Add a `reason` argument to the directive.
i Safe fix: Suppress rule lint/nursery/useDeprecatedReason
11 11 │ query {
12 12member
13- ··@deprecated(abc123)
13+ ··#·biome-ignore·lint/nursery/useDeprecatedReason:·<explanation>
14 │ + ··@deprecated(abc:·123)
14 15 │ }
15 16 │
```

0 comments on commit 1c60340

Please sign in to comment.