Skip to content

refactor(linter): extract common logic from jsdoc/require-yields and jsdoc/require-returns #10383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graphite-app merged 1 commit into from
Apr 13, 2025
Merged

refactor(linter): extract common logic from jsdoc/require-yields and jsdoc/require-returns #10383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 5 additions & 33 deletions crates/oxc_linter/src/rules/jsdoc/require_returns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use oxc_ast::{
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_semantic::{JSDoc, JSDocTag};
use oxc_semantic::JSDoc;
use oxc_span::Span;
use rustc_hash::FxHashMap;
use serde::Deserialize;
Expand All @@ -13,7 +13,8 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
default_true, get_function_nearest_jsdoc_node, should_ignore_as_avoid,
default_true, get_function_nearest_jsdoc_node, is_duplicated_special_tag,
is_missing_special_tag, should_ignore_as_avoid, should_ignore_as_custom_skip,
should_ignore_as_internal, should_ignore_as_private,
},
};
Expand Down Expand Up @@ -229,47 +230,18 @@ impl Rule for RequireReturns {
let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::<Vec<_>>();
let resolved_returns_tag_name = settings.resolve_tag_name("returns");

if is_missing_returns_tag(&jsdoc_tags, resolved_returns_tag_name) {
if is_missing_special_tag(&jsdoc_tags, resolved_returns_tag_name) {
ctx.diagnostic(missing_returns_diagnostic(*func_span));
continue;
}

if let Some(span) = is_duplicated_returns_tag(&jsdoc_tags, resolved_returns_tag_name) {
if let Some(span) = is_duplicated_special_tag(&jsdoc_tags, resolved_returns_tag_name) {
ctx.diagnostic(duplicate_returns_diagnostic(span));
}
}
}
}

const CUSTOM_SKIP_TAG_NAMES: [&str; 6] =
["abstract", "virtual", "class", "constructor", "type", "interface"];

fn should_ignore_as_custom_skip(jsdoc: &JSDoc) -> bool {
jsdoc.tags().iter().any(|tag| CUSTOM_SKIP_TAG_NAMES.contains(&tag.kind.parsed()))
}

fn is_missing_returns_tag(jsdoc_tags: &[&JSDocTag], resolved_returns_tag_name: &str) -> bool {
jsdoc_tags.iter().all(|tag| tag.kind.parsed() != resolved_returns_tag_name)
}

fn is_duplicated_returns_tag(
jsdoc_tags: &Vec<&JSDocTag>,
resolved_returns_tag_name: &str,
) -> Option<Span> {
let mut returns_found = false;
for tag in jsdoc_tags {
if tag.kind.parsed() == resolved_returns_tag_name {
if returns_found {
return Some(tag.kind.span);
}

returns_found = true;
}
}

None
}

/// - Some(true): `Promise` with value
/// - Some(false): `Promise` without value
/// - None: Not a `Promise` but some other expression
Expand Down
39 changes: 5 additions & 34 deletions crates/oxc_linter/src/rules/jsdoc/require_yields.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_semantic::{JSDoc, JSDocTag};
use oxc_span::Span;
use phf::phf_set;
use serde::Deserialize;

use crate::{
AstNode,
context::LintContext,
rule::Rule,
utils::{
get_function_nearest_jsdoc_node, should_ignore_as_avoid, should_ignore_as_internal,
get_function_nearest_jsdoc_node, is_duplicated_special_tag, is_missing_special_tag,
should_ignore_as_avoid, should_ignore_as_custom_skip, should_ignore_as_internal,
should_ignore_as_private,
},
};
Expand Down Expand Up @@ -149,15 +149,15 @@ impl Rule for RequireYields {
// Without this option, need to check `yield` value.
// Check will be performed in `YieldExpression` branch.
if config.force_require_yields
&& is_missing_yields_tag(&jsdoc_tags, resolved_yields_tag_name)
&& is_missing_special_tag(&jsdoc_tags, resolved_yields_tag_name)
{
ctx.diagnostic(missing_yields(func.span));
return;
}

// Other checks are always performed

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

if is_missing_yields_tag(&jsdoc_tags, resolved_yields_tag_name) {
if is_missing_special_tag(&jsdoc_tags, resolved_yields_tag_name) {
ctx.diagnostic(missing_yields(generator_func.span));
}
}
Expand All @@ -245,35 +245,6 @@ impl Rule for RequireYields {
}
}

const CUSTOM_SKIP_TAG_NAMES: phf::Set<&'static str> = phf_set! {
"abstract", "virtual", "class", "constructor", "type", "interface"
};
fn should_ignore_as_custom_skip(jsdoc: &JSDoc) -> bool {
jsdoc.tags().iter().any(|tag| CUSTOM_SKIP_TAG_NAMES.contains(tag.kind.parsed()))
}

fn is_missing_yields_tag(jsdoc_tags: &[&JSDocTag], resolved_yields_tag_name: &str) -> bool {
jsdoc_tags.iter().all(|tag| tag.kind.parsed() != resolved_yields_tag_name)
}

fn is_duplicated_yields_tag(
jsdoc_tags: &Vec<&JSDocTag>,
resolved_yields_tag_name: &str,
) -> Option<Span> {
let mut yields_found = false;
for tag in jsdoc_tags {
if tag.kind.parsed() == resolved_yields_tag_name {
if yields_found {
return Some(tag.kind.span);
}

yields_found = true;
}
}

None
}

fn is_missing_yields_tag_with_generator_tag(
jsdoc_tags: &Vec<&JSDocTag>,
resolved_yields_tag_name: &str,
Expand Down
31 changes: 30 additions & 1 deletion crates/oxc_linter/src/utils/jsdoc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,41 @@ use oxc_ast::{
AstKind,
ast::{BindingPattern, BindingPatternKind, Expression, FormalParameters},
};
use oxc_semantic::{JSDoc, Semantic};
use oxc_semantic::{JSDoc, JSDocTag, Semantic};
use oxc_span::Span;
use rustc_hash::FxHashSet;

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

pub const CUSTOM_SKIP_TAG_NAMES: [&str; 6] =
["abstract", "class", "constructor", "interface", "type", "virtual"];

pub fn should_ignore_as_custom_skip(jsdoc: &JSDoc) -> bool {
jsdoc.tags().iter().any(|tag| CUSTOM_SKIP_TAG_NAMES.binary_search(&tag.kind.parsed()).is_ok())
}

pub fn is_missing_special_tag(jsdoc_tags: &[&JSDocTag], resolved_tag_name: &str) -> bool {
jsdoc_tags.iter().all(|tag| tag.kind.parsed() != resolved_tag_name)
}

pub fn is_duplicated_special_tag(
jsdoc_tags: &Vec<&JSDocTag>,
resolved_returns_tag_name: &str,
) -> Option<Span> {
let mut returns_found = false;
for tag in jsdoc_tags {
if tag.kind.parsed() == resolved_returns_tag_name {
if returns_found {
return Some(tag.kind.span);
}

returns_found = true;
}
}

None
}

/// JSDoc is often attached on the parent node of a function.
///
/// ```js
Expand Down
Loading