Skip to content

Commit 77ef61a

Browse files
camchenrycamchenry
committed
fix(linter): fix diagnostic spans for oxc/no-async-await (#8721)
Fixes broken diagnostic spans for async functions that did not correctly report on the `async` keyword. I changed the diagnostic reporting to look for the `async` keyword itself within a given span which is a little slower but worth it for accuracy I think. I also updated the diagnostic to be async-specific as we don't report on await yet.
1 parent d318238 commit 77ef61a

File tree

2 files changed

+112
-38
lines changed

2 files changed

+112
-38
lines changed

‎crates/oxc_linter/src/rules/oxc/no_async_await.rs

+65-18
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
use oxc_ast::AstKind;
22
use oxc_diagnostics::OxcDiagnostic;
33
use oxc_macros::declare_oxc_lint;
4-
use oxc_semantic::NodeId;
5-
use oxc_span::Span;
4+
use oxc_span::{GetSpan, Span};
65

76
use crate::{context::LintContext, rule::Rule, AstNode};
87

9-
fn no_async_await_diagnostic(span: Span) -> OxcDiagnostic {
10-
OxcDiagnostic::warn("Unexpected async/await")
11-
.with_help("Async/await is not allowed")
8+
fn no_async_diagnostic(span: Span) -> OxcDiagnostic {
9+
OxcDiagnostic::warn("async is not allowed")
10+
.with_help("Remove the `async` keyword")
1211
.with_label(span)
1312
}
1413

@@ -37,29 +36,58 @@ impl Rule for NoAsyncAwait {
3736
match node.kind() {
3837
AstKind::Function(func_decl) => {
3938
if func_decl.r#async {
40-
report(node.id(), func_decl.span, ctx);
39+
let parent_kind = ctx.nodes().parent_kind(node.id());
40+
let async_span = match &func_decl.id {
41+
// named function like `async function run() {}`
42+
Some(id) => Span::new(func_decl.span.start, id.span.end),
43+
// anonymous function like `async function() {}`
44+
None => match parent_kind {
45+
// Actually part of a method definition like:
46+
// ```
47+
// class Foo {
48+
// async bar() {}
49+
// }
50+
// ```
51+
Some(AstKind::MethodDefinition(method_def)) => {
52+
Span::new(method_def.span.start, method_def.key.span().start)
53+
}
54+
// The function is part of an object property like:
55+
// ```
56+
// const obj = {
57+
// async foo() {}
58+
// };
59+
// ```
60+
Some(AstKind::ObjectProperty(obj_prop)) => {
61+
Span::new(obj_prop.span.start, obj_prop.key.span().start)
62+
}
63+
_ => func_decl.span,
64+
},
65+
};
66+
report_on_async_span(async_span, ctx);
4167
}
4268
}
4369
AstKind::ArrowFunctionExpression(arrow_expr) => {
4470
if arrow_expr.r#async {
45-
report(node.id(), arrow_expr.span, ctx);
71+
let async_span = Span::new(arrow_expr.span.start, arrow_expr.params.span.start);
72+
report_on_async_span(async_span, ctx);
4673
}
4774
}
4875
_ => {}
4976
}
5077
}
5178
}
5279

53-
fn report(node_id: NodeId, func_span: Span, ctx: &LintContext<'_>) {
54-
/// "async".len()
55-
const ASYNC_LEN: u32 = 5;
80+
/// "async".len()
81+
const ASYNC_LEN: u32 = 5;
5682

57-
let parent = ctx.nodes().parent_kind(node_id);
58-
if let Some(AstKind::ObjectProperty(obj_prop)) = parent {
59-
ctx.diagnostic(no_async_await_diagnostic(Span::sized(obj_prop.span.start, ASYNC_LEN)));
60-
} else {
61-
ctx.diagnostic(no_async_await_diagnostic(Span::sized(func_span.start, ASYNC_LEN)));
62-
}
83+
#[allow(clippy::cast_possible_truncation)]
84+
fn report_on_async_span(async_span: Span, ctx: &LintContext<'_>) {
85+
// find the `async` keyword within the span and report on it
86+
let Some(async_keyword_offset) = ctx.source_range(async_span).find("async") else {
87+
return;
88+
};
89+
let async_keyword_span = Span::sized(async_span.start + async_keyword_offset as u32, ASYNC_LEN);
90+
ctx.diagnostic(no_async_diagnostic(async_keyword_span));
6391
}
6492

6593
#[test]
@@ -71,6 +99,9 @@ fn test() {
7199
"const foo = () => {}",
72100
"function foo () { return bar(); }",
73101
"class Foo { foo() {} }",
102+
"class async { }",
103+
"const async = {};",
104+
"class async { async() { async(); } }",
74105
];
75106

76107
let fail = vec![
@@ -83,7 +114,6 @@ fn test() {
83114
async test() {}
84115
};
85116
",
86-
// FIXME: diagnostics on method `foo` have incorrect spans
87117
"
88118
class Foo {
89119
async foo() {}
@@ -94,12 +124,29 @@ fn test() {
94124
public async foo() {}
95125
}
96126
",
97-
// this one is fine
98127
"
99128
const obj = {
100129
async foo() {}
101130
}
102131
",
132+
"
133+
class async {
134+
async async() {
135+
async();
136+
}
137+
}
138+
",
139+
"
140+
class async {
141+
async async() {
142+
function async() {
143+
const async = {
144+
async: async () => {},
145+
}
146+
}
147+
}
148+
}
149+
",
103150
];
104151

105152
Tester::new(NoAsyncAwait::NAME, NoAsyncAwait::PLUGIN, pass, fail).test_and_snapshot();

‎crates/oxc_linter/src/snapshots/oxc_no_async_await.snap

+47-20
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,93 @@
11
---
22
source: crates/oxc_linter/src/tester.rs
33
---
4-
⚠ oxc(no-async-await): Unexpected async/await
4+
⚠ oxc(no-async-await): async is not allowed
55
╭─[no_async_await.tsx:1:1]
66
1 │ async function foo() {}
77
· ─────
88
╰────
9-
help: Async/await is not allowed
9+
help: Remove the `async` keyword
1010

11-
⚠ oxc(no-async-await): Unexpected async/await
11+
⚠ oxc(no-async-await): async is not allowed
1212
╭─[no_async_await.tsx:1:13]
1313
1 │ const foo = async () => {}
1414
· ─────
1515
╰────
16-
help: Async/await is not allowed
16+
help: Remove the `async` keyword
1717

18-
⚠ oxc(no-async-await): Unexpected async/await
18+
⚠ oxc(no-async-await): async is not allowed
1919
╭─[no_async_await.tsx:1:1]
2020
1 │ async () => {}
2121
· ─────
2222
╰────
23-
help: Async/await is not allowed
23+
help: Remove the `async` keyword
2424

25-
⚠ oxc(no-async-await): Unexpected async/await
25+
⚠ oxc(no-async-await): async is not allowed
2626
╭─[no_async_await.tsx:1:14]
2727
1 │ const test = async () => {};
2828
· ─────
2929
╰────
30-
help: Async/await is not allowed
30+
help: Remove the `async` keyword
3131

32-
⚠ oxc(no-async-await): Unexpected async/await
32+
⚠ oxc(no-async-await): async is not allowed
3333
╭─[no_async_await.tsx:3:17]
3434
2 │ const test = {
3535
3 │ async test() {}
3636
· ─────
3737
4 │ };
3838
╰────
39-
help: Async/await is not allowed
39+
help: Remove the `async` keyword
4040

41-
⚠ oxc(no-async-await): Unexpected async/await
42-
╭─[no_async_await.tsx:3:22]
41+
⚠ oxc(no-async-await): async is not allowed
42+
╭─[no_async_await.tsx:3:13]
4343
2 │ class Foo {
4444
3 │ async foo() {}
45-
· ─────
45+
· ─────
4646
4 │ }
4747
╰────
48-
help: Async/await is not allowed
48+
help: Remove the `async` keyword
4949

50-
⚠ oxc(no-async-await): Unexpected async/await
51-
╭─[no_async_await.tsx:3:29]
50+
⚠ oxc(no-async-await): async is not allowed
51+
╭─[no_async_await.tsx:3:20]
5252
2 │ class Foo {
5353
3 │ public async foo() {}
54-
· ─────
54+
· ─────
5555
4 │ }
5656
╰────
57-
help: Async/await is not allowed
57+
help: Remove the `async` keyword
5858

59-
⚠ oxc(no-async-await): Unexpected async/await
59+
⚠ oxc(no-async-await): async is not allowed
6060
╭─[no_async_await.tsx:3:13]
6161
2 │ const obj = {
6262
3 │ async foo() {}
6363
· ─────
6464
4 │ }
6565
╰────
66-
help: Async/await is not allowed
66+
help: Remove the `async` keyword
67+
68+
⚠ oxc(no-async-await): async is not allowed
69+
╭─[no_async_await.tsx:3:13]
70+
2 │ class async {
71+
3 │ async async() {
72+
· ─────
73+
4 │ async();
74+
╰────
75+
help: Remove the `async` keyword
76+
77+
⚠ oxc(no-async-await): async is not allowed
78+
╭─[no_async_await.tsx:3:13]
79+
2 │ class async {
80+
3 │ async async() {
81+
· ─────
82+
4 │ function async() {
83+
╰────
84+
help: Remove the `async` keyword
85+
86+
⚠ oxc(no-async-await): async is not allowed
87+
╭─[no_async_await.tsx:6:32]
88+
5 │ const async = {
89+
6 │ async: async () => {},
90+
· ─────
91+
7 │ }
92+
╰────
93+
help: Remove the `async` keyword

0 commit comments

Comments
 (0)