Skip to content

Commit 2e1ef4c

Browse files
committed
refactor(linter): extract common logic from jsdoc/require-yields and jsdoc/require-returns (#10383)
The functions are duplicated, so we can extract the common logic.
1 parent 1bb61c6 commit 2e1ef4c

File tree

3 files changed

+40
-68
lines changed

3 files changed

+40
-68
lines changed

crates/oxc_linter/src/rules/jsdoc/require_returns.rs

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use oxc_ast::{
44
};
55
use oxc_diagnostics::OxcDiagnostic;
66
use oxc_macros::declare_oxc_lint;
7-
use oxc_semantic::{JSDoc, JSDocTag};
7+
use oxc_semantic::JSDoc;
88
use oxc_span::Span;
99
use rustc_hash::FxHashMap;
1010
use serde::Deserialize;
@@ -13,7 +13,8 @@ use crate::{
1313
context::LintContext,
1414
rule::Rule,
1515
utils::{
16-
default_true, get_function_nearest_jsdoc_node, should_ignore_as_avoid,
16+
default_true, get_function_nearest_jsdoc_node, is_duplicated_special_tag,
17+
is_missing_special_tag, should_ignore_as_avoid, should_ignore_as_custom_skip,
1718
should_ignore_as_internal, should_ignore_as_private,
1819
},
1920
};
@@ -229,47 +230,18 @@ impl Rule for RequireReturns {
229230
let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::<Vec<_>>();
230231
let resolved_returns_tag_name = settings.resolve_tag_name("returns");
231232

232-
if is_missing_returns_tag(&jsdoc_tags, resolved_returns_tag_name) {
233+
if is_missing_special_tag(&jsdoc_tags, resolved_returns_tag_name) {
233234
ctx.diagnostic(missing_returns_diagnostic(*func_span));
234235
continue;
235236
}
236237

237-
if let Some(span) = is_duplicated_returns_tag(&jsdoc_tags, resolved_returns_tag_name) {
238+
if let Some(span) = is_duplicated_special_tag(&jsdoc_tags, resolved_returns_tag_name) {
238239
ctx.diagnostic(duplicate_returns_diagnostic(span));
239240
}
240241
}
241242
}
242243
}
243244

244-
const CUSTOM_SKIP_TAG_NAMES: [&str; 6] =
245-
["abstract", "virtual", "class", "constructor", "type", "interface"];
246-
247-
fn should_ignore_as_custom_skip(jsdoc: &JSDoc) -> bool {
248-
jsdoc.tags().iter().any(|tag| CUSTOM_SKIP_TAG_NAMES.contains(&tag.kind.parsed()))
249-
}
250-
251-
fn is_missing_returns_tag(jsdoc_tags: &[&JSDocTag], resolved_returns_tag_name: &str) -> bool {
252-
jsdoc_tags.iter().all(|tag| tag.kind.parsed() != resolved_returns_tag_name)
253-
}
254-
255-
fn is_duplicated_returns_tag(
256-
jsdoc_tags: &Vec<&JSDocTag>,
257-
resolved_returns_tag_name: &str,
258-
) -> Option<Span> {
259-
let mut returns_found = false;
260-
for tag in jsdoc_tags {
261-
if tag.kind.parsed() == resolved_returns_tag_name {
262-
if returns_found {
263-
return Some(tag.kind.span);
264-
}
265-
266-
returns_found = true;
267-
}
268-
}
269-
270-
None
271-
}
272-
273245
/// - Some(true): `Promise` with value
274246
/// - Some(false): `Promise` without value
275247
/// - None: Not a `Promise` but some other expression

crates/oxc_linter/src/rules/jsdoc/require_yields.rs

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@ use oxc_diagnostics::OxcDiagnostic;
33
use oxc_macros::declare_oxc_lint;
44
use oxc_semantic::{JSDoc, JSDocTag};
55
use oxc_span::Span;
6-
use phf::phf_set;
76
use serde::Deserialize;
87

98
use crate::{
109
AstNode,
1110
context::LintContext,
1211
rule::Rule,
1312
utils::{
14-
get_function_nearest_jsdoc_node, should_ignore_as_avoid, should_ignore_as_internal,
13+
get_function_nearest_jsdoc_node, is_duplicated_special_tag, is_missing_special_tag,
14+
should_ignore_as_avoid, should_ignore_as_custom_skip, should_ignore_as_internal,
1515
should_ignore_as_private,
1616
},
1717
};
@@ -149,15 +149,15 @@ impl Rule for RequireYields {
149149
// Without this option, need to check `yield` value.
150150
// Check will be performed in `YieldExpression` branch.
151151
if config.force_require_yields
152-
&& is_missing_yields_tag(&jsdoc_tags, resolved_yields_tag_name)
152+
&& is_missing_special_tag(&jsdoc_tags, resolved_yields_tag_name)
153153
{
154154
ctx.diagnostic(missing_yields(func.span));
155155
return;
156156
}
157157

158158
// Other checks are always performed
159159

160-
if let Some(span) = is_duplicated_yields_tag(&jsdoc_tags, resolved_yields_tag_name)
160+
if let Some(span) = is_duplicated_special_tag(&jsdoc_tags, resolved_yields_tag_name)
161161
{
162162
ctx.diagnostic(duplicate_yields(span));
163163
return;
@@ -236,7 +236,7 @@ impl Rule for RequireYields {
236236
let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::<Vec<_>>();
237237
let resolved_yields_tag_name = settings.resolve_tag_name("yields");
238238

239-
if is_missing_yields_tag(&jsdoc_tags, resolved_yields_tag_name) {
239+
if is_missing_special_tag(&jsdoc_tags, resolved_yields_tag_name) {
240240
ctx.diagnostic(missing_yields(generator_func.span));
241241
}
242242
}
@@ -245,35 +245,6 @@ impl Rule for RequireYields {
245245
}
246246
}
247247

248-
const CUSTOM_SKIP_TAG_NAMES: phf::Set<&'static str> = phf_set! {
249-
"abstract", "virtual", "class", "constructor", "type", "interface"
250-
};
251-
fn should_ignore_as_custom_skip(jsdoc: &JSDoc) -> bool {
252-
jsdoc.tags().iter().any(|tag| CUSTOM_SKIP_TAG_NAMES.contains(tag.kind.parsed()))
253-
}
254-
255-
fn is_missing_yields_tag(jsdoc_tags: &[&JSDocTag], resolved_yields_tag_name: &str) -> bool {
256-
jsdoc_tags.iter().all(|tag| tag.kind.parsed() != resolved_yields_tag_name)
257-
}
258-
259-
fn is_duplicated_yields_tag(
260-
jsdoc_tags: &Vec<&JSDocTag>,
261-
resolved_yields_tag_name: &str,
262-
) -> Option<Span> {
263-
let mut yields_found = false;
264-
for tag in jsdoc_tags {
265-
if tag.kind.parsed() == resolved_yields_tag_name {
266-
if yields_found {
267-
return Some(tag.kind.span);
268-
}
269-
270-
yields_found = true;
271-
}
272-
}
273-
274-
None
275-
}
276-
277248
fn is_missing_yields_tag_with_generator_tag(
278249
jsdoc_tags: &Vec<&JSDocTag>,
279250
resolved_yields_tag_name: &str,

crates/oxc_linter/src/utils/jsdoc.rs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,41 @@ use oxc_ast::{
22
AstKind,
33
ast::{BindingPattern, BindingPatternKind, Expression, FormalParameters},
44
};
5-
use oxc_semantic::{JSDoc, Semantic};
5+
use oxc_semantic::{JSDoc, JSDocTag, Semantic};
66
use oxc_span::Span;
77
use rustc_hash::FxHashSet;
88

99
use crate::{AstNode, config::JSDocPluginSettings};
1010

11+
pub const CUSTOM_SKIP_TAG_NAMES: [&str; 6] =
12+
["abstract", "class", "constructor", "interface", "type", "virtual"];
13+
14+
pub fn should_ignore_as_custom_skip(jsdoc: &JSDoc) -> bool {
15+
jsdoc.tags().iter().any(|tag| CUSTOM_SKIP_TAG_NAMES.binary_search(&tag.kind.parsed()).is_ok())
16+
}
17+
18+
pub fn is_missing_special_tag(jsdoc_tags: &[&JSDocTag], resolved_tag_name: &str) -> bool {
19+
jsdoc_tags.iter().all(|tag| tag.kind.parsed() != resolved_tag_name)
20+
}
21+
22+
pub fn is_duplicated_special_tag(
23+
jsdoc_tags: &Vec<&JSDocTag>,
24+
resolved_returns_tag_name: &str,
25+
) -> Option<Span> {
26+
let mut returns_found = false;
27+
for tag in jsdoc_tags {
28+
if tag.kind.parsed() == resolved_returns_tag_name {
29+
if returns_found {
30+
return Some(tag.kind.span);
31+
}
32+
33+
returns_found = true;
34+
}
35+
}
36+
37+
None
38+
}
39+
1140
/// JSDoc is often attached on the parent node of a function.
1241
///
1342
/// ```js

0 commit comments

Comments
 (0)