From cd7b95087e29ec7ba68aed9e3dc460a12866e8dd Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 18 Apr 2024 13:31:12 +0900 Subject: [PATCH 1/9] Wip --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/jsdoc/check_tag_names.rs | 939 ++++++++++++++++++ 2 files changed, 941 insertions(+) create mode 100644 crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index fc185e516505b..a8a10ceafa6c0 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -360,6 +360,7 @@ mod nextjs { mod jsdoc { pub mod check_access; pub mod check_property_names; + pub mod check_tag_names; pub mod empty_tags; pub mod require_property; pub mod require_property_description; @@ -691,6 +692,7 @@ oxc_macros::declare_all_lint_rules! { nextjs::no_before_interactive_script_outside_document, jsdoc::check_access, jsdoc::check_property_names, + jsdoc::check_tag_names, jsdoc::empty_tags, jsdoc::require_property, jsdoc::require_property_type, diff --git a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs new file mode 100644 index 0000000000000..7ae078ed8465b --- /dev/null +++ b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs @@ -0,0 +1,939 @@ +use itertools::Itertools; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use phf::phf_set; +use serde::Deserialize; + +use crate::{context::LintContext, rule::Rule}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-jsdoc(check-tag-names): Invalid tag name found.")] +#[diagnostic(severity(warning), help("{1}"))] +struct CheckTagNamesDiagnostic(#[label] pub Span, String); + +#[derive(Debug, Default, Clone)] +pub struct CheckTagNames(Box); + +declare_oxc_lint!( + /// ### What it does + /// Reports invalid block tag names. + /// Additionally checks for tag names that are redundant when using a type checker such as TypeScript. + /// + /// ### Why is this bad? + /// Using invalid tags can lead to confusion and make the documentation harder to read. + /// + /// ### Example + /// ```javascript + /// // Passing + /// /** @param */ + /// + /// // Failing + /// /** @Param */ + /// /** @foo */ + /// + /// /** + /// * This is redundant when typed. + /// * @type {string} + /// */ + /// ``` + CheckTagNames, + correctness +); + +#[derive(Debug, Default, Clone, Deserialize)] +struct CheckTagnamesConfig { + #[serde(default, rename = "definedTags")] + defined_tags: Vec, + #[serde(default, rename = "jsxTags")] + jsx_tags: bool, + #[serde(default)] + typed: bool, +} + +const VALID_BLOCK_TAGS: phf::Set<&'static str> = phf_set! { +"abstract", +"access", +"alias", +"async", +"augments", +"author", +"borrows", +"callback", +"class", +"classdesc", +"constant", +"constructs", +"copyright", +"default", +"deprecated", +"description", +"enum", +"event", +"example", +"exports", +"external", +"file", +"fires", +"function", +"generator", +"global", +"hideconstructor", +"ignore", +"implements", +"inheritdoc", +"inner", +"instance", +"interface", +"kind", +"lends", +"license", +"listens", +"member", +"memberof", +"memberof!", +"mixes", +"mixin", +"module", +"name", +"namespace", +"override", +"package", +"param", +"private", +"property", +"protected", +"public", +"readonly", +"requires", +"returns", +"see", +"since", +"static", +"summary", +"this", +"throws", +"todo", +"tutorial", +"type", +"typedef", +"variation", +"version", +"yields" +}; + +impl Rule for CheckTagNames { + 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(&self, ctx: &LintContext) { + let settings = &ctx.settings().jsdoc; + let config = &self.0; + println!("๐Ÿ‘ป {config:?}"); + println!("๐Ÿ‘ป {settings:?}"); + + let user_preferred_tags = settings.list_preferred_tag_names(); + let is_valid_tag = |tag_name: &str| { + if config.defined_tags.contains(&tag_name.to_string()) + || user_preferred_tags.contains(&tag_name.to_string()) + { + return true; + } + + if VALID_BLOCK_TAGS.contains(&settings.resolve_tag_name(tag_name)) { + return true; + } + false + }; + + // TODO: typed, d.ts + for jsdoc in ctx.semantic().jsdoc().iter_all() { + for tag in jsdoc.tags() { + let tag_name = tag.kind.parsed(); + println!("๐Ÿ‘ป {tag_name}"); + + if let Some(message) = settings.is_blocked_tag_name(tag_name) { + ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, message)); + continue; + } + println!(" => not blocked"); + + if !is_valid_tag(tag_name) { + ctx.diagnostic(CheckTagNamesDiagnostic( + tag.kind.span, + format!("`@{tag_name}` is invalid tag name."), + )); + continue; + } + println!(" => is valid"); + } + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ +// (" +// /** @default 0 */ +// let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @default 0 */ +// declare let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @abstract */ +// let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @abstract */ +// declare let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @abstract */ +// { declare let a; } +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// function test() { +// /** @abstract */ +// declare let a; +// } +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @template name */ +// let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @param param - takes information */ +// function takesOne(param) {} +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +(" + /** + * @param foo + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @memberof! foo + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @arg foo + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "param": "arg", + }, + }, + }))), +(" + /** + * @bar foo + */ + function quux (foo) { + + } + ", Some(serde_json::json!([ + { + "definedTags": [ + "bar", + ], + }, + ])), None), +(" + /** + * @baz @bar foo + */ + function quux (foo) { + + } + ", Some(serde_json::json!([ + { + "definedTags": [ + "baz", "bar", + ], + }, + ])), None), +(" + /** + * @baz @bar foo + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "param": "baz", + "returns": { + "message": "Prefer `bar`", + "replacement": "bar", + }, + "todo": false, + }, + }, + }))), +(" + /** + * @returns + */ + function quux (foo) {} + ", None, None), +("", None, None), +(" + /** + * + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @todo + */ + function quux () { + + } + ", None, None), +(" + /** + * @extends Foo + */ + function quux () { + + } + ", None, Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "augments": { + "message": "@extends is to be used over @augments.", + "replacement": "extends", + }, + }, + }, + }))), +(" + /** + * (Set tag name preference to itself to get aliases to + * work along with main tag name.) + * @augments Bar + * @extends Foo + */ + function quux () { + } + ", None, Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "extends": "extends", + }, + }, + }))), +(" + /** + * Registers the `target` class as a transient dependency; each time the dependency is resolved a new instance will be created. + * + * @param target - The class / constructor function to register as transient. + * + * @example ```ts + @transient() + class Foo { } + ``` + * @param Time for a new tag + */ + export function transient(target?: T): T { + // ... + } + ", None, None), +(" + /** @jsx h */ + /** @jsxFrag Fragment */ + /** @jsxImportSource preact */ + /** @jsxRuntime automatic */ + ", Some(serde_json::json!([ + { + "jsxTags": true, + }, + ])), None), +(" + /** + * @internal + */ + ", None, Some(serde_json::json!({ + "jsdoc": { + }, + }))), +(" + interface WebTwain { + /** + * Converts the images specified by the indices to base64 synchronously. + * @function WebTwain#ConvertToBase64 + * @returns {Base64Result} + + ConvertToBase64(): Base64Result; + */ + + /** + * Converts the images specified by the indices to base64 asynchronously. + * @function WebTwain#ConvertToBase64 + * @returns {boolean} + */ + ConvertToBase64(): boolean; + } + ", None, None), +(" + /** + * @overload + * @satisfies + */ + ", None, Some(serde_json::json!({ + "jsdoc": { + }, + }))), +// (" +// /** +// * @module +// * A comment related to the module +// */ +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None) + ]; + + let fail = vec![ + // ( + // "/** @type {string} */let a; + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * Existing comment. + // * @type {string} + // */ + // let a; + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** @abstract */ + // let a; + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // const a = { + // /** @abstract */ + // b: true, + // }; + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** @template */ + // let a; + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * Prior description. + // * + // * @template + // */ + // let a; + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + ( + " + /** @typoo {string} */ + let a; + ", + None, + None, + ), + ( + " + /** + * @Param + */ + function quux () { + + } + ", + None, + None, + ), + ( + " + /** + * @foo + */ + function quux () { + + } + ", + None, + None, + ), + ( + " + /** + * @arg foo + */ + function quux (foo) { + + } + ", + None, + None, + ), + // ( + // " + // /** + // * @param foo + // */ + // function quux (foo) { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "param": "arg", + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @constructor foo + // */ + // function quux (foo) { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "constructor": "cons", + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @arg foo + // */ + // function quux (foo) { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "arg": "somethingDifferent", + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @param foo + // */ + // function quux (foo) { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "param": "parameter", + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @bar foo + // */ + // function quux (foo) { + + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @baz @bar foo + // */ + // function quux (foo) { + + // } + // ", + // Some(serde_json::json!([ + // { + // "definedTags": [ + // "bar", + // ], + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @bar + // * @baz + // */ + // function quux (foo) { + + // } + // ", + // Some(serde_json::json!([ + // { + // "definedTags": [ + // "bar", + // ], + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @todo + // */ + // function quux () { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "todo": false, + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @todo + // */ + // function quux () { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "todo": { + // "message": "Please resolve to-dos or add to the tracker", + // }, + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @todo + // */ + // function quux () { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "todo": { + // "message": "Please use x-todo instead of todo", + // "replacement": "x-todo", + // }, + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @todo + // */ + // function quux () { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "todo": 55, + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @property {object} a + // * @prop {boolean} b + // */ + // function quux () { + + // } + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @abc foo + // * @abcd bar + // */ + // function quux () { + + // } + // ", + // Some(serde_json::json!([ + // { + // "definedTags": [ + // "abcd", + // ], + // }, + // ])), + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "abc": "abcd", + // }, + // }, + // })), + // ), + // ( + // " + // /** + // * @abc + // * @abcd + // */ + // function quux () { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "abc": "abcd", + // }, + // }, + // })), + // ), + ("", None, None), + // ( + // " + // /** @jsx h */ + // /** @jsxFrag Fragment */ + // /** @jsxImportSource preact */ + // /** @jsxRuntime automatic */ + // ", + // None, + // None, + // ), + // ( + // " + // /** + // * @constructor + // */ + // function Test() { + // this.works = false; + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "returns": "return", + // }, + // }, + // })), + // ), + // ( + // " + // /** @typedef {Object} MyObject + // * @property {string} id - my id + // */ + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @property {string} id - my id + // */ + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** @typedef {Object} MyObject */ + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** @typedef {Object} MyObject + // */ + // ", + // Some(serde_json::json!([ + // { + // "typed": true, + // }, + // ])), + // None, + // ), + // ( + // " + // /** + // * @todo + // */ + // function quux () { + + // } + // ", + // None, + // Some(serde_json::json!({ + // "jsdoc": { + // "tagNamePreference": { + // "todo": { + // "message": "Please don\"t use todo", + // }, + // }, + // }, + // })), + // ), + ]; + + Tester::new(CheckTagNames::NAME, pass, fail).test_and_snapshot(); +} From 691fc5db93afc88b61afe00efd777d36f6816470 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Fri, 19 Apr 2024 18:22:46 +0900 Subject: [PATCH 2/9] Cover basic cases --- .../oxc_linter/src/config/settings/jsdoc.rs | 54 +- .../src/rules/jsdoc/check_tag_names.rs | 938 +++++++++--------- 2 files changed, 498 insertions(+), 494 deletions(-) diff --git a/crates/oxc_linter/src/config/settings/jsdoc.rs b/crates/oxc_linter/src/config/settings/jsdoc.rs index a4682567438e6..c3e5f80ef8e35 100644 --- a/crates/oxc_linter/src/config/settings/jsdoc.rs +++ b/crates/oxc_linter/src/config/settings/jsdoc.rs @@ -73,11 +73,8 @@ pub struct JSDocPluginSettings { impl JSDocPluginSettings { pub fn check_blocked_tag_name(&self, tag_name: &str) -> Option { match self.tag_name_preference.get(tag_name) { - Some(TagNamePreference::FalseOnly(_)) => Some(format!("Unexpected tag `@{tag_name}`")), - Some( - TagNamePreference::ObjectWithMessage { message } - | TagNamePreference::ObjectWithMessageAndReplacement { message, .. }, - ) => Some(message.to_string()), + Some(TagNamePreference::FalseOnly(_)) => Some(format!("Unexpected tag `@{tag_name}`.")), + Some(TagNamePreference::ObjectWithMessage { message }) => Some(message.to_string()), _ => None, } } @@ -103,29 +100,27 @@ impl JSDocPluginSettings { TagNamePreference::TagNameOnly(replacement) | TagNamePreference::ObjectWithMessageAndReplacement { replacement, .. }, ) => replacement.to_string(), - _ => { - // https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#default-preferred-aliases - match original_name { - "virtual" => "abstract", - "extends" => "augments", - "constructor" => "class", - "const" => "constant", - "defaultvalue" => "default", - "desc" => "description", - "host" => "external", - "fileoverview" | "overview" => "file", - "emits" => "fires", - "func" | "method" => "function", - "var" => "member", - "arg" | "argument" => "param", - "prop" => "property", - "return" => "returns", - "exception" => "throws", - "yield" => "yields", - _ => original_name, - } - .to_string() + // https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#default-preferred-aliases + _ => match original_name { + "virtual" => "abstract", + "extends" => "augments", + "constructor" => "class", + "const" => "constant", + "defaultvalue" => "default", + "desc" => "description", + "host" => "external", + "fileoverview" | "overview" => "file", + "emits" => "fires", + "func" | "method" => "function", + "var" => "member", + "arg" | "argument" => "param", + "prop" => "property", + "return" => "returns", + "exception" => "throws", + "yield" => "yields", + _ => original_name, } + .to_string(), } } } @@ -140,6 +135,7 @@ fn default_true() -> bool { #[serde(untagged)] enum TagNamePreference { TagNameOnly(String), + #[allow(dead_code)] // `message` is not used for now ObjectWithMessageAndReplacement { message: String, replacement: String, @@ -242,9 +238,9 @@ mod test { .unwrap(); assert_eq!( settings.check_blocked_tag_name("foo"), - Some("Unexpected tag `@foo`".to_string()) + Some("Unexpected tag `@foo`.".to_string()) ); assert_eq!(settings.check_blocked_tag_name("bar"), Some("do not use bar".to_string())); - assert_eq!(settings.check_blocked_tag_name("baz"), Some("baz is noop now".to_string())); + assert_eq!(settings.check_blocked_tag_name("baz"), None); } } diff --git a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs index 7ae078ed8465b..e630d5786e2e5 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs @@ -1,4 +1,3 @@ -use itertools::Itertools; use oxc_diagnostics::{ miette::{self, Diagnostic}, thiserror::Error, @@ -97,6 +96,9 @@ const VALID_BLOCK_TAGS: phf::Set<&'static str> = phf_set! { "memberof!", "mixes", "mixin", +// Undocumented, but exists +// https://github.com/jsdoc/jsdoc/blob/a08ac18a11f5b0d93421d1e8ecf632468db2d045/packages/jsdoc-tag/lib/definitions/core.js#L374 +"modifies", "module", "name", "namespace", @@ -122,7 +124,20 @@ const VALID_BLOCK_TAGS: phf::Set<&'static str> = phf_set! { "typedef", "variation", "version", -"yields" +"yields", +// TypeScript specific +"import", +"internal", +"overload", +"satisfies", +"template", +}; + +const JSX_TAGS: phf::Set<&'static str> = phf_set! { +"jsx", +"jsxFrag", +"jsxImportSource", +"jsxRuntime", }; impl Rule for CheckTagNames { @@ -137,43 +152,55 @@ impl Rule for CheckTagNames { fn run_once(&self, ctx: &LintContext) { let settings = &ctx.settings().jsdoc; let config = &self.0; - println!("๐Ÿ‘ป {config:?}"); - println!("๐Ÿ‘ป {settings:?}"); let user_preferred_tags = settings.list_preferred_tag_names(); - let is_valid_tag = |tag_name: &str| { - if config.defined_tags.contains(&tag_name.to_string()) - || user_preferred_tags.contains(&tag_name.to_string()) - { - return true; - } - - if VALID_BLOCK_TAGS.contains(&settings.resolve_tag_name(tag_name)) { - return true; - } - false - }; // TODO: typed, d.ts + // TODO: Bundle multiple diagnostics into one? + // TODO: Test for all tags...? for jsdoc in ctx.semantic().jsdoc().iter_all() { for tag in jsdoc.tags() { let tag_name = tag.kind.parsed(); - println!("๐Ÿ‘ป {tag_name}"); - if let Some(message) = settings.is_blocked_tag_name(tag_name) { - ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, message)); + // If explicitly blocked, report + if let Some(reason) = settings.check_blocked_tag_name(tag_name) { + ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, reason)); + continue; + } + + // If valid JSX tags, skip + if config.jsx_tags && JSX_TAGS.contains(tag_name) { continue; } - println!(" => not blocked"); - if !is_valid_tag(tag_name) { - ctx.diagnostic(CheckTagNamesDiagnostic( - tag.kind.span, - format!("`@{tag_name}` is invalid tag name."), - )); + let is_valid = config.defined_tags.contains(&tag_name.to_string()) + || VALID_BLOCK_TAGS.contains(tag_name); + + // If valid + if is_valid { + // But preferred, report to use it + let preferred_name = settings.resolve_tag_name(tag_name); + if preferred_name != tag_name { + ctx.diagnostic(CheckTagNamesDiagnostic( + tag.kind.span, + format!("Replace tag `@{tag_name}` with `@{preferred_name}`."), + )); + continue; + } + + continue; + } + + // If invalid but user preferred, skip + if user_preferred_tags.contains(&tag_name.to_string()) { continue; } - println!(" => is valid"); + + // Otherwise, report + ctx.diagnostic(CheckTagNamesDiagnostic( + tag.kind.span, + format!("`@{tag_name}` is invalid tag name."), + )); } } } @@ -184,75 +211,9 @@ fn test() { use crate::tester::Tester; let pass = vec![ -// (" -// /** @default 0 */ -// let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @default 0 */ -// declare let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @abstract */ -// let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @abstract */ -// declare let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @abstract */ -// { declare let a; } -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// function test() { -// /** @abstract */ -// declare let a; -// } -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @template name */ -// let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @param param - takes information */ -// function takesOne(param) {} -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), (" /** - * @param foo + * @param foo (pass: valid name) */ function quux (foo) { @@ -260,7 +221,7 @@ fn test() { ", None, None), (" /** - * @memberof! foo + * @memberof! foo (pass: valid name) */ function quux (foo) { @@ -268,7 +229,7 @@ fn test() { ", None, None), (" /** - * @arg foo + * @arg foo (pass: invalid name but user preferred) */ function quux (foo) { @@ -282,7 +243,7 @@ fn test() { }))), (" /** - * @bar foo + * @bar foo (pass: invalid name but defined) */ function quux (foo) { @@ -296,7 +257,7 @@ fn test() { ])), None), (" /** - * @baz @bar foo + * @baz @bar foo (pass: invalid names but defined) */ function quux (foo) { @@ -310,7 +271,7 @@ fn test() { ])), None), (" /** - * @baz @bar foo + * @baz @bar foo (pass: invalid names but user preferred) */ function quux (foo) { @@ -329,14 +290,14 @@ fn test() { }))), (" /** - * @returns + * @returns (pass: valid name) */ function quux (foo) {} ", None, None), ("", None, None), (" /** - * + * (pass: no tag) */ function quux (foo) { @@ -344,7 +305,7 @@ fn test() { ", None, None), (" /** - * @todo + * @todo (pass: valid name) */ function quux () { @@ -352,7 +313,7 @@ fn test() { ", None, None), (" /** - * @extends Foo + * @extends Foo (pass: invalid name but user preferred) */ function quux () { @@ -372,7 +333,7 @@ fn test() { * (Set tag name preference to itself to get aliases to * work along with main tag name.) * @augments Bar - * @extends Foo + * @extends Foo (pass: invalid name but user preferred) */ function quux () { } @@ -393,7 +354,7 @@ fn test() { @transient() class Foo { } ``` - * @param Time for a new tag + * @param Time for a new tag (pass: valid names) */ export function transient(target?: T): T { // ... @@ -403,7 +364,7 @@ fn test() { /** @jsx h */ /** @jsxFrag Fragment */ /** @jsxImportSource preact */ - /** @jsxRuntime automatic */ + /** @jsxRuntime automatic (pass: valid jsx names)*/ ", Some(serde_json::json!([ { "jsxTags": true, @@ -411,39 +372,87 @@ fn test() { ])), None), (" /** - * @internal + * @internal (pass: valid name) */ ", None, Some(serde_json::json!({ "jsdoc": { }, }))), -(" - interface WebTwain { - /** - * Converts the images specified by the indices to base64 synchronously. - * @function WebTwain#ConvertToBase64 - * @returns {Base64Result} - - ConvertToBase64(): Base64Result; - */ - - /** - * Converts the images specified by the indices to base64 asynchronously. - * @function WebTwain#ConvertToBase64 - * @returns {boolean} - */ - ConvertToBase64(): boolean; - } - ", None, None), (" /** * @overload - * @satisfies + * @satisfies (pass: valid names) */ ", None, Some(serde_json::json!({ "jsdoc": { }, }))), +// (" +// /** @default 0 */ +// let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @default 0 */ +// declare let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @abstract */ +// let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @abstract */ +// declare let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @abstract */ +// { declare let a; } +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// function test() { +// /** @abstract */ +// declare let a; +// } +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @template name */ +// let a; +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), +// (" +// /** @param param - takes information */ +// function takesOne(param) {} +// ", Some(serde_json::json!([ +// { +// "typed": true, +// }, +// ])), None), // (" // /** // * @module @@ -457,88 +466,9 @@ fn test() { ]; let fail = vec![ - // ( - // "/** @type {string} */let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * Existing comment. - // * @type {string} - // */ - // let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** @abstract */ - // let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // const a = { - // /** @abstract */ - // b: true, - // }; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** @template */ - // let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * Prior description. - // * - // * @template - // */ - // let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), ( " - /** @typoo {string} */ + /** @typoo {string} (fail: invalid name) */ let a; ", None, @@ -547,7 +477,7 @@ fn test() { ( " /** - * @Param + * @Param (fail: invalid name) */ function quux () { @@ -559,7 +489,7 @@ fn test() { ( " /** - * @foo + * @foo (fail: invalid name) */ function quux () { @@ -571,7 +501,7 @@ fn test() { ( " /** - * @arg foo + * @arg foo (fail: invalid name, default aliased) */ function quux (foo) { @@ -580,289 +510,387 @@ fn test() { None, None, ), - // ( - // " - // /** - // * @param foo - // */ - // function quux (foo) { + ( + " + /** + * @param foo (fail: valid name but user preferred) + */ + function quux (foo) { - // } - // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "param": "arg", - // }, - // }, - // })), - // ), - // ( - // " - // /** - // * @constructor foo - // */ - // function quux (foo) { + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "param": "arg", + }, + }, + })), + ), + ( + " + /** + * @constructor foo (fail: invalid name and user preferred) + */ + function quux (foo) { - // } - // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "constructor": "cons", - // }, - // }, - // })), - // ), - // ( - // " - // /** - // * @arg foo - // */ - // function quux (foo) { + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "constructor": "cons", + }, + }, + })), + ), + ( + " + /** + * @arg foo + */ + function quux (foo) { - // } - // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "arg": "somethingDifferent", - // }, - // }, - // })), - // ), - // ( - // " - // /** - // * @param foo - // */ - // function quux (foo) { + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "arg": "somethingDifferent", + }, + }, + })), + ), + ( + " + /** + * @param foo + */ + function quux (foo) { - // } - // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "param": "parameter", - // }, - // }, - // })), - // ), - // ( - // " - // /** - // * @bar foo - // */ - // function quux (foo) { + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "param": "parameter", + }, + }, + })), + ), + ( + " + /** + * @bar foo + */ + function quux (foo) { - // } - // ", - // None, - // None, - // ), - // ( - // " - // /** - // * @baz @bar foo - // */ - // function quux (foo) { + } + ", + None, + None, + ), + ( + " + /** + * @baz @bar foo + */ + function quux (foo) { + + } + ", + Some(serde_json::json!([ + { + "definedTags": [ + "bar", + ], + }, + ])), + None, + ), + ( + " + /** + * @bar + * @baz + */ + function quux (foo) { + + } + ", + Some(serde_json::json!([ + { + "definedTags": [ + "bar", + ], + }, + ])), + None, + ), + ( + " + /** + * @todo + */ + function quux () { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "todo": false, + }, + }, + })), + ), + ( + " + /** + * @todo + */ + function quux () { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "todo": { + "message": "Please resolve to-dos or add to the tracker", + }, + }, + }, + })), + ), + ( + " + /** + * @todo + */ + function quux () { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "todo": { + "message": "Please use x-todo instead of todo", + "replacement": "x-todo", + }, + }, + }, + })), + ), + ( + " + /** + * @todo + */ + function quux () { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "todo": "55", + }, + }, + })), + ), + ( + " + /** + * @property {object} a + * @prop {boolean} b + */ + function quux () { + + } + ", + None, + None, + ), + ( + " + /** + * @abc foo + * @abcd bar + */ + function quux () { + + } + ", + Some(serde_json::json!([ + { + "definedTags": [ + "abcd", + ], + }, + ])), + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "abc": "abcd", + }, + }, + })), + ), + ( + " + /** + * @abc + * @abcd + */ + function quux () { - // } + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "abc": "abcd", + }, + }, + })), + ), + ( + " + /** @jsx h */ + /** @jsxFrag Fragment */ + /** @jsxImportSource preact */ + /** @jsxRuntime automatic */ + ", + None, + None, + ), + ( + " + /** + * @constructor + */ + function Test() { + this.works = false; + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "returns": "return", + }, + }, + })), + ), + ( + " + /** + * @todo + */ + function quux () { + + } + ", + None, + Some(serde_json::json!({ + "jsdoc": { + "tagNamePreference": { + "todo": { + "message": "Please don\"t use todo", + }, + }, + }, + })), + ), + // ( + // "/** @type {string} */let a; // ", // Some(serde_json::json!([ // { - // "definedTags": [ - // "bar", - // ], + // "typed": true, // }, // ])), // None, // ), // ( // " - // /** - // * @bar - // * @baz - // */ - // function quux (foo) { - - // } - // ", + // /** + // * Existing comment. + // * @type {string} + // */ + // let a; + // ", // Some(serde_json::json!([ // { - // "definedTags": [ - // "bar", - // ], + // "typed": true, // }, // ])), // None, // ), // ( // " - // /** - // * @todo - // */ - // function quux () { - - // } - // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "todo": false, - // }, - // }, - // })), - // ), - // ( - // " - // /** - // * @todo - // */ - // function quux () { - - // } + // /** @abstract */ + // let a; // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "todo": { - // "message": "Please resolve to-dos or add to the tracker", - // }, - // }, + // Some(serde_json::json!([ + // { + // "typed": true, // }, - // })), - // ), - // ( - // " - // /** - // * @todo - // */ - // function quux () { - - // } - // ", + // ])), // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "todo": { - // "message": "Please use x-todo instead of todo", - // "replacement": "x-todo", - // }, - // }, - // }, - // })), // ), // ( // " - // /** - // * @todo - // */ - // function quux () { - - // } + // const a = { + // /** @abstract */ + // b: true, + // }; // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "todo": 55, - // }, + // Some(serde_json::json!([ + // { + // "typed": true, // }, - // })), - // ), - // ( - // " - // /** - // * @property {object} a - // * @prop {boolean} b - // */ - // function quux () { - - // } - // ", - // None, + // ])), // None, // ), // ( // " - // /** - // * @abc foo - // * @abcd bar - // */ - // function quux () { - - // } + // /** @template */ + // let a; // ", // Some(serde_json::json!([ // { - // "definedTags": [ - // "abcd", - // ], + // "typed": true, // }, // ])), - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "abc": "abcd", - // }, - // }, - // })), - // ), - // ( - // " - // /** - // * @abc - // * @abcd - // */ - // function quux () { - - // } - // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "abc": "abcd", - // }, - // }, - // })), - // ), - ("", None, None), - // ( - // " - // /** @jsx h */ - // /** @jsxFrag Fragment */ - // /** @jsxImportSource preact */ - // /** @jsxRuntime automatic */ - // ", - // None, // None, // ), // ( // " - // /** - // * @constructor - // */ - // function Test() { - // this.works = false; - // } + // /** + // * Prior description. + // * + // * @template + // */ + // let a; // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "returns": "return", - // }, + // Some(serde_json::json!([ + // { + // "typed": true, // }, - // })), + // ])), + // None, // ), // ( // " @@ -913,26 +941,6 @@ fn test() { // ])), // None, // ), - // ( - // " - // /** - // * @todo - // */ - // function quux () { - - // } - // ", - // None, - // Some(serde_json::json!({ - // "jsdoc": { - // "tagNamePreference": { - // "todo": { - // "message": "Please don\"t use todo", - // }, - // }, - // }, - // })), - // ), ]; Tester::new(CheckTagNames::NAME, pass, fail).test_and_snapshot(); From b5cad445607e58d1abc5e7e4c739ed352f888ee8 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Fri, 19 Apr 2024 21:50:48 +0900 Subject: [PATCH 3/9] Refactor --- .../oxc_linter/src/config/settings/jsdoc.rs | 103 +++++++++++++----- .../src/rules/jsdoc/check_tag_names.rs | 28 ++--- 2 files changed, 86 insertions(+), 45 deletions(-) diff --git a/crates/oxc_linter/src/config/settings/jsdoc.rs b/crates/oxc_linter/src/config/settings/jsdoc.rs index c3e5f80ef8e35..ad5f678012a6c 100644 --- a/crates/oxc_linter/src/config/settings/jsdoc.rs +++ b/crates/oxc_linter/src/config/settings/jsdoc.rs @@ -70,7 +70,38 @@ pub struct JSDocPluginSettings { // }[] } +// https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#default-preferred-aliases +fn get_default_aliased_tag_name(original_name: &str) -> Option { + let aliased_name = match original_name { + "virtual" => "abstract", + "extends" => "augments", + "constructor" => "class", + "const" => "constant", + "defaultvalue" => "default", + "desc" => "description", + "host" => "external", + "fileoverview" | "overview" => "file", + "emits" => "fires", + "func" | "method" => "function", + "var" => "member", + "arg" | "argument" => "param", + "prop" => "property", + "return" => "returns", + "exception" => "throws", + "yield" => "yields", + _ => original_name, + } + .to_string(); + + if aliased_name != original_name { + return Some(aliased_name); + } + None +} + impl JSDocPluginSettings { + /// Only for `check-tag-names` rule + /// Return `Some(reason)` if blocked pub fn check_blocked_tag_name(&self, tag_name: &str) -> Option { match self.tag_name_preference.get(tag_name) { Some(TagNamePreference::FalseOnly(_)) => Some(format!("Unexpected tag `@{tag_name}`.")), @@ -78,8 +109,26 @@ impl JSDocPluginSettings { _ => None, } } + /// Only for `check-tag-names` rule + /// Return `Some(reason)` if replacement found, include default alias + pub fn check_preferred_tag_name(&self, original_name: &str) -> Option { + let reason = |preferred_name: &str| -> String { + format!("Replace tag `@{original_name}` with `@{preferred_name}`.") + }; - pub fn list_preferred_tag_names(&self) -> Vec { + match self.tag_name_preference.get(original_name) { + Some(TagNamePreference::TagNameOnly(preferred_name)) => Some(reason(preferred_name)), + Some(TagNamePreference::ObjectWithMessageAndReplacement { message, .. }) => { + Some(message.to_string()) + } + _ => get_default_aliased_tag_name(original_name) + .filter(|preferred_name| preferred_name != original_name) + .map(|preferred_name| reason(&preferred_name)), + } + } + /// Only for `check-tag-names` rule + /// Return all user replacement tag names + pub fn list_user_defined_tag_names(&self) -> Vec { self.tag_name_preference .iter() .filter_map(|(_, pref)| match pref { @@ -100,27 +149,7 @@ impl JSDocPluginSettings { TagNamePreference::TagNameOnly(replacement) | TagNamePreference::ObjectWithMessageAndReplacement { replacement, .. }, ) => replacement.to_string(), - // https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#default-preferred-aliases - _ => match original_name { - "virtual" => "abstract", - "extends" => "augments", - "constructor" => "class", - "const" => "constant", - "defaultvalue" => "default", - "desc" => "description", - "host" => "external", - "fileoverview" | "overview" => "file", - "emits" => "fires", - "func" | "method" => "function", - "var" => "member", - "arg" | "argument" => "param", - "prop" => "property", - "return" => "returns", - "exception" => "throws", - "yield" => "yields", - _ => original_name, - } - .to_string(), + _ => get_default_aliased_tag_name(original_name).unwrap_or(original_name.to_string()), } } } @@ -135,7 +164,6 @@ fn default_true() -> bool { #[serde(untagged)] enum TagNamePreference { TagNameOnly(String), - #[allow(dead_code)] // `message` is not used for now ObjectWithMessageAndReplacement { message: String, replacement: String, @@ -204,9 +232,9 @@ mod test { } #[test] - fn list_preferred_tag_names() { + fn list_user_defined_tag_names() { let settings = JSDocPluginSettings::deserialize(&serde_json::json!({})).unwrap(); - assert_eq!(settings.list_preferred_tag_names().len(), 0); + assert_eq!(settings.list_user_defined_tag_names().len(), 0); let settings = JSDocPluginSettings::deserialize(&serde_json::json!({ "tagNamePreference": { @@ -218,7 +246,7 @@ mod test { } })) .unwrap(); - let mut preferred = settings.list_preferred_tag_names(); + let mut preferred = settings.list_user_defined_tag_names(); preferred.sort_unstable(); assert_eq!(preferred, vec!["bar", "noop", "overridedefault"]); } @@ -243,4 +271,27 @@ mod test { assert_eq!(settings.check_blocked_tag_name("bar"), Some("do not use bar".to_string())); assert_eq!(settings.check_blocked_tag_name("baz"), None); } + + #[test] + fn check_preferred_tag_name() { + let settings = JSDocPluginSettings::deserialize(&serde_json::json!({})).unwrap(); + assert_eq!(settings.check_preferred_tag_name("foo"), None); + + let settings = JSDocPluginSettings::deserialize(&serde_json::json!({ + "tagNamePreference": { + "foo": false, + "bar": { "message": "do not use bar" }, + "baz": { "message": "baz is noop now", "replacement": "noop" }, + "qux": "quux" + } + })) + .unwrap(); + assert_eq!(settings.check_preferred_tag_name("foo"), None,); + assert_eq!(settings.check_preferred_tag_name("bar"), None); + assert_eq!(settings.check_preferred_tag_name("baz"), Some("baz is noop now".to_string())); + assert_eq!( + settings.check_preferred_tag_name("qux"), + Some("Replace tag `@qux` with `@quux`.".to_string()) + ); + } } diff --git a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs index e630d5786e2e5..ba191ca1a75f6 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs @@ -153,7 +153,7 @@ impl Rule for CheckTagNames { let settings = &ctx.settings().jsdoc; let config = &self.0; - let user_preferred_tags = settings.list_preferred_tag_names(); + let user_preferred_tags = settings.list_user_defined_tag_names(); // TODO: typed, d.ts // TODO: Bundle multiple diagnostics into one? @@ -168,34 +168,24 @@ impl Rule for CheckTagNames { continue; } - // If valid JSX tags, skip - if config.jsx_tags && JSX_TAGS.contains(tag_name) { + // If user preferred, skip + if user_preferred_tags.contains(&tag_name.to_string()) { continue; } - let is_valid = config.defined_tags.contains(&tag_name.to_string()) - || VALID_BLOCK_TAGS.contains(tag_name); - // If valid - if is_valid { + if config.jsx_tags && JSX_TAGS.contains(tag_name) + || config.defined_tags.contains(&tag_name.to_string()) + || VALID_BLOCK_TAGS.contains(tag_name) + { // But preferred, report to use it - let preferred_name = settings.resolve_tag_name(tag_name); - if preferred_name != tag_name { - ctx.diagnostic(CheckTagNamesDiagnostic( - tag.kind.span, - format!("Replace tag `@{tag_name}` with `@{preferred_name}`."), - )); - continue; + if let Some(reason) = settings.check_preferred_tag_name(tag_name) { + ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, reason)); } continue; } - // If invalid but user preferred, skip - if user_preferred_tags.contains(&tag_name.to_string()) { - continue; - } - // Otherwise, report ctx.diagnostic(CheckTagNamesDiagnostic( tag.kind.span, From 8cbc34e11f5c729ae3774645bb1e9271c4bc47bc Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Mon, 22 Apr 2024 17:38:30 +0900 Subject: [PATCH 4/9] Implement typed mode --- .../src/rules/jsdoc/check_tag_names.rs | 624 +++++++++++------- 1 file changed, 369 insertions(+), 255 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs index ba191ca1a75f6..6aadbe98c82d7 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs @@ -125,7 +125,7 @@ const VALID_BLOCK_TAGS: phf::Set<&'static str> = phf_set! { "variation", "version", "yields", -// TypeScript specific +// TypeScript specific -- "import", "internal", "overload", @@ -140,6 +140,54 @@ const JSX_TAGS: phf::Set<&'static str> = phf_set! { "jsxRuntime", }; +const ALWAYS_INVALID_TAGS_IF_TYPED: phf::Set<&'static str> = phf_set! { +"augments", +"callback", +"class", +"enum", +"implements", +"private", +"property", +"protected", +"public", +"readonly", +"this", +"type", +"typedef", +}; +const OUTSIDE_AMBIENT_INVALID_TAGS_IF_TYPED: phf::Set<&'static str> = phf_set! { +"abstract", +"access", +"class", +"constant", +"constructs", +// I'm not sure but this seems to be allowed... +// https://github.com/gajus/eslint-plugin-jsdoc/blob/e343ab5b1efaa59b07c600138aee070b4083857e/src/rules/checkTagNames.js#L140 +// "default", +"enum", +"export", +"exports", +"function", +"global", +"inherits", +"instance", +"interface", +"member", +"memberof", +"memberOf", +"method", +"mixes", +"mixin", +"module", +"name", +"namespace", +"override", +"property", +"requires", +"static", +"this", +}; + impl Rule for CheckTagNames { fn from_configuration(value: serde_json::Value) -> Self { value @@ -153,11 +201,19 @@ impl Rule for CheckTagNames { let settings = &ctx.settings().jsdoc; let config = &self.0; - let user_preferred_tags = settings.list_user_defined_tag_names(); + let user_defined_tags = settings.list_user_defined_tag_names(); + + let is_dts = ctx.file_path().to_str().map_or(false, |p| p.ends_with(".d.ts")); + // NOTE: Original rule seems to check `declare` context with visiting AST nodes. + // https://github.com/gajus/eslint-plugin-jsdoc/blob/e343ab5b1efaa59b07c600138aee070b4083857e/src/rules/checkTagNames.js#L121 + // But... + // - No test case covers this(= only checks inside of `.d.ts`) + // - I never seen this usage before + // So, I leave this part out for now. + let is_declare = false; + let is_ambient = is_dts || is_declare; - // TODO: typed, d.ts // TODO: Bundle multiple diagnostics into one? - // TODO: Test for all tags...? for jsdoc in ctx.semantic().jsdoc().iter_all() { for tag in jsdoc.tags() { let tag_name = tag.kind.parsed(); @@ -168,29 +224,56 @@ impl Rule for CheckTagNames { continue; } - // If user preferred, skip - if user_preferred_tags.contains(&tag_name.to_string()) { + // If user defined, skip + if user_defined_tags.contains(&tag_name.to_string()) { continue; } - // If valid - if config.jsx_tags && JSX_TAGS.contains(tag_name) + let is_valid = config.jsx_tags && JSX_TAGS.contains(tag_name) || config.defined_tags.contains(&tag_name.to_string()) - || VALID_BLOCK_TAGS.contains(tag_name) - { - // But preferred, report to use it - if let Some(reason) = settings.check_preferred_tag_name(tag_name) { - ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, reason)); - } + || VALID_BLOCK_TAGS.contains(tag_name); + + // If invalid or unknown, report + if !is_valid { + ctx.diagnostic(CheckTagNamesDiagnostic( + tag.kind.span, + format!("`@{tag_name}` is invalid tag name."), + )); + continue; + } + // If valid but preferred, report to use it + if let Some(reason) = settings.check_preferred_tag_name(tag_name) { + ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, reason)); continue; } - // Otherwise, report - ctx.diagnostic(CheckTagNamesDiagnostic( - tag.kind.span, - format!("`@{tag_name}` is invalid tag name."), - )); + // Additional check for `typed` mode + if config.typed { + if ALWAYS_INVALID_TAGS_IF_TYPED.contains(tag_name) { + ctx.diagnostic(CheckTagNamesDiagnostic( + tag.kind.span, + format!("`@{tag_name}` is redundant when using a type system."), + )); + continue; + } + + if tag.kind.parsed() == "template" && tag.comment().parsed().is_empty() { + ctx.diagnostic(CheckTagNamesDiagnostic( + tag.kind.span, + format!("`@{tag_name}` without a name is redundant when using a type system."), + )); + continue; + } + + if !is_ambient && OUTSIDE_AMBIENT_INVALID_TAGS_IF_TYPED.contains(tag_name) { + ctx.diagnostic(CheckTagNamesDiagnostic( + tag.kind.span, + format!("`@{tag_name}` is redundant outside of ambient(`declare` or `.d.ts`) contexts when using a type system."), + )); + continue; + } + } } } } @@ -377,82 +460,41 @@ fn test() { "jsdoc": { }, }))), -// (" -// /** @default 0 */ -// let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @default 0 */ -// declare let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @abstract */ -// let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @abstract */ -// declare let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @abstract */ -// { declare let a; } -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// function test() { -// /** @abstract */ -// declare let a; -// } -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @template name */ -// let a; -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** @param param - takes information */ -// function takesOne(param) {} -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None), -// (" -// /** -// * @module -// * A comment related to the module -// */ -// ", Some(serde_json::json!([ -// { -// "typed": true, -// }, -// ])), None) + ( + " + /** + * @module + * A comment related to the module + */ + ", + None, + None, + ), + // Typed + (" + /** @default 0 */ + let a; + ", Some(serde_json::json!([ + { + "typed": true, + }, + ])), None), +(" + /** @template name */ + let a; + ", Some(serde_json::json!([ + { + "typed": true, + }, + ])), None), +(" + /** @param param - takes information */ + function takesOne(param) {} + ", Some(serde_json::json!([ + { + "typed": true, + }, + ])), None), ]; let fail = vec![ @@ -539,7 +581,7 @@ fn test() { ( " /** - * @arg foo + * @arg foo (fail: invalid name and user preferred) */ function quux (foo) { @@ -557,7 +599,7 @@ fn test() { ( " /** - * @param foo + * @param foo (fail: valid name but user preferred) */ function quux (foo) { @@ -575,7 +617,7 @@ fn test() { ( " /** - * @bar foo + * @bar foo (fail: invalid name) */ function quux (foo) { @@ -587,7 +629,7 @@ fn test() { ( " /** - * @baz @bar foo + * @baz @bar foo (fail: invalid name) */ function quux (foo) { @@ -606,7 +648,7 @@ fn test() { " /** * @bar - * @baz + * @baz (fail: invalid name) */ function quux (foo) { @@ -624,7 +666,7 @@ fn test() { ( " /** - * @todo + * @todo (fail: valid name but blocked) */ function quux () { @@ -642,7 +684,7 @@ fn test() { ( " /** - * @todo + * @todo (fail: valid name but blocked) */ function quux () { @@ -662,7 +704,7 @@ fn test() { ( " /** - * @todo + * @todo (fail: valid name but blocked) */ function quux () { @@ -680,29 +722,11 @@ fn test() { }, })), ), - ( - " - /** - * @todo - */ - function quux () { - - } - ", - None, - Some(serde_json::json!({ - "jsdoc": { - "tagNamePreference": { - "todo": "55", - }, - }, - })), - ), ( " /** * @property {object} a - * @prop {boolean} b + * @prop {boolean} b (fail: invalid name, default aliased) */ function quux () { @@ -714,7 +738,7 @@ fn test() { ( " /** - * @abc foo + * @abc foo (fail: invalid name and user preferred) * @abcd bar */ function quux () { @@ -739,7 +763,7 @@ fn test() { ( " /** - * @abc + * @abc (fail: invalid name and user preferred) * @abcd */ function quux () { @@ -768,7 +792,7 @@ fn test() { ( " /** - * @constructor + * @constructor (fail: invalid name) */ function Test() { this.works = false; @@ -786,7 +810,7 @@ fn test() { ( " /** - * @todo + * @todo (fail: valid name but blocked) */ function quux () { @@ -797,141 +821,231 @@ fn test() { "jsdoc": { "tagNamePreference": { "todo": { - "message": "Please don\"t use todo", + "message": "Please don't use todo", }, }, }, })), ), - // ( - // "/** @type {string} */let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * Existing comment. - // * @type {string} - // */ - // let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** @abstract */ - // let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // const a = { - // /** @abstract */ - // b: true, - // }; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** @template */ - // let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * Prior description. - // * - // * @template - // */ - // let a; - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** @typedef {Object} MyObject - // * @property {string} id - my id - // */ - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** - // * @property {string} id - my id - // */ - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** @typedef {Object} MyObject */ - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), - // ( - // " - // /** @typedef {Object} MyObject - // */ - // ", - // Some(serde_json::json!([ - // { - // "typed": true, - // }, - // ])), - // None, - // ), + // Typed + ( + " + /** + * @module + * A comment related to the module + */ + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + "/** @type {string} */let a; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** + * Existing comment. + * @type {string} + */ + let a; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** @typedef {Object} MyObject + * @property {string} id - my id + */ + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** + * @property {string} id - my id + */ + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** @typedef {Object} MyObject */ + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** @typedef {Object} MyObject + */ + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** @abstract */ + let a; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + const a = { + /** @abstract */ + b: true, + }; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** @template */ + let a; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** + * Prior description. + * + * @template + */ + let a; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), ]; + let dts_pass = vec![ + ( + " + /** @default 0 */ + declare let a; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** @abstract */ + let a; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** @abstract */ + declare let a; + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + /** @abstract */ + { declare let a; } + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ( + " + function test() { + /** @abstract */ + declare let a; + } + ", + Some(serde_json::json!([ + { + "typed": true, + }, + ])), + None, + ), + ]; + let dts_fail = vec![( + " + /** @typoo {string} (fail: invalid name) */ + let a; + ", + None, + None, + )]; + + // Currently only 1 snapshot can be saved under a rule name + Tester::new(CheckTagNames::NAME, dts_pass, dts_fail).change_rule_path("test.d.ts").test(); Tester::new(CheckTagNames::NAME, pass, fail).test_and_snapshot(); } From 2b7f91bb5b5587a23603d270eb1970df91240e67 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Mon, 22 Apr 2024 17:38:40 +0900 Subject: [PATCH 5/9] Update snapshot --- .../src/snapshots/check_tag_names.snap | 317 ++++++++++++++++++ 1 file changed, 317 insertions(+) create mode 100644 crates/oxc_linter/src/snapshots/check_tag_names.snap diff --git a/crates/oxc_linter/src/snapshots/check_tag_names.snap b/crates/oxc_linter/src/snapshots/check_tag_names.snap new file mode 100644 index 0000000000000..83b436fd0e40f --- /dev/null +++ b/crates/oxc_linter/src/snapshots/check_tag_names.snap @@ -0,0 +1,317 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: check_tag_names +--- + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:2:24] + 1 โ”‚ + 2 โ”‚ /** @typoo {string} (fail: invalid name) */ + ยท โ”€โ”€โ”€โ”€โ”€โ”€ + 3 โ”‚ let a; + โ•ฐโ”€โ”€โ”€โ”€ + help: `@typoo` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @Param (fail: invalid name) + ยท โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@Param` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @foo (fail: invalid name) + ยท โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@foo` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @arg foo (fail: invalid name, default aliased) + ยท โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@arg` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @param foo (fail: valid name but user preferred) + ยท โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: Replace tag `@param` with `@arg`. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @constructor foo (fail: invalid name and user preferred) + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@constructor` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:34] + 2 โ”‚ /** + 3 โ”‚ * @arg foo (fail: invalid name and user preferred) + ยท โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@arg` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @param foo (fail: valid name but user preferred) + ยท โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: Replace tag `@param` with `@parameter`. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @bar foo (fail: invalid name) + ยท โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@bar` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @baz @bar foo (fail: invalid name) + ยท โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@baz` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:4:27] + 3 โ”‚ * @bar + 4 โ”‚ * @baz (fail: invalid name) + ยท โ”€โ”€โ”€โ”€ + 5 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@baz` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @todo (fail: valid name but blocked) + ยท โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: Unexpected tag `@todo`. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @todo (fail: valid name but blocked) + ยท โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: Please resolve to-dos or add to the tracker + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @todo (fail: valid name but blocked) + ยท โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: Please use x-todo instead of todo + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:4:25] + 3 โ”‚ * @property {object} a + 4 โ”‚ * @prop {boolean} b (fail: invalid name, default aliased) + ยท โ”€โ”€โ”€โ”€โ”€ + 5 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@prop` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @abc foo (fail: invalid name and user preferred) + ยท โ”€โ”€โ”€โ”€ + 4 โ”‚ * @abcd bar + โ•ฐโ”€โ”€โ”€โ”€ + help: `@abc` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:34] + 2 โ”‚ /** + 3 โ”‚ * @abc (fail: invalid name and user preferred) + ยท โ”€โ”€โ”€โ”€ + 4 โ”‚ * @abcd + โ•ฐโ”€โ”€โ”€โ”€ + help: `@abc` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:2:24] + 1 โ”‚ + 2 โ”‚ /** @jsx h */ + ยท โ”€โ”€โ”€โ”€ + 3 โ”‚ /** @jsxFrag Fragment */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@jsx` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:24] + 2 โ”‚ /** @jsx h */ + 3 โ”‚ /** @jsxFrag Fragment */ + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ /** @jsxImportSource preact */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@jsxFrag` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:4:24] + 3 โ”‚ /** @jsxFrag Fragment */ + 4 โ”‚ /** @jsxImportSource preact */ + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 5 โ”‚ /** @jsxRuntime automatic */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@jsxImportSource` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:5:24] + 4 โ”‚ /** @jsxImportSource preact */ + 5 โ”‚ /** @jsxRuntime automatic */ + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 6 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@jsxRuntime` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:21] + 2 โ”‚ /** + 3 โ”‚ * @constructor (fail: invalid name) + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@constructor` is invalid tag name. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:25] + 2 โ”‚ /** + 3 โ”‚ * @todo (fail: valid name but blocked) + ยท โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: Please don't use todo + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:15] + 2 โ”‚ /** + 3 โ”‚ * @module + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ * A comment related to the module + โ•ฐโ”€โ”€โ”€โ”€ + help: `@module` is redundant outside of ambient(`declare` or `.d.ts`) contexts when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:1:5] + 1 โ”‚ /** @type {string} */let a; + ยท โ”€โ”€โ”€โ”€โ”€ + 2 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@type` is redundant when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:4:24] + 3 โ”‚ * Existing comment. + 4 โ”‚ * @type {string} + ยท โ”€โ”€โ”€โ”€โ”€ + 5 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@type` is redundant when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:2:22] + 1 โ”‚ + 2 โ”‚ /** @typedef {Object} MyObject + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 3 โ”‚ * @property {string} id - my id + โ•ฐโ”€โ”€โ”€โ”€ + help: `@typedef` is redundant when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:21] + 2 โ”‚ /** @typedef {Object} MyObject + 3 โ”‚ * @property {string} id - my id + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@property` is redundant when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:21] + 2 โ”‚ /** + 3 โ”‚ * @property {string} id - my id + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@property` is redundant when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:2:22] + 1 โ”‚ + 2 โ”‚ /** @typedef {Object} MyObject */ + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 3 โ”‚ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@typedef` is redundant when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:2:22] + 1 โ”‚ + 2 โ”‚ /** @typedef {Object} MyObject + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 3 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@typedef` is redundant when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:2:24] + 1 โ”‚ + 2 โ”‚ /** @abstract */ + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 3 โ”‚ let a; + โ•ฐโ”€โ”€โ”€โ”€ + help: `@abstract` is redundant outside of ambient(`declare` or `.d.ts`) contexts when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:3:26] + 2 โ”‚ const a = { + 3 โ”‚ /** @abstract */ + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 4 โ”‚ b: true, + โ•ฐโ”€โ”€โ”€โ”€ + help: `@abstract` is redundant outside of ambient(`declare` or `.d.ts`) contexts when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:2:24] + 1 โ”‚ + 2 โ”‚ /** @template */ + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 3 โ”‚ let a; + โ•ฐโ”€โ”€โ”€โ”€ + help: `@template` without a name is redundant when using a type system. + + โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. + โ•ญโ”€[check_tag_names.tsx:5:23] + 4 โ”‚ * + 5 โ”‚ * @template + ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + 6 โ”‚ */ + โ•ฐโ”€โ”€โ”€โ”€ + help: `@template` without a name is redundant when using a type system. From 779fa6960ace629f13abe8543919683afbdb6187 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Mon, 22 Apr 2024 18:09:16 +0900 Subject: [PATCH 6/9] Make test pass --- .../src/rules/jsdoc/check_tag_names.rs | 130 +++++++++--------- .../src/snapshots/check_tag_names.snap | 14 +- 2 files changed, 71 insertions(+), 73 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs index 6aadbe98c82d7..ee639cf19b384 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs @@ -218,31 +218,20 @@ impl Rule for CheckTagNames { for tag in jsdoc.tags() { let tag_name = tag.kind.parsed(); - // If explicitly blocked, report - if let Some(reason) = settings.check_blocked_tag_name(tag_name) { - ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, reason)); - continue; - } - - // If user defined, skip - if user_defined_tags.contains(&tag_name.to_string()) { + // If user explicitly allowed, skip + if user_defined_tags.contains(&tag_name.to_string()) + || config.defined_tags.contains(&tag_name.to_string()) + { continue; } - let is_valid = config.jsx_tags && JSX_TAGS.contains(tag_name) - || config.defined_tags.contains(&tag_name.to_string()) - || VALID_BLOCK_TAGS.contains(tag_name); - - // If invalid or unknown, report - if !is_valid { - ctx.diagnostic(CheckTagNamesDiagnostic( - tag.kind.span, - format!("`@{tag_name}` is invalid tag name."), - )); + // If user explicitly blocked, report + if let Some(reason) = settings.check_blocked_tag_name(tag_name) { + ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, reason)); continue; } - // If valid but preferred, report to use it + // If preferred or default aliased, report to use it if let Some(reason) = settings.check_preferred_tag_name(tag_name) { ctx.diagnostic(CheckTagNamesDiagnostic(tag.kind.span, reason)); continue; @@ -274,6 +263,17 @@ impl Rule for CheckTagNames { continue; } } + + // If invalid or unknown, report + let is_valid = (config.jsx_tags && JSX_TAGS.contains(tag_name)) + || VALID_BLOCK_TAGS.contains(tag_name); + if !is_valid { + ctx.diagnostic(CheckTagNamesDiagnostic( + tag.kind.span, + format!("`@{tag_name}` is invalid tag name."), + )); + continue; + } } } } @@ -300,20 +300,6 @@ fn test() { } ", None, None), -(" - /** - * @arg foo (pass: invalid name but user preferred) - */ - function quux (foo) { - - } - ", None, Some(serde_json::json!({ - "jsdoc": { - "tagNamePreference": { - "param": "arg", - }, - }, - }))), (" /** * @bar foo (pass: invalid name but defined) @@ -350,7 +336,7 @@ fn test() { } ", None, Some(serde_json::json!({ - "jsdoc": { + "settings": { "jsdoc": { "tagNamePreference": { "param": "baz", "returns": { @@ -359,7 +345,21 @@ fn test() { }, "todo": false, }, - }, + }}, + }))), +(" + /** + * @arg foo (pass: invalid name but user preferred) + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ + "settings" : { "jsdoc": { + "tagNamePreference": { + "param": "arg", + }, + }}, }))), (" /** @@ -392,14 +392,14 @@ fn test() { } ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "augments": { "message": "@extends is to be used over @augments.", "replacement": "extends", }, }, - }, + }}, }))), (" /** @@ -411,11 +411,11 @@ fn test() { function quux () { } ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "extends": "extends", }, - }, + }}, }))), (" /** @@ -448,8 +448,7 @@ fn test() { * @internal (pass: valid name) */ ", None, Some(serde_json::json!({ - "jsdoc": { - }, + "settings" : { "jsdoc": { }}, }))), (" /** @@ -457,8 +456,7 @@ fn test() { * @satisfies (pass: valid names) */ ", None, Some(serde_json::json!({ - "jsdoc": { - }, + "settings" : { "jsdoc": { }}, }))), ( " @@ -553,11 +551,11 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "param": "arg", }, - }, + }}, })), ), ( @@ -571,11 +569,11 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "constructor": "cons", }, - }, + }}, })), ), ( @@ -589,11 +587,11 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "arg": "somethingDifferent", }, - }, + }}, })), ), ( @@ -607,11 +605,11 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "param": "parameter", }, - }, + }}, })), ), ( @@ -674,11 +672,11 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "todo": false, }, - }, + }}, })), ), ( @@ -692,13 +690,13 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "todo": { "message": "Please resolve to-dos or add to the tracker", }, }, - }, + }}, })), ), ( @@ -712,14 +710,14 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "todo": { "message": "Please use x-todo instead of todo", "replacement": "x-todo", }, }, - }, + }}, })), ), ( @@ -753,11 +751,11 @@ fn test() { }, ])), Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "abc": "abcd", }, - }, + }}, })), ), ( @@ -772,11 +770,11 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "abc": "abcd", }, - }, + }}, })), ), ( @@ -800,11 +798,11 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "returns": "return", }, - }, + }}, })), ), ( @@ -818,13 +816,13 @@ fn test() { ", None, Some(serde_json::json!({ - "jsdoc": { + "settings" : { "jsdoc": { "tagNamePreference": { "todo": { "message": "Please don't use todo", }, }, - }, + }}, })), ), // Typed @@ -1045,7 +1043,7 @@ fn test() { None, )]; + Tester::new(CheckTagNames::NAME, pass, fail).test_and_snapshot(); // Currently only 1 snapshot can be saved under a rule name Tester::new(CheckTagNames::NAME, dts_pass, dts_fail).change_rule_path("test.d.ts").test(); - Tester::new(CheckTagNames::NAME, pass, fail).test_and_snapshot(); } diff --git a/crates/oxc_linter/src/snapshots/check_tag_names.snap b/crates/oxc_linter/src/snapshots/check_tag_names.snap index 83b436fd0e40f..05db33a0518f2 100644 --- a/crates/oxc_linter/src/snapshots/check_tag_names.snap +++ b/crates/oxc_linter/src/snapshots/check_tag_names.snap @@ -36,7 +36,7 @@ expression: check_tag_names ยท โ”€โ”€โ”€โ”€ 4 โ”‚ */ โ•ฐโ”€โ”€โ”€โ”€ - help: `@arg` is invalid tag name. + help: Replace tag `@arg` with `@param`. โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. โ•ญโ”€[check_tag_names.tsx:3:25] @@ -54,7 +54,7 @@ expression: check_tag_names ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ 4 โ”‚ */ โ•ฐโ”€โ”€โ”€โ”€ - help: `@constructor` is invalid tag name. + help: Replace tag `@constructor` with `@cons`. โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. โ•ญโ”€[check_tag_names.tsx:3:34] @@ -63,7 +63,7 @@ expression: check_tag_names ยท โ”€โ”€โ”€โ”€ 4 โ”‚ */ โ•ฐโ”€โ”€โ”€โ”€ - help: `@arg` is invalid tag name. + help: Replace tag `@arg` with `@somethingDifferent`. โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. โ•ญโ”€[check_tag_names.tsx:3:25] @@ -135,7 +135,7 @@ expression: check_tag_names ยท โ”€โ”€โ”€โ”€โ”€ 5 โ”‚ */ โ•ฐโ”€โ”€โ”€โ”€ - help: `@prop` is invalid tag name. + help: Replace tag `@prop` with `@property`. โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. โ•ญโ”€[check_tag_names.tsx:3:25] @@ -144,7 +144,7 @@ expression: check_tag_names ยท โ”€โ”€โ”€โ”€ 4 โ”‚ * @abcd bar โ•ฐโ”€โ”€โ”€โ”€ - help: `@abc` is invalid tag name. + help: Replace tag `@abc` with `@abcd`. โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. โ•ญโ”€[check_tag_names.tsx:3:34] @@ -153,7 +153,7 @@ expression: check_tag_names ยท โ”€โ”€โ”€โ”€ 4 โ”‚ * @abcd โ•ฐโ”€โ”€โ”€โ”€ - help: `@abc` is invalid tag name. + help: Replace tag `@abc` with `@abcd`. โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. โ•ญโ”€[check_tag_names.tsx:2:24] @@ -198,7 +198,7 @@ expression: check_tag_names ยท โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ 4 โ”‚ */ โ•ฐโ”€โ”€โ”€โ”€ - help: `@constructor` is invalid tag name. + help: Replace tag `@constructor` with `@class`. โš  eslint-plugin-jsdoc(check-tag-names): Invalid tag name found. โ•ญโ”€[check_tag_names.tsx:3:25] From 8fa622aa51572a1c5a58e683bbbb6c55a8c1ebab Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Mon, 22 Apr 2024 20:11:30 +0900 Subject: [PATCH 7/9] Fix up comments --- .../src/rules/jsdoc/check_tag_names.rs | 254 +++++++++--------- 1 file changed, 126 insertions(+), 128 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs index ee639cf19b384..4455cf6053603 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs @@ -54,138 +54,138 @@ struct CheckTagnamesConfig { } const VALID_BLOCK_TAGS: phf::Set<&'static str> = phf_set! { -"abstract", -"access", -"alias", -"async", -"augments", -"author", -"borrows", -"callback", -"class", -"classdesc", -"constant", -"constructs", -"copyright", -"default", -"deprecated", -"description", -"enum", -"event", -"example", -"exports", -"external", -"file", -"fires", -"function", -"generator", -"global", -"hideconstructor", -"ignore", -"implements", -"inheritdoc", -"inner", -"instance", -"interface", -"kind", -"lends", -"license", -"listens", -"member", -"memberof", -"memberof!", -"mixes", -"mixin", -// Undocumented, but exists -// https://github.com/jsdoc/jsdoc/blob/a08ac18a11f5b0d93421d1e8ecf632468db2d045/packages/jsdoc-tag/lib/definitions/core.js#L374 -"modifies", -"module", -"name", -"namespace", -"override", -"package", -"param", -"private", -"property", -"protected", -"public", -"readonly", -"requires", -"returns", -"see", -"since", -"static", -"summary", -"this", -"throws", -"todo", -"tutorial", -"type", -"typedef", -"variation", -"version", -"yields", -// TypeScript specific -- -"import", -"internal", -"overload", -"satisfies", -"template", + "abstract", + "access", + "alias", + "async", + "augments", + "author", + "borrows", + "callback", + "class", + "classdesc", + "constant", + "constructs", + "copyright", + "default", + "deprecated", + "description", + "enum", + "event", + "example", + "exports", + "external", + "file", + "fires", + "function", + "generator", + "global", + "hideconstructor", + "ignore", + "implements", + "inheritdoc", + "inner", + "instance", + "interface", + "kind", + "lends", + "license", + "listens", + "member", + "memberof", + "memberof!", + "mixes", + "mixin", + // Undocumented, but exists + // https://github.com/jsdoc/jsdoc/blob/a08ac18a11f5b0d93421d1e8ecf632468db2d045/packages/jsdoc-tag/lib/definitions/core.js#L374 + "modifies", + "module", + "name", + "namespace", + "override", + "package", + "param", + "private", + "property", + "protected", + "public", + "readonly", + "requires", + "returns", + "see", + "since", + "static", + "summary", + "this", + "throws", + "todo", + "tutorial", + "type", + "typedef", + "variation", + "version", + "yields", + // JSDoc TS specific + "import", + "internal", + "overload", + "satisfies", + "template", }; const JSX_TAGS: phf::Set<&'static str> = phf_set! { -"jsx", -"jsxFrag", -"jsxImportSource", -"jsxRuntime", + "jsx", + "jsxFrag", + "jsxImportSource", + "jsxRuntime", }; const ALWAYS_INVALID_TAGS_IF_TYPED: phf::Set<&'static str> = phf_set! { -"augments", -"callback", -"class", -"enum", -"implements", -"private", -"property", -"protected", -"public", -"readonly", -"this", -"type", -"typedef", + "augments", + "callback", + "class", + "enum", + "implements", + "private", + "property", + "protected", + "public", + "readonly", + "this", + "type", + "typedef", }; const OUTSIDE_AMBIENT_INVALID_TAGS_IF_TYPED: phf::Set<&'static str> = phf_set! { -"abstract", -"access", -"class", -"constant", -"constructs", -// I'm not sure but this seems to be allowed... -// https://github.com/gajus/eslint-plugin-jsdoc/blob/e343ab5b1efaa59b07c600138aee070b4083857e/src/rules/checkTagNames.js#L140 -// "default", -"enum", -"export", -"exports", -"function", -"global", -"inherits", -"instance", -"interface", -"member", -"memberof", -"memberOf", -"method", -"mixes", -"mixin", -"module", -"name", -"namespace", -"override", -"property", -"requires", -"static", -"this", + "abstract", + "access", + "class", + "constant", + "constructs", + // I'm not sure but this seems to be allowed... + // https://github.com/gajus/eslint-plugin-jsdoc/blob/e343ab5b1efaa59b07c600138aee070b4083857e/src/rules/checkTagNames.js#L140 + // "default", + "enum", + "export", + "exports", + "function", + "global", + "inherits", + "instance", + "interface", + "member", + "memberof", + "memberOf", + "method", + "mixes", + "mixin", + "module", + "name", + "namespace", + "override", + "property", + "requires", + "static", + "this", }; impl Rule for CheckTagNames { @@ -200,20 +200,18 @@ impl Rule for CheckTagNames { fn run_once(&self, ctx: &LintContext) { let settings = &ctx.settings().jsdoc; let config = &self.0; - let user_defined_tags = settings.list_user_defined_tag_names(); let is_dts = ctx.file_path().to_str().map_or(false, |p| p.ends_with(".d.ts")); - // NOTE: Original rule seems to check `declare` context with visiting AST nodes. + // NOTE: The original rule seems to check `declare` context by visiting AST nodes. // https://github.com/gajus/eslint-plugin-jsdoc/blob/e343ab5b1efaa59b07c600138aee070b4083857e/src/rules/checkTagNames.js#L121 // But... // - No test case covers this(= only checks inside of `.d.ts`) - // - I never seen this usage before + // - I've never seen this usage before // So, I leave this part out for now. let is_declare = false; let is_ambient = is_dts || is_declare; - // TODO: Bundle multiple diagnostics into one? for jsdoc in ctx.semantic().jsdoc().iter_all() { for tag in jsdoc.tags() { let tag_name = tag.kind.parsed(); From d93eb98940810e8b36dff0e1da7c8d9f83574a7b Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Mon, 22 Apr 2024 21:19:00 +0900 Subject: [PATCH 8/9] Cosmetic --- crates/oxc_linter/src/config/settings/jsdoc.rs | 8 ++++---- crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/config/settings/jsdoc.rs b/crates/oxc_linter/src/config/settings/jsdoc.rs index ad5f678012a6c..8cc9119c5be59 100644 --- a/crates/oxc_linter/src/config/settings/jsdoc.rs +++ b/crates/oxc_linter/src/config/settings/jsdoc.rs @@ -110,7 +110,7 @@ impl JSDocPluginSettings { } } /// Only for `check-tag-names` rule - /// Return `Some(reason)` if replacement found, include default alias + /// Return `Some(reason)` if replacement found or default aliased pub fn check_preferred_tag_name(&self, original_name: &str) -> Option { let reason = |preferred_name: &str| -> String { format!("Replace tag `@{original_name}` with `@{preferred_name}`.") @@ -128,20 +128,20 @@ impl JSDocPluginSettings { } /// Only for `check-tag-names` rule /// Return all user replacement tag names - pub fn list_user_defined_tag_names(&self) -> Vec { + pub fn list_user_defined_tag_names(&self) -> Vec<&str> { self.tag_name_preference .iter() .filter_map(|(_, pref)| match pref { TagNamePreference::TagNameOnly(replacement) | TagNamePreference::ObjectWithMessageAndReplacement { replacement, .. } => { - Some(replacement.to_string()) + Some(replacement.as_str()) } _ => None, }) .collect() } - /// Resolve original, known tag name to user preferred name + /// Resolve original, known tag name to user preferred, default aliased name /// If not defined, return original name pub fn resolve_tag_name(&self, original_name: &str) -> String { match self.tag_name_preference.get(original_name) { diff --git a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs index 4455cf6053603..a58841a1428ee 100644 --- a/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs +++ b/crates/oxc_linter/src/rules/jsdoc/check_tag_names.rs @@ -217,7 +217,7 @@ impl Rule for CheckTagNames { let tag_name = tag.kind.parsed(); // If user explicitly allowed, skip - if user_defined_tags.contains(&tag_name.to_string()) + if user_defined_tags.contains(&tag_name) || config.defined_tags.contains(&tag_name.to_string()) { continue; From d1c90a0864c803c528493f24a14882c79832a255 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Tue, 23 Apr 2024 16:59:33 +0900 Subject: [PATCH 9/9] Fix settings util --- .../oxc_linter/src/config/settings/jsdoc.rs | 67 +++++++++---------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/crates/oxc_linter/src/config/settings/jsdoc.rs b/crates/oxc_linter/src/config/settings/jsdoc.rs index 8cc9119c5be59..b31fe54cd3a4c 100644 --- a/crates/oxc_linter/src/config/settings/jsdoc.rs +++ b/crates/oxc_linter/src/config/settings/jsdoc.rs @@ -70,35 +70,6 @@ pub struct JSDocPluginSettings { // }[] } -// https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#default-preferred-aliases -fn get_default_aliased_tag_name(original_name: &str) -> Option { - let aliased_name = match original_name { - "virtual" => "abstract", - "extends" => "augments", - "constructor" => "class", - "const" => "constant", - "defaultvalue" => "default", - "desc" => "description", - "host" => "external", - "fileoverview" | "overview" => "file", - "emits" => "fires", - "func" | "method" => "function", - "var" => "member", - "arg" | "argument" => "param", - "prop" => "property", - "return" => "returns", - "exception" => "throws", - "yield" => "yields", - _ => original_name, - } - .to_string(); - - if aliased_name != original_name { - return Some(aliased_name); - } - None -} - impl JSDocPluginSettings { /// Only for `check-tag-names` rule /// Return `Some(reason)` if blocked @@ -121,9 +92,34 @@ impl JSDocPluginSettings { Some(TagNamePreference::ObjectWithMessageAndReplacement { message, .. }) => { Some(message.to_string()) } - _ => get_default_aliased_tag_name(original_name) - .filter(|preferred_name| preferred_name != original_name) - .map(|preferred_name| reason(&preferred_name)), + _ => { + // https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#default-preferred-aliases + let aliased_name = match original_name { + "virtual" => "abstract", + "extends" => "augments", + "constructor" => "class", + "const" => "constant", + "defaultvalue" => "default", + "desc" => "description", + "host" => "external", + "fileoverview" | "overview" => "file", + "emits" => "fires", + "func" | "method" => "function", + "var" => "member", + "arg" | "argument" => "param", + "prop" => "property", + "return" => "returns", + "exception" => "throws", + "yield" => "yields", + _ => original_name, + }; + + if aliased_name != original_name { + return Some(reason(aliased_name)); + } + + None + } } } /// Only for `check-tag-names` rule @@ -141,7 +137,7 @@ impl JSDocPluginSettings { .collect() } - /// Resolve original, known tag name to user preferred, default aliased name + /// Resolve original, known tag name to user preferred name /// If not defined, return original name pub fn resolve_tag_name(&self, original_name: &str) -> String { match self.tag_name_preference.get(original_name) { @@ -149,7 +145,7 @@ impl JSDocPluginSettings { TagNamePreference::TagNameOnly(replacement) | TagNamePreference::ObjectWithMessageAndReplacement { replacement, .. }, ) => replacement.to_string(), - _ => get_default_aliased_tag_name(original_name).unwrap_or(original_name.to_string()), + _ => original_name.to_string(), } } } @@ -210,9 +206,6 @@ mod test { fn resolve_tag_name() { let settings = JSDocPluginSettings::deserialize(&serde_json::json!({})).unwrap(); assert_eq!(settings.resolve_tag_name("foo"), "foo".to_string()); - assert_eq!(settings.resolve_tag_name("virtual"), "abstract".to_string()); - assert_eq!(settings.resolve_tag_name("fileoverview"), "file".to_string()); - assert_eq!(settings.resolve_tag_name("overview"), "file".to_string()); let settings = JSDocPluginSettings::deserialize(&serde_json::json!({ "tagNamePreference": {