From f10336927e5be11cb68c4e8f082074622d918092 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Wed, 1 May 2024 16:50:55 +0900 Subject: [PATCH 01/11] feat(linter/jsdoc): Implement require-yields rule --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/jsdoc/require_yields.rs | 1250 +++++++++++++++++ 2 files changed, 1252 insertions(+) create mode 100644 crates/oxc_linter/src/rules/jsdoc/require_yields.rs diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 0ba5dae87cd21..429cee63160f6 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -372,6 +372,7 @@ mod jsdoc { pub mod require_property_description; pub mod require_property_name; pub mod require_property_type; + pub mod require_yields; } mod tree_shaking { @@ -710,5 +711,6 @@ oxc_macros::declare_all_lint_rules! { jsdoc::require_property_type, jsdoc::require_property_name, jsdoc::require_property_description, + jsdoc::require_yields, tree_shaking::no_side_effects_in_initialization, } diff --git a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs new file mode 100644 index 0000000000000..20dc1c886b895 --- /dev/null +++ b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs @@ -0,0 +1,1250 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use serde::Deserialize; +use serde_derive_default; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-jsdoc(require-yields): Missing `@yields` declaration.")] +#[diagnostic(severity(warning), help("Add `@yields` tag to the JSDoc comment."))] +struct RequireYieldsDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct RequireYields(Box); + +declare_oxc_lint!( + /// ### What it does + /// Requires that yields are documented. + /// Will also report if multiple `@yields` tags are present. + /// + /// ### Why is this bad? + /// The rule is intended to prevent the omission of `@yields` tags when they are necessary. + /// + /// ### Example + /// ```javascript + /// // Passing + /// /** * @yields Foo */ + /// function * quux (foo) { yield foo; } + /// + /// // Failing + /// function * quux (foo) { yield foo; } + /// /** + /// * @yields {undefined} + /// * @yields {void} + /// */ + /// function * quux (foo) {} + /// ``` + RequireYields, + correctness +); + +#[derive(Debug, serde_derive_default::Default, Clone, Deserialize)] +struct RequireYieldsConfig { + #[serde(default, rename = "withGeneratorTag")] + with_generator_tag: bool, + #[serde(default = "default_true", rename = "forceRequireYields")] + force_require_yields: bool, + #[serde(default = "default_exempted_by", rename = "exemptedBy")] + exempted_by: Vec, +} + +fn default_exempted_by() -> Vec { + vec!["inheritdoc".to_string()] +} +fn default_true() -> bool { + true +} + +impl Rule for RequireYields { + fn from_configuration(value: serde_json::Value) -> Self { + value + .as_array() + .and_then(|arr| arr.first()) + .and_then(|value| serde_json::from_value(value.clone()).ok()) + .map_or_else(Self::default, |value| Self(Box::new(value))) + } + + fn run_once<'a>(&self, ctx: &LintContext<'a>) { + // fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + // let AstKind::YieldExpression(yield_expression) = node.kind() else { + // return; + // }; + + let config = &self.0; + println!("๐Ÿ‘ป {config:#?}"); + let settings = &ctx.settings().jsdoc; + println!("๐Ÿ‘ป {settings:#?}"); + + ctx.diagnostic(RequireYieldsDiagnostic(Span::default())); + + // if config.forceRequireYields { + // + // } else { + // if node != YieldExpression { return } + // if !yieldValue { return } + // const functionNode = parent(node); + // } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ( + " + /** + * @yields Foo. + */ + function * quux () { + + yield foo; + } + ", + None, + None, + ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function quux (bar) { + // bar.doSomething(function * (baz) { + // yield baz.corge(); + // }) + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @yields {Array} + // */ + // function * quux (bar) { + // yield bar.doSomething(function * (baz) { + // yield baz.corge(); + // }) + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @inheritdoc + // */ + // function * quux (foo) { + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @override + // */ + // function * quux (foo) { + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @constructor + // */ + // function * quux (foo) { + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @implements + // */ + // function * quux (foo) { + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @override + // */ + // function * quux (foo) { + + // yield foo; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @class + // */ + // function * quux (foo) { + // yield foo; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @yields {object} + // */ + // function * quux () { + + // yield {a: foo}; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @yields {void} + // */ + // function * quux () { + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @yields {undefined} + // */ + // function * quux () { + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @yields {void} + // */ + // function quux () { + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @yields {void} + // */ + // function * quux () { + // yield undefined; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @yields {void} + // */ + // function * quux () { + // yield undefined; + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @yields {void} + // */ + // function * quux () { + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @yields {void} + // */ + // function * quux () { + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @yields {void} + // */ + // function * quux () { + // yield; + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** @type {SpecialIterator} */ + // function * quux () { + // yield 5; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @yields {Something} + // */ + // async function * quux () { + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * + // */ + // async function * quux () {} + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // const quux = async function * () {} + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @type {MyCallback} + // */ + // function * quux () { + // yield; + // } + // ", + // Some(serde_json::json!([ + // { + // "exemptedBy": [ + // "type", + // ], + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @param {array} a + // */ + // async function * foo (a) { + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @function + // */ + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @callback + // */ + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @generator + // */ + // ", + // Some(serde_json::json!([ + // { + // "withGeneratorTag": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @generator + // * @yields + // */ + // *function() {} + // ", + // Some(serde_json::json!([ + // { + // "withGeneratorTag": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @yields + // */ + // function * quux (foo) { + + // const a = yield foo; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux (foo) { + // const a = function * bar () { + // yield foo; + // } + // } + // ", + // None, + // None, + // ), + ]; + + let fail = vec![ + // ( + // " + // /** + // * + // */ + // function * quux (foo) { + + // yield foo; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux (foo) { + // someLabel: { + // yield foo; + // } + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux (foo) { + + // const a = yield foo; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux (foo) { + // yield foo; + // } + // ", + // None, + // Some(serde_json::json!({ "settings": { + // "jsdoc": { + // "tagNamePreference": { + // "yields": "yield", + // }, + // }, + // } })), + // ), + // ( + // " + // /** + // * + // */ + // function * quux() { + // yield 5; + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux() { + // yield; + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * + // */ + // const quux = async function * () { + // yield; + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * + // */ + // async function * quux () { + // yield; + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // yield; + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @function + // * @generator + // */ + // *function() {} + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @yields {undefined} + // * @yields {void} + // */ + // function * quux (foo) { + + // return foo; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @param foo + // */ + // function * quux (foo) { + // yield 'bar'; + // } + // ", + // Some(serde_json::json!([ + // { + // "exemptedBy": [ + // "notPresent", + // ], + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @param {array} a + // */ + // async function * foo(a) { + // return; + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @param {array} a + // */ + // async function * foo(a) { + // yield Promise.all(a); + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // class quux { + // /** + // * + // */ + // * quux () { + // yield; + // } + // } + // ", + // Some(serde_json::json!([ + // { + // "forceRequireYields": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @param {array} a + // */ + // async function * foo(a) { + // yield Promise.all(a); + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // if (true) { + // yield; + // } + // yield true; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // if (yield false) { + + // } + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // b ? yield false : true + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // try { + // yield true; + // } catch (err) { + // } + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // try { + // } finally { + // yield true; + // } + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // try { + // yield; + // } catch (err) { + // } + // yield true; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // try { + // something(); + // } catch (err) { + // yield true; + // } + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // switch (true) { + // case 'abc': + // yield true; + // } + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // switch (true) { + // case 'abc': + // yield; + // } + // yield true; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // for (const i of abc) { + // yield true; + // } + // yield; + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // for (const a in b) { + // yield true; + // } + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * + // */ + // function * quux () { + // for (let i=0; i Date: Wed, 1 May 2024 17:27:59 +0900 Subject: [PATCH 02/11] Fix default --- .../src/rules/jsdoc/require_yields.rs | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs index 20dc1c886b895..ff040755784fc 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs @@ -6,7 +6,6 @@ use oxc_diagnostics::{ use oxc_macros::declare_oxc_lint; use oxc_span::Span; use serde::Deserialize; -use serde_derive_default; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -44,7 +43,7 @@ declare_oxc_lint!( correctness ); -#[derive(Debug, serde_derive_default::Default, Clone, Deserialize)] +#[derive(Debug, Clone, Deserialize)] struct RequireYieldsConfig { #[serde(default, rename = "withGeneratorTag")] with_generator_tag: bool, @@ -53,6 +52,15 @@ struct RequireYieldsConfig { #[serde(default = "default_exempted_by", rename = "exemptedBy")] exempted_by: Vec, } +impl Default for RequireYieldsConfig { + fn default() -> Self { + Self { + with_generator_tag: false, + force_require_yields: true, + exempted_by: default_exempted_by(), + } + } +} fn default_exempted_by() -> Vec { vec!["inheritdoc".to_string()] @@ -70,11 +78,10 @@ impl Rule for RequireYields { .map_or_else(Self::default, |value| Self(Box::new(value))) } - fn run_once<'a>(&self, ctx: &LintContext<'a>) { - // fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - // let AstKind::YieldExpression(yield_expression) = node.kind() else { - // return; - // }; + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::YieldExpression(yield_expression) = node.kind() else { + return; + }; let config = &self.0; println!("๐Ÿ‘ป {config:#?}"); @@ -213,7 +220,7 @@ fn test() { // * @override // */ // function * quux (foo) { - + // yield foo; // } // ", @@ -238,7 +245,7 @@ fn test() { // * @yields {object} // */ // function * quux () { - + // yield {a: foo}; // } // ", @@ -488,7 +495,7 @@ fn test() { // * @yields // */ // function * quux (foo) { - + // const a = yield foo; // } // ", @@ -518,7 +525,7 @@ fn test() { // * // */ // function * quux (foo) { - + // yield foo; // } // ", @@ -545,7 +552,7 @@ fn test() { // * // */ // function * quux (foo) { - + // const a = yield foo; // } // ", @@ -672,7 +679,7 @@ fn test() { // * @yields {void} // */ // function * quux (foo) { - + // return foo; // } // ", @@ -781,7 +788,7 @@ fn test() { // */ // function * quux () { // if (yield false) { - + // } // } // ", From a7a94e0d2e9f6bdd1061dabbfbeb3b8266a51890 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 2 May 2024 14:48:54 +0900 Subject: [PATCH 03/11] Fix typo --- crates/oxc_linter/src/config/settings/jsdoc.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/config/settings/jsdoc.rs b/crates/oxc_linter/src/config/settings/jsdoc.rs index 1a358484c2717..c8c59dd78e32c 100644 --- a/crates/oxc_linter/src/config/settings/jsdoc.rs +++ b/crates/oxc_linter/src/config/settings/jsdoc.rs @@ -19,7 +19,7 @@ pub struct JSDocPluginSettings { pub override_replaces_docs: bool, /// Only for `require-(yields|returns|description|example|param|throws)` rule #[serde(default, rename = "augmentsExtendsReplacesDocs")] - pub arguments_extends_replaces_docs: bool, + pub augments_extends_replaces_docs: bool, /// Only for `require-(yields|returns|description|example|param|throws)` rule #[serde(default, rename = "implementsReplacesDocs")] pub implements_replaces_docs: bool, @@ -79,7 +79,7 @@ impl Default for JSDocPluginSettings { // Exists only for these defaults ignore_replaces_docs: true, override_replaces_docs: true, - arguments_extends_replaces_docs: false, + augments_extends_replaces_docs: false, implements_replaces_docs: false, exempt_destructured_roots_from_checks: false, tag_name_preference: FxHashMap::default(), @@ -202,7 +202,7 @@ mod test { assert_eq!(settings.tag_name_preference.len(), 0); assert!(settings.ignore_replaces_docs); assert!(settings.override_replaces_docs); - assert!(!settings.arguments_extends_replaces_docs); + assert!(!settings.augments_extends_replaces_docs); assert!(!settings.implements_replaces_docs); let settings = JSDocPluginSettings::default(); @@ -212,7 +212,7 @@ mod test { assert_eq!(settings.tag_name_preference.len(), 0); assert!(settings.ignore_replaces_docs); assert!(settings.override_replaces_docs); - assert!(!settings.arguments_extends_replaces_docs); + assert!(!settings.augments_extends_replaces_docs); assert!(!settings.implements_replaces_docs); } From a8bff39da3823c2cc91213c28c897c1175c20a73 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 2 May 2024 14:49:04 +0900 Subject: [PATCH 04/11] Add more boundary --- crates/oxc_linter/src/utils/jsdoc.rs | 36 +++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/utils/jsdoc.rs b/crates/oxc_linter/src/utils/jsdoc.rs index 08ce096d97f0f..9829a194143d1 100644 --- a/crates/oxc_linter/src/utils/jsdoc.rs +++ b/crates/oxc_linter/src/utils/jsdoc.rs @@ -1,6 +1,7 @@ use crate::{config::JSDocPluginSettings, context::LintContext, AstNode}; use oxc_ast::AstKind; use oxc_semantic::JSDoc; +use rustc_hash::FxHashSet; /// JSDoc is often attached on the parent node of a function. /// @@ -30,7 +31,10 @@ pub fn get_function_nearest_jsdoc_node<'a, 'b>( match current_node.kind() { AstKind::VariableDeclaration(_) | AstKind::MethodDefinition(_) - | AstKind::PropertyDefinition(_) => return None, + | AstKind::PropertyDefinition(_) + // /** This is NOT for `ArrowFunctionExpression` */ + // function outer() { inner(() => {}) } + | AstKind::CallExpression(_) => return None, _ => current_node = ctx.nodes().parent_node(current_node.id())?, } } @@ -69,3 +73,33 @@ pub fn should_ignore_as_private(jsdoc: &JSDoc, settings: &JSDocPluginSettings) - false } + +pub fn should_ignore_as_avoid( + jsdoc: &JSDoc, + settings: &JSDocPluginSettings, + exempted_tag_names: &[String], +) -> bool { + let mut ignore_tag_names = + exempted_tag_names.iter().map(std::convert::Into::into).collect::>(); + if settings.ignore_replaces_docs { + ignore_tag_names.insert(settings.resolve_tag_name("ignore")); + } + if settings.override_replaces_docs { + ignore_tag_names.insert(settings.resolve_tag_name("override")); + } + if settings.augments_extends_replaces_docs { + ignore_tag_names.insert(settings.resolve_tag_name("augments")); + ignore_tag_names.insert(settings.resolve_tag_name("extends")); + } + if settings.implements_replaces_docs { + ignore_tag_names.insert(settings.resolve_tag_name("implements")); + } + + for tag in jsdoc.tags() { + if ignore_tag_names.contains(tag.kind.parsed()) { + return true; + } + } + + false +} From 35ce483e089cebaac88262863583d6ce0e5ad1e1 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 2 May 2024 14:49:14 +0900 Subject: [PATCH 05/11] Base impl --- .../src/rules/jsdoc/require_yields.rs | 2472 +++++++++-------- 1 file changed, 1317 insertions(+), 1155 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs index ff040755784fc..79cf42342389d 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs @@ -4,10 +4,21 @@ use oxc_diagnostics::{ thiserror::Error, }; use oxc_macros::declare_oxc_lint; +use oxc_semantic::{JSDoc, JSDocTag}; use oxc_span::Span; +use rustc_hash::FxHashSet; use serde::Deserialize; -use crate::{context::LintContext, rule::Rule, AstNode}; +use crate::{ + config::JSDocPluginSettings, + context::LintContext, + rule::Rule, + utils::{ + get_function_nearest_jsdoc_node, should_ignore_as_avoid, should_ignore_as_internal, + should_ignore_as_private, + }, + AstNode, +}; #[derive(Debug, Error, Diagnostic)] #[error("eslint-plugin-jsdoc(require-yields): Missing `@yields` declaration.")] @@ -45,19 +56,19 @@ declare_oxc_lint!( #[derive(Debug, Clone, Deserialize)] struct RequireYieldsConfig { - #[serde(default, rename = "withGeneratorTag")] - with_generator_tag: bool, - #[serde(default = "default_true", rename = "forceRequireYields")] - force_require_yields: bool, #[serde(default = "default_exempted_by", rename = "exemptedBy")] exempted_by: Vec, + #[serde(default, rename = "forceRequireYields")] + force_require_yields: bool, + #[serde(default, rename = "withGeneratorTag")] + with_generator_tag: bool, } impl Default for RequireYieldsConfig { fn default() -> Self { Self { - with_generator_tag: false, - force_require_yields: true, exempted_by: default_exempted_by(), + force_require_yields: false, + with_generator_tag: false, } } } @@ -65,9 +76,6 @@ impl Default for RequireYieldsConfig { fn default_exempted_by() -> Vec { vec!["inheritdoc".to_string()] } -fn default_true() -> bool { - true -} impl Rule for RequireYields { fn from_configuration(value: serde_json::Value) -> Self { @@ -79,25 +87,180 @@ impl Rule for RequireYields { } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - let AstKind::YieldExpression(yield_expression) = node.kind() else { - return; - }; - let config = &self.0; - println!("๐Ÿ‘ป {config:#?}"); - let settings = &ctx.settings().jsdoc; - println!("๐Ÿ‘ป {settings:#?}"); - ctx.diagnostic(RequireYieldsDiagnostic(Span::default())); + match node.kind() { + AstKind::Function(func) + if func.generator && (func.is_expression() || func.is_declaration()) => + { + let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) + .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + else { + return; + }; + + let settings = &ctx.settings().jsdoc; + if jsdocs.iter().any(|jsdoc| { + should_ignore_as_skip(jsdoc) + || should_ignore_as_avoid(jsdoc, settings, &config.exempted_by) + || should_ignore_as_private(jsdoc, settings) + || should_ignore_as_internal(jsdoc, settings) + }) { + return; + } + + let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::>(); + + if config.force_require_yields && is_missing_yields_tag(&jsdoc_tags, settings) { + // TODO: Diagnostic w/ missing yields! + ctx.diagnostic(RequireYieldsDiagnostic(func.span)); + return; + } + + if let Some(span) = is_duplicated_yields_tag(&jsdoc_tags, settings) { + // TODO: Diagnostic w/ duplicate yields! + ctx.diagnostic(RequireYieldsDiagnostic(span)); + return; + } + + if config.with_generator_tag { + if let Some(span) = + is_missing_yields_tag_with_generator_tag(&jsdoc_tags, settings) + { + // TODO: Diagnostic w/ generator! + ctx.diagnostic(RequireYieldsDiagnostic(span)); + } + } + } + AstKind::YieldExpression(yield_expr) => { + // With this option, no needs to check `yield` value. + // We can perform all checks in `Function` branch instead. + if config.force_require_yields { + return; + } + + // Do not check `yield` without value + if yield_expr.argument.is_none() { + return; + } + + // Find the nearest generator function + let mut generator_func_node = None; + let mut current_node = node; + while let Some(parent_node) = ctx.nodes().parent_node(current_node.id()) { + // If syntax is valid, `yield` should be inside a generator function + if let AstKind::Function(func) = parent_node.kind() { + if func.generator && (func.is_expression() || func.is_declaration()) { + generator_func_node = Some((func, parent_node)); + break; + } + } + current_node = parent_node; + } + let Some((generator_func, generator_func_node)) = generator_func_node else { + return; + }; + + // If no JSDoc is found, skip + let Some(jsdocs) = get_function_nearest_jsdoc_node(generator_func_node, ctx) + .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + else { + return; + }; + + let settings = &ctx.settings().jsdoc; + if jsdocs.iter().any(|jsdoc| { + should_ignore_as_skip(jsdoc) + || should_ignore_as_avoid(jsdoc, settings, &config.exempted_by) + || should_ignore_as_private(jsdoc, settings) + || should_ignore_as_internal(jsdoc, settings) + }) { + return; + } + + let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::>(); + + if is_missing_yields_tag(&jsdoc_tags, settings) { + // TODO: Diagnostic w/ missing yields! + ctx.diagnostic(RequireYieldsDiagnostic(generator_func.span)); + } + } + _ => {} + } + } +} + +fn should_ignore_as_skip(jsdoc: &JSDoc) -> bool { + let ignore_tag_names = ["abstract", "virtual", "class", "constructor", "type", "interface"] + .iter() + .map(|s| (*s).to_string()) + .collect::>(); + + for tag in jsdoc.tags() { + if ignore_tag_names.contains(tag.kind.parsed()) { + return true; + } + } + + false +} + +fn is_missing_yields_tag(jsdoc_tags: &Vec<&JSDocTag>, settings: &JSDocPluginSettings) -> bool { + let resolved_yields_tag_name = settings.resolve_tag_name("yields"); + + for tag in jsdoc_tags { + if tag.kind.parsed() == resolved_yields_tag_name { + return false; + } + } + + true +} + +fn is_duplicated_yields_tag( + jsdoc_tags: &Vec<&JSDocTag>, + settings: &JSDocPluginSettings, +) -> Option { + let resolved_yields_tag_name = settings.resolve_tag_name("yields"); + + 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>, + settings: &JSDocPluginSettings, +) -> Option { + let resolved_yields_tag_name = settings.resolve_tag_name("yields"); + let resolved_generator_tag_name = settings.resolve_tag_name("generator"); + + let (mut yields_found, mut generator_found) = (None, None); + for tag in jsdoc_tags { + let tag_name = tag.kind.parsed(); - // if config.forceRequireYields { - // - // } else { - // if node != YieldExpression { return } - // if !yieldValue { return } - // const functionNode = parent(node); - // } + if tag_name == resolved_yields_tag_name { + yields_found = Some(tag.kind.span); + } + if tag_name == resolved_generator_tag_name { + generator_found = Some(tag.kind.span); + } + } + + if let (Some(generator_tag_span), None) = (generator_found, yields_found) { + return Some(generator_tag_span); } + + None } #[test] @@ -118,1138 +281,1137 @@ fn test() { None, None, ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function quux (bar) { - // bar.doSomething(function * (baz) { - // yield baz.corge(); - // }) - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @yields {Array} - // */ - // function * quux (bar) { - // yield bar.doSomething(function * (baz) { - // yield baz.corge(); - // }) - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @inheritdoc - // */ - // function * quux (foo) { - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @override - // */ - // function * quux (foo) { - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @constructor - // */ - // function * quux (foo) { - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @implements - // */ - // function * quux (foo) { - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @override - // */ - // function * quux (foo) { - - // yield foo; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @class - // */ - // function * quux (foo) { - // yield foo; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @yields {object} - // */ - // function * quux () { - - // yield {a: foo}; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @yields {void} - // */ - // function * quux () { - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @yields {undefined} - // */ - // function * quux () { - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @yields {void} - // */ - // function quux () { - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @yields {void} - // */ - // function * quux () { - // yield undefined; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @yields {void} - // */ - // function * quux () { - // yield undefined; - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @yields {void} - // */ - // function * quux () { - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @yields {void} - // */ - // function * quux () { - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @yields {void} - // */ - // function * quux () { - // yield; - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** @type {SpecialIterator} */ - // function * quux () { - // yield 5; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @yields {Something} - // */ - // async function * quux () { - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * - // */ - // async function * quux () {} - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // const quux = async function * () {} - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @type {MyCallback} - // */ - // function * quux () { - // yield; - // } - // ", - // Some(serde_json::json!([ - // { - // "exemptedBy": [ - // "type", - // ], - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @param {array} a - // */ - // async function * foo (a) { - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @function - // */ - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @callback - // */ - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @generator - // */ - // ", - // Some(serde_json::json!([ - // { - // "withGeneratorTag": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @generator - // * @yields - // */ - // *function() {} - // ", - // Some(serde_json::json!([ - // { - // "withGeneratorTag": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @yields - // */ - // function * quux (foo) { - - // const a = yield foo; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux (foo) { - // const a = function * bar () { - // yield foo; - // } - // } - // ", - // None, - // None, - // ), - ]; - - let fail = vec![ - // ( - // " - // /** - // * - // */ - // function * quux (foo) { - - // yield foo; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux (foo) { - // someLabel: { - // yield foo; - // } - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux (foo) { - - // const a = yield foo; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux (foo) { - // yield foo; - // } - // ", - // None, - // Some(serde_json::json!({ "settings": { - // "jsdoc": { - // "tagNamePreference": { - // "yields": "yield", - // }, - // }, - // } })), - // ), - // ( - // " - // /** - // * - // */ - // function * quux() { - // yield 5; - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux() { - // yield; - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * - // */ - // const quux = async function * () { - // yield; - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * - // */ - // async function * quux () { - // yield; - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // yield; - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @function - // * @generator - // */ - // *function() {} - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @yields {undefined} - // * @yields {void} - // */ - // function * quux (foo) { + ( + " + /** + * pass(without yield, no config) + */ + function * quux () { + } + ", + None, + None, + ), + ( + " + /** + * + */ + function * quux () { + yield; + } + ", + None, + None, + ), + ( + " + /** + * + */ + function quux (bar) { + bar.doSomething(function * (baz) { + yield baz.corge(); + }) + } + ", + None, + None, + ), + ( + " + /** + * @yields {Array} + */ + function * quux (bar) { + yield bar.doSomething(function * (baz) { + yield baz.corge(); + }) + } + ", + None, + None, + ), + ( + " + /** + * @inheritdoc + */ + function * quux (foo) { + } + ", + None, + None, + ), + ( + " + /** + * @override + */ + function * quux (foo) { + } + ", + None, + None, + ), + ( + " + /** + * @constructor + */ + function * quux (foo) { + } + ", + None, + None, + ), + ( + " + /** + * @implements + */ + function * quux (foo) { + yield; + } + ", + None, + None, + ), + ( + " + /** + * pass(`@override` found, settings should be default true) + * @override + */ + function * quux (foo) { - // return foo; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @param foo - // */ - // function * quux (foo) { - // yield 'bar'; - // } - // ", - // Some(serde_json::json!([ - // { - // "exemptedBy": [ - // "notPresent", - // ], - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @param {array} a - // */ - // async function * foo(a) { - // return; - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @param {array} a - // */ - // async function * foo(a) { - // yield Promise.all(a); - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // class quux { - // /** - // * - // */ - // * quux () { - // yield; - // } - // } - // ", - // Some(serde_json::json!([ - // { - // "forceRequireYields": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @param {array} a - // */ - // async function * foo(a) { - // yield Promise.all(a); - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // if (true) { - // yield; - // } - // yield true; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // if (yield false) { + yield foo; + } + ", + None, + None, + ), + ( + " + /** + * @class + */ + function * quux (foo) { + yield foo; + } + ", + None, + None, + ), + ( + " + /** + * @yields {object} + */ + function * quux () { - // } - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // b ? yield false : true - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // try { - // yield true; - // } catch (err) { - // } - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // try { - // } finally { - // yield true; - // } - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // try { - // yield; - // } catch (err) { - // } - // yield true; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // try { - // something(); - // } catch (err) { - // yield true; - // } - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // switch (true) { - // case 'abc': - // yield true; - // } - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // switch (true) { - // case 'abc': - // yield; - // } - // yield true; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // for (const i of abc) { - // yield true; - // } - // yield; - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // for (const a in b) { - // yield true; - // } - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * - // */ - // function * quux () { - // for (let i=0; i Date: Thu, 2 May 2024 15:07:56 +0900 Subject: [PATCH 06/11] Cover more test cases --- .../src/rules/jsdoc/require_yields.rs | 51 ++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs index 79cf42342389d..1c1a2f679bdd0 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs @@ -21,9 +21,19 @@ use crate::{ }; #[derive(Debug, Error, Diagnostic)] -#[error("eslint-plugin-jsdoc(require-yields): Missing `@yields` declaration.")] -#[diagnostic(severity(warning), help("Add `@yields` tag to the JSDoc comment."))] -struct RequireYieldsDiagnostic(#[label] pub Span); +enum RequireYieldsDiagnostic { + #[error("eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function.")] + #[diagnostic(severity(warning), help("Add `@yields` tag to the JSDoc comment."))] + MissingYields(#[label] Span), + #[error("eslint-plugin-jsdoc(require-yields): Duplicate `@yields` tags.")] + #[diagnostic(severity(warning), help("Remove redundunt `@yields` tag."))] + DuplicateYields(#[label] Span), + #[error( + "eslint-plugin-jsdoc(require-yields): `@yields` tag is missing with `@generator` tag." + )] + #[diagnostic(severity(warning), help("Add `@yields` tag to the JSDoc comment."))] + MissingYieldsWithGenerator(#[label] Span), +} #[derive(Debug, Default, Clone)] pub struct RequireYields(Box); @@ -112,14 +122,12 @@ impl Rule for RequireYields { let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::>(); if config.force_require_yields && is_missing_yields_tag(&jsdoc_tags, settings) { - // TODO: Diagnostic w/ missing yields! - ctx.diagnostic(RequireYieldsDiagnostic(func.span)); + ctx.diagnostic(RequireYieldsDiagnostic::MissingYields(func.span)); return; } if let Some(span) = is_duplicated_yields_tag(&jsdoc_tags, settings) { - // TODO: Diagnostic w/ duplicate yields! - ctx.diagnostic(RequireYieldsDiagnostic(span)); + ctx.diagnostic(RequireYieldsDiagnostic::DuplicateYields(span)); return; } @@ -127,8 +135,7 @@ impl Rule for RequireYields { if let Some(span) = is_missing_yields_tag_with_generator_tag(&jsdoc_tags, settings) { - // TODO: Diagnostic w/ generator! - ctx.diagnostic(RequireYieldsDiagnostic(span)); + ctx.diagnostic(RequireYieldsDiagnostic::MissingYieldsWithGenerator(span)); } } } @@ -181,8 +188,7 @@ impl Rule for RequireYields { let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::>(); if is_missing_yields_tag(&jsdoc_tags, settings) { - // TODO: Diagnostic w/ missing yields! - ctx.diagnostic(RequireYieldsDiagnostic(generator_func.span)); + ctx.diagnostic(RequireYieldsDiagnostic::MissingYields(generator_func.span)); } } _ => {} @@ -338,6 +344,7 @@ fn test() { * @inheritdoc */ function * quux (foo) { + yield 'inherit!'; } ", None, @@ -572,17 +579,15 @@ fn test() { ( " /** - * @type {MyCallback} + * @mytype {MyCallback} */ function * quux () { - yield; + yield 2; } ", Some(serde_json::json!([ { - "exemptedBy": [ - "type", - ], + "exemptedBy": ["mytype"], }, ])), None, @@ -1412,8 +1417,18 @@ fn test() { None, None, ), + ( + " + /** + * fail(`@generator`+missing `@yields`, with config) + * @generator + */ + function*() {} + ", + Some(serde_json::json!([{ "withGeneratorTag": true, }])), + None, + ), ]; - Tester::new(RequireYields::NAME, pass, fail).test(); - // Tester::new(RequireYields::NAME, pass, fail).test_and_snapshot(); + Tester::new(RequireYields::NAME, pass, fail).test_and_snapshot(); } From 6827fe356ba5b1b3fda0292bcf439ed19c133bbf Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 2 May 2024 16:14:31 +0900 Subject: [PATCH 07/11] Fix up --- .../src/rules/jsdoc/require_yields.rs | 129 ++-- .../src/snapshots/require_yields.snap | 610 ++++++++++++++++++ 2 files changed, 689 insertions(+), 50 deletions(-) create mode 100644 crates/oxc_linter/src/snapshots/require_yields.snap diff --git a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs index 1c1a2f679bdd0..da4d92b50b734 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs @@ -6,11 +6,10 @@ use oxc_diagnostics::{ use oxc_macros::declare_oxc_lint; use oxc_semantic::{JSDoc, JSDocTag}; use oxc_span::Span; -use rustc_hash::FxHashSet; +use phf::phf_set; use serde::Deserialize; use crate::{ - config::JSDocPluginSettings, context::LintContext, rule::Rule, utils::{ @@ -29,7 +28,7 @@ enum RequireYieldsDiagnostic { #[diagnostic(severity(warning), help("Remove redundunt `@yields` tag."))] DuplicateYields(#[label] Span), #[error( - "eslint-plugin-jsdoc(require-yields): `@yields` tag is missing with `@generator` tag." + "eslint-plugin-jsdoc(require-yields): `@yields` tag is requried when using `@generator` tag." )] #[diagnostic(severity(warning), help("Add `@yields` tag to the JSDoc comment."))] MissingYieldsWithGenerator(#[label] Span), @@ -99,10 +98,24 @@ impl Rule for RequireYields { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let config = &self.0; + // This rule checks generator function should have JSDoc `@yields` tag. + // By default, this rule only checks: + // ``` + // function*() { yield withValue; } + // ``` + // + // If `config.forceRequireYields` is `true`, also checks: + // ``` + // function*() {} + // function*() { yield; } + // ``` + // + // If generator function does not have JSDoc, it will be skipped. match node.kind() { AstKind::Function(func) if func.generator && (func.is_expression() || func.is_declaration()) => { + // If no JSDoc is found, skip let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) .and_then(|node| ctx.jsdoc().get_all_by_node(node)) else { @@ -110,35 +123,59 @@ impl Rule for RequireYields { }; let settings = &ctx.settings().jsdoc; - if jsdocs.iter().any(|jsdoc| { - should_ignore_as_skip(jsdoc) - || should_ignore_as_avoid(jsdoc, settings, &config.exempted_by) - || should_ignore_as_private(jsdoc, settings) - || should_ignore_as_internal(jsdoc, settings) - }) { + // If JSDoc is found but safely ignored, skip + if jsdocs + .iter() + .filter(|jsdoc| !should_ignore_as_custom_skip(jsdoc)) + .filter(|jsdoc| !should_ignore_as_avoid(jsdoc, settings, &config.exempted_by)) + .filter(|jsdoc| !should_ignore_as_private(jsdoc, settings)) + .filter(|jsdoc| !should_ignore_as_internal(jsdoc, settings)) + .count() + == 0 + { return; } let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::>(); + let resolved_yields_tag_name = settings.resolve_tag_name("yields"); - if config.force_require_yields && is_missing_yields_tag(&jsdoc_tags, settings) { + // 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) + { ctx.diagnostic(RequireYieldsDiagnostic::MissingYields(func.span)); return; } - if let Some(span) = is_duplicated_yields_tag(&jsdoc_tags, settings) { + // Other checks are always performed + + if let Some(span) = is_duplicated_yields_tag(&jsdoc_tags, &resolved_yields_tag_name) + { ctx.diagnostic(RequireYieldsDiagnostic::DuplicateYields(span)); return; } if config.with_generator_tag { - if let Some(span) = - is_missing_yields_tag_with_generator_tag(&jsdoc_tags, settings) - { + let resolved_generator_tag_name = settings.resolve_tag_name("generator"); + + if let Some(span) = is_missing_yields_tag_with_generator_tag( + &jsdoc_tags, + &resolved_yields_tag_name, + &resolved_generator_tag_name, + ) { ctx.diagnostic(RequireYieldsDiagnostic::MissingYieldsWithGenerator(span)); } } } + // Q. Why not perform all checks in `Function` branch? + // A. Rule behavior is different whether `yield` value is present or not. + // + // `oxc_semantic` add node flag to detect `yield` is used or NOT. + // But existence of value is still unknown. + // + // Find `YieldExpression` inside of `(Generator)Function` requires more complex logic. + // Use bottom-up approach to find the nearest generator function instead. AstKind::YieldExpression(yield_expr) => { // With this option, no needs to check `yield` value. // We can perform all checks in `Function` branch instead. @@ -176,18 +213,23 @@ impl Rule for RequireYields { }; let settings = &ctx.settings().jsdoc; - if jsdocs.iter().any(|jsdoc| { - should_ignore_as_skip(jsdoc) - || should_ignore_as_avoid(jsdoc, settings, &config.exempted_by) - || should_ignore_as_private(jsdoc, settings) - || should_ignore_as_internal(jsdoc, settings) - }) { + // If JSDoc is found but safely ignored, skip + if jsdocs + .iter() + .filter(|jsdoc| !should_ignore_as_custom_skip(jsdoc)) + .filter(|jsdoc| !should_ignore_as_avoid(jsdoc, settings, &config.exempted_by)) + .filter(|jsdoc| !should_ignore_as_private(jsdoc, settings)) + .filter(|jsdoc| !should_ignore_as_internal(jsdoc, settings)) + .count() + == 0 + { return; } let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::>(); + let resolved_yields_tag_name = settings.resolve_tag_name("yields"); - if is_missing_yields_tag(&jsdoc_tags, settings) { + if is_missing_yields_tag(&jsdoc_tags, &resolved_yields_tag_name) { ctx.diagnostic(RequireYieldsDiagnostic::MissingYields(generator_func.span)); } } @@ -196,39 +238,28 @@ impl Rule for RequireYields { } } -fn should_ignore_as_skip(jsdoc: &JSDoc) -> bool { - let ignore_tag_names = ["abstract", "virtual", "class", "constructor", "type", "interface"] +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() - .map(|s| (*s).to_string()) - .collect::>(); - - for tag in jsdoc.tags() { - if ignore_tag_names.contains(tag.kind.parsed()) { - return true; - } - } - - false + .map(|tag| tag.kind.parsed()) + .any(|tag_name| CUSTOM_SKIP_TAG_NAMES.contains(tag_name)) } -fn is_missing_yields_tag(jsdoc_tags: &Vec<&JSDocTag>, settings: &JSDocPluginSettings) -> bool { - let resolved_yields_tag_name = settings.resolve_tag_name("yields"); - - for tag in jsdoc_tags { - if tag.kind.parsed() == resolved_yields_tag_name { - return false; - } - } - - true +fn is_missing_yields_tag(jsdoc_tags: &[&JSDocTag], resolved_yields_tag_name: &str) -> bool { + jsdoc_tags + .iter() + .map(|tag| tag.kind.parsed()) + .all(|tag_name| tag_name != resolved_yields_tag_name) } fn is_duplicated_yields_tag( jsdoc_tags: &Vec<&JSDocTag>, - settings: &JSDocPluginSettings, + resolved_yields_tag_name: &str, ) -> Option { - let resolved_yields_tag_name = settings.resolve_tag_name("yields"); - let mut yields_found = false; for tag in jsdoc_tags { if tag.kind.parsed() == resolved_yields_tag_name { @@ -245,11 +276,9 @@ fn is_duplicated_yields_tag( fn is_missing_yields_tag_with_generator_tag( jsdoc_tags: &Vec<&JSDocTag>, - settings: &JSDocPluginSettings, + resolved_yields_tag_name: &str, + resolved_generator_tag_name: &str, ) -> Option { - let resolved_yields_tag_name = settings.resolve_tag_name("yields"); - let resolved_generator_tag_name = settings.resolve_tag_name("generator"); - let (mut yields_found, mut generator_found) = (None, None); for tag in jsdoc_tags { let tag_name = tag.kind.parsed(); diff --git a/crates/oxc_linter/src/snapshots/require_yields.snap b/crates/oxc_linter/src/snapshots/require_yields.snap new file mode 100644 index 0000000000000..558c828809bdc --- /dev/null +++ b/crates/oxc_linter/src/snapshots/require_yields.snap @@ -0,0 +1,610 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: require_yields +--- + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ function * quux (foo) { yield foo; } + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 6 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux (foo) { + 6 โ”‚ โ”‚ someLabel: { + 7 โ”‚ โ”‚ yield foo; + 8 โ”‚ โ”‚ } + 9 โ”‚ โ•ฐโ”€โ–ถ } + 10 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux (foo) { + 6 โ”‚ โ”‚ + 7 โ”‚ โ”‚ const a = yield foo; + 8 โ”‚ โ•ฐโ”€โ–ถ } + 9 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux (foo) { + 6 โ”‚ โ”‚ yield foo; + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux() { + 6 โ”‚ โ”‚ yield 5; + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux() { + 6 โ”‚ โ”‚ yield; + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:33] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ const quux = async function * () { + 6 โ”‚ โ”‚ yield; + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:21] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ async function * quux () { + 6 โ”‚ โ”‚ yield; + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ yield; + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:6:25] + 5 โ”‚ */ + 6 โ”‚ function*() {} + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 7 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Duplicate `@yields` tags. + โ•ญโ”€[require_yields.tsx:4:23] + 3 โ”‚ * @yields {undefined} + 4 โ”‚ * @yields {void} + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 5 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: Remove redundunt `@yields` tag. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux (foo) { + 6 โ”‚ โ”‚ yield 'bar'; + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:16] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ async function * foo(a) { + 6 โ”‚ โ”‚ return; + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:16] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ async function * foo(a) { + 6 โ”‚ โ”‚ yield Promise.all(a); + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:6:25] + 5 โ”‚ */ + 6 โ”‚ โ•ญโ”€โ–ถ * quux () { + 7 โ”‚ โ”‚ yield; + 8 โ”‚ โ•ฐโ”€โ–ถ } + 9 โ”‚ } + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:16] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ async function * foo(a) { + 6 โ”‚ โ”‚ yield Promise.all(a); + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ if (true) { + 7 โ”‚ โ”‚ yield; + 8 โ”‚ โ”‚ } + 9 โ”‚ โ”‚ yield true; + 10 โ”‚ โ•ฐโ”€โ–ถ } + 11 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ if (yield false) { + 7 โ”‚ โ”‚ + 8 โ”‚ โ”‚ } + 9 โ”‚ โ•ฐโ”€โ–ถ } + 10 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ b ? yield false : true + 7 โ”‚ โ•ฐโ”€โ–ถ } + 8 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ try { + 7 โ”‚ โ”‚ yield true; + 8 โ”‚ โ”‚ } catch (err) { + 9 โ”‚ โ”‚ } + 10 โ”‚ โ”‚ yield; + 11 โ”‚ โ•ฐโ”€โ–ถ } + 12 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ try { + 7 โ”‚ โ”‚ } finally { + 8 โ”‚ โ”‚ yield true; + 9 โ”‚ โ”‚ } + 10 โ”‚ โ”‚ yield; + 11 โ”‚ โ•ฐโ”€โ–ถ } + 12 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ try { + 7 โ”‚ โ”‚ } finally { + 8 โ”‚ โ”‚ yield true; + 9 โ”‚ โ”‚ } + 10 โ”‚ โ”‚ yield; + 11 โ”‚ โ•ฐโ”€โ–ถ } + 12 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ try { + 7 โ”‚ โ”‚ yield; + 8 โ”‚ โ”‚ } catch (err) { + 9 โ”‚ โ”‚ } + 10 โ”‚ โ”‚ yield true; + 11 โ”‚ โ•ฐโ”€โ–ถ } + 12 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ try { + 7 โ”‚ โ”‚ something(); + 8 โ”‚ โ”‚ } catch (err) { + 9 โ”‚ โ”‚ yield true; + 10 โ”‚ โ”‚ } + 11 โ”‚ โ”‚ yield; + 12 โ”‚ โ•ฐโ”€โ–ถ } + 13 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ switch (true) { + 7 โ”‚ โ”‚ case 'abc': + 8 โ”‚ โ”‚ yield true; + 9 โ”‚ โ”‚ } + 10 โ”‚ โ”‚ yield; + 11 โ”‚ โ•ฐโ”€โ–ถ } + 12 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ switch (true) { + 7 โ”‚ โ”‚ case 'abc': + 8 โ”‚ โ”‚ yield; + 9 โ”‚ โ”‚ } + 10 โ”‚ โ”‚ yield true; + 11 โ”‚ โ•ฐโ”€โ–ถ } + 12 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ for (const i of abc) { + 7 โ”‚ โ”‚ yield true; + 8 โ”‚ โ”‚ } + 9 โ”‚ โ”‚ yield; + 10 โ”‚ โ•ฐโ”€โ–ถ } + 11 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ for (const a in b) { + 7 โ”‚ โ”‚ yield true; + 8 โ”‚ โ”‚ } + 9 โ”‚ โ•ฐโ”€โ–ถ } + 10 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: Add `@yields` tag to the JSDoc comment. + + โš  eslint-plugin-jsdoc(require-yields): Missing JSDoc `@yields` declaration for generator function. + โ•ญโ”€[require_yields.tsx:5:20] + 4 โ”‚ */ + 5 โ”‚ โ•ญโ”€โ–ถ function * quux () { + 6 โ”‚ โ”‚ for (let i=0; i Date: Thu, 2 May 2024 16:18:33 +0900 Subject: [PATCH 08/11] Refactor --- crates/oxc_linter/src/rules/jsdoc/require_yields.rs | 11 ++--------- crates/oxc_linter/src/utils/jsdoc.rs | 8 +------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs index da4d92b50b734..fb29f73cfa9f3 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs @@ -242,18 +242,11 @@ 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() - .map(|tag| tag.kind.parsed()) - .any(|tag_name| CUSTOM_SKIP_TAG_NAMES.contains(tag_name)) + 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() - .map(|tag| tag.kind.parsed()) - .all(|tag_name| tag_name != resolved_yields_tag_name) + jsdoc_tags.iter().all(|tag| tag.kind.parsed() != resolved_yields_tag_name) } fn is_duplicated_yields_tag( diff --git a/crates/oxc_linter/src/utils/jsdoc.rs b/crates/oxc_linter/src/utils/jsdoc.rs index 9829a194143d1..148067777ff2e 100644 --- a/crates/oxc_linter/src/utils/jsdoc.rs +++ b/crates/oxc_linter/src/utils/jsdoc.rs @@ -95,11 +95,5 @@ pub fn should_ignore_as_avoid( ignore_tag_names.insert(settings.resolve_tag_name("implements")); } - for tag in jsdoc.tags() { - if ignore_tag_names.contains(tag.kind.parsed()) { - return true; - } - } - - false + jsdoc.tags().iter().any(|tag| ignore_tag_names.contains(tag.kind.parsed())) } From 8754ee0c256409aec6dcd98c34fb25b63c1cc96c Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 2 May 2024 16:22:19 +0900 Subject: [PATCH 09/11] Update comment --- crates/oxc_linter/src/utils/jsdoc.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/utils/jsdoc.rs b/crates/oxc_linter/src/utils/jsdoc.rs index 148067777ff2e..ee916f879e391 100644 --- a/crates/oxc_linter/src/utils/jsdoc.rs +++ b/crates/oxc_linter/src/utils/jsdoc.rs @@ -28,11 +28,12 @@ pub fn get_function_nearest_jsdoc_node<'a, 'b>( // Whether the node has attached JSDoc or not is determined by `JSDocBuilder` while ctx.jsdoc().get_all_by_node(current_node).is_none() { // Tie-breaker, otherwise every loop will end at `Program` node! + // Maybe more checks should be added match current_node.kind() { AstKind::VariableDeclaration(_) | AstKind::MethodDefinition(_) | AstKind::PropertyDefinition(_) - // /** This is NOT for `ArrowFunctionExpression` */ + // /** This JSDoc should NOT found for `ArrowFunctionExpression` callback */ // function outer() { inner(() => {}) } | AstKind::CallExpression(_) => return None, _ => current_node = ctx.nodes().parent_node(current_node.id())?, From 6545188a6d5caaaa66a1b5eba996f2e8c024754e Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 2 May 2024 16:27:04 +0900 Subject: [PATCH 10/11] Fix typo --- crates/oxc_linter/src/rules/jsdoc/require_yields.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs index fb29f73cfa9f3..a0473661360fe 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs @@ -28,7 +28,7 @@ enum RequireYieldsDiagnostic { #[diagnostic(severity(warning), help("Remove redundunt `@yields` tag."))] DuplicateYields(#[label] Span), #[error( - "eslint-plugin-jsdoc(require-yields): `@yields` tag is requried when using `@generator` tag." + "eslint-plugin-jsdoc(require-yields): `@yields` tag is required when using `@generator` tag." )] #[diagnostic(severity(warning), help("Add `@yields` tag to the JSDoc comment."))] MissingYieldsWithGenerator(#[label] Span), From 73d06c91dccbed5d03ec2b60147fedc0180d0a72 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 2 May 2024 16:33:05 +0900 Subject: [PATCH 11/11] Update snapshot --- crates/oxc_linter/src/snapshots/require_yields.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/snapshots/require_yields.snap b/crates/oxc_linter/src/snapshots/require_yields.snap index 558c828809bdc..78ec43cbefdf2 100644 --- a/crates/oxc_linter/src/snapshots/require_yields.snap +++ b/crates/oxc_linter/src/snapshots/require_yields.snap @@ -600,7 +600,7 @@ expression: require_yields โ•ฐโ”€โ”€โ”€โ”€ help: Add `@yields` tag to the JSDoc comment. - โš  eslint-plugin-jsdoc(require-yields): `@yields` tag is requried when using `@generator` tag. + โš  eslint-plugin-jsdoc(require-yields): `@yields` tag is required when using `@generator` tag. โ•ญโ”€[require_yields.tsx:4:25] 3 โ”‚ * fail(`@generator`+missing `@yields`, with config) 4 โ”‚ * @generator