From c18ad00539f103007bffd44592c5bcd28861722d Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 14:19:01 +0900 Subject: [PATCH 1/9] feat(linter/jsdoc): Implement require-param rule --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/jsdoc/require_param.rs | 1470 +++++++++++++++++ .../src/snapshots/require_param.snap | 426 +++++ crates/oxc_linter/src/utils/jsdoc.rs | 6 +- 4 files changed, 1903 insertions(+), 1 deletion(-) create mode 100644 crates/oxc_linter/src/rules/jsdoc/require_param.rs create mode 100644 crates/oxc_linter/src/snapshots/require_param.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index ec6c46e2f2298..e7900cc063c39 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -390,6 +390,7 @@ mod jsdoc { pub mod empty_tags; pub mod implements_on_classes; pub mod no_defaults; + pub mod require_param; pub mod require_property; pub mod require_property_description; pub mod require_property_name; @@ -756,6 +757,7 @@ oxc_macros::declare_all_lint_rules! { jsdoc::empty_tags, jsdoc::implements_on_classes, jsdoc::no_defaults, + jsdoc::require_param, jsdoc::require_property, jsdoc::require_property_type, jsdoc::require_property_name, diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs new file mode 100644 index 0000000000000..48b9ae75c3b72 --- /dev/null +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -0,0 +1,1470 @@ +use oxc_ast::{ + ast::{BindingPattern, BindingPatternKind, Expression, FormalParameters, MethodDefinitionKind}, + AstKind, +}; +use oxc_diagnostics::LabeledSpan; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::{AstNode, JSDoc}; +use oxc_span::Span; +use regex::Regex; +use rustc_hash::FxHashSet; +use serde::Deserialize; + +use crate::{ + context::LintContext, + rule::Rule, + utils::{ + get_function_nearest_jsdoc_node, should_ignore_as_avoid, should_ignore_as_internal, + should_ignore_as_private, + }, +}; + +#[derive(Debug, Default, Clone)] +pub struct RequireParam(Box); + +declare_oxc_lint!( + /// ### What it does + /// Requires that all function parameters are documented with JSDoc `@param` tags. + /// + /// ### Why is this bad? + /// The rule is aimed at enforcing code quality and maintainability by requiring that all function parameters are documented. + /// + /// ### Example + /// ```javascript + /// // Passing + /// /** @param foo */ + /// function quux (foo) {} + /// + /// // Failing + /// /** @param foo */ + /// function quux (foo, bar) {} + /// ``` + RequireParam, + pedantic, +); + +#[derive(Debug, Clone, Deserialize)] +struct RequireParamConfig { + #[serde(default = "default_exempted_by", rename = "exemptedBy")] + exempted_by: Vec, + #[serde(default = "default_true", rename = "checkConstructors")] + check_constructors: bool, + #[serde(default, rename = "checkGetters")] + check_getters: bool, + #[serde(default, rename = "checkSetters")] + check_setters: bool, + #[serde(default = "default_true", rename = "checkDestructuredRoots")] + check_destructured_roots: bool, + #[serde(default = "default_true", rename = "checkDestructured")] + check_destructured: bool, + #[serde(default, rename = "checkRestProperty")] + check_rest_property: bool, + #[serde(default = "default_check_types_pattern", rename = "checkTypesPattern")] + check_types_pattern: String, + // TODO: Support this config + // #[serde(default, rename = "useDefaultObjectProperties")] + // use_default_object_properties: bool, +} +impl Default for RequireParamConfig { + fn default() -> Self { + Self { + exempted_by: default_exempted_by(), + check_constructors: false, + check_getters: default_true(), + check_setters: default_true(), + check_destructured_roots: default_true(), + check_destructured: default_true(), + check_rest_property: false, + check_types_pattern: default_check_types_pattern(), + } + } +} +fn default_exempted_by() -> Vec { + vec!["inheritdoc".to_string()] +} +fn default_true() -> bool { + true +} +fn default_check_types_pattern() -> String { + "^(?:[oO]bject|[aA]rray|PlainObject|Generic(?:Object|Array))$".to_string() +} + +impl Rule for RequireParam { + 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<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + // Collected targets from `FormalParameters` + let params_to_check = match node.kind() { + AstKind::Function(func) => collect_params(&func.params), + AstKind::ArrowFunctionExpression(arrow_func) => collect_params(&arrow_func.params), + // If not a function, skip + _ => return, + }; + + let Some(func_def_node) = get_function_nearest_jsdoc_node(node, ctx) else { + return; + }; + // If no JSDoc is found, skip + let Some(jsdocs) = ctx.jsdoc().get_all_by_node(func_def_node) else { + return; + }; + + let config = &self.0; + let check_types_regex = Regex::new(config.check_types_pattern.as_str()) + .expect("`config.checkTypesPattern` should be a valid regex pattern"); + let settings = &ctx.settings().jsdoc; + + // If config disabled checking, skip + if let AstKind::MethodDefinition(method_def) = func_def_node.kind() { + match method_def.kind { + MethodDefinitionKind::Get => { + if !config.check_getters { + return; + } + } + MethodDefinitionKind::Set => { + if !config.check_setters { + return; + } + } + MethodDefinitionKind::Constructor => { + if !config.check_constructors { + return; + } + } + MethodDefinitionKind::Method => {} + } + } + + // 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; + } + + // Collected JSDoc `@param` tags + let tags_to_check = collect_tags(&jsdocs, &settings.resolve_tag_name("param")); + let shallow_tags = + tags_to_check.iter().filter(|(name, _)| !name.contains('.')).collect::>(); + + let mut violations = vec![]; + for (idx, param) in params_to_check.iter().enumerate() { + match param { + ParamKind::Single(param) => { + if !config.check_rest_property && param.is_rest { + continue; + } + + if !tags_to_check.iter().any(|(name, _)| **name == param.name) { + violations.push(param.span); + } + } + ParamKind::Nested(params) => { + // If false, skip nested root + if !config.check_destructured_roots { + continue; + } + + let matched_param_tag = shallow_tags.get(idx); + + // If {type} exists... + if let Some((_, Some(r#type))) = matched_param_tag { + // ... and doesn't match the pattern, skip + if !check_types_regex.is_match(r#type) { + continue; + } + } + + // If false, skip nested props + if !config.check_destructured { + continue; + } + + let root_name = matched_param_tag.map_or("", |(name, _)| name); + let mut not_checking_names = FxHashSet::default(); + for param in params { + if !config.check_rest_property && param.is_rest { + continue; + } + + let full_param_name = format!("{root_name}.{}", param.name); + for (name, type_part) in &tags_to_check { + if !is_name_equal(name, &full_param_name) { + continue; + } + let Some(r#type) = type_part else { + continue; + }; + if check_types_regex.is_match(r#type) { + continue; + } + + not_checking_names.insert(name); + } + + if not_checking_names.iter().any(|&name| full_param_name.starts_with(name)) + { + continue; + } + + if !tags_to_check + .iter() + .any(|(name, _)| is_name_equal(name, &full_param_name)) + { + violations.push(param.span); + } + } + } + } + } + + if !violations.is_empty() { + let labels = violations + .iter() + .map(|span| LabeledSpan::new_with_span(None, *span)) + .collect::>(); + ctx.diagnostic( + OxcDiagnostic::warn("eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters.") + .with_help("Add `@param` tag with name.") + .with_labels(labels), + ); + } + } +} + +#[derive(Debug, Clone)] +struct Param { + span: Span, + name: String, + is_rest: bool, +} + +#[derive(Debug, Clone)] +enum ParamKind { + Single(Param), + Nested(Vec), +} + +fn collect_params(params: &FormalParameters) -> Vec { + // NOTE: Property level `is_rest` is implemented. + // - fn(a, { b1, ...b2 }) + // ^^^^^ + // But Object|Array level `is_rest` is not implemented + // - fn(a, ...{ b }) + // ^^^^ ^ + // Tests are not covering these cases... + fn get_param_name(pattern: &BindingPattern, is_rest: bool) -> ParamKind { + match &pattern.kind { + BindingPatternKind::BindingIdentifier(ident) => { + ParamKind::Single(Param { span: ident.span, name: ident.name.to_string(), is_rest }) + } + BindingPatternKind::ObjectPattern(obj_pat) => { + let mut collected = vec![]; + + for prop in &obj_pat.properties { + let name = prop.key.name().expect("Object key"); + + match get_param_name(&prop.value, false) { + ParamKind::Single(param) => { + collected.push(Param { name: format!("{name}"), ..param }); + } + ParamKind::Nested(params) => { + collected.push(Param { + span: prop.span, + name: format!("{name}"), + is_rest: false, + }); + + for param in params { + collected.push(Param { + name: format!("{name}.{}", param.name), + ..param + }); + } + } + } + } + + if let Some(rest) = &obj_pat.rest { + match get_param_name(&rest.argument, true) { + ParamKind::Single(param) => collected.push(param), + ParamKind::Nested(params) => collected.extend(params), + } + } + + ParamKind::Nested(collected) + } + BindingPatternKind::ArrayPattern(arr_pat) => { + let mut collected = vec![]; + + for (idx, elm) in arr_pat.elements.iter().enumerate() { + let name = format!("\"{idx}\""); + + if let Some(pat) = elm { + match get_param_name(pat, false) { + ParamKind::Single(param) => collected.push(Param { name, ..param }), + ParamKind::Nested(params) => collected.extend(params), + } + } + } + + if let Some(rest) = &arr_pat.rest { + match get_param_name(&rest.argument, true) { + ParamKind::Single(param) => collected.push(param), + ParamKind::Nested(params) => collected.extend(params), + } + } + + ParamKind::Nested(collected) + } + BindingPatternKind::AssignmentPattern(assign_pat) => match &assign_pat.right { + Expression::Identifier(_) => get_param_name(&assign_pat.left, false), + _ => { + // TODO: If `config.useDefaultObjectProperties` = true, + // collect default parameters from `assign_pat.right` like: + // { prop = { a: 1, b: 2 }} => [prop, prop.a, prop.b] + // get_param_name(&assign_pat.left, false) + // } + get_param_name(&assign_pat.left, false) + } + }, + } + } + + let mut collected = + params.items.iter().map(|param| get_param_name(¶m.pattern, false)).collect::>(); + + if let Some(rest) = ¶ms.rest { + match get_param_name(&rest.argument, true) { + ParamKind::Single(param) => collected.push(ParamKind::Single(param)), + ParamKind::Nested(params) => collected.push(ParamKind::Nested(params)), + } + } + + collected +} + +fn collect_tags<'a>( + jsdocs: &[JSDoc<'a>], + resolved_param_tag_name: &str, +) -> Vec<(&'a str, Option<&'a str>)> { + let mut collected = vec![]; + + for tag in jsdocs + .iter() + .flat_map(JSDoc::tags) + .filter(|tag| tag.kind.parsed() == resolved_param_tag_name) + { + let (type_part, Some(name_part), _) = tag.type_name_comment() else { + continue; + }; + + let name = name_part.parsed(); + // thisParam is special, not collected as `FormalParameter`, should be ignored + if name == "this" { + continue; + } + + collected.push((name, type_part.map(|p| p.parsed()))); + } + + collected +} + +fn should_ignore_as_custom_skip(jsdoc: &JSDoc) -> bool { + jsdoc.tags().iter().any(|tag| "type" == tag.kind.parsed()) +} + +/// Compare to string param names without quotes +/// e.g. `foo."bar"` +fn is_name_equal(a: &str, b: &str) -> bool { + a.chars().filter(|c| *c != '"').collect::>() + == b.chars().filter(|c| *c != '"').collect::>() +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + (" + /** + * @param foo + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @param root0 + * @param root0.foo + */ + function quux ({foo}) { + + } + ", None, None), +(" + /** + * @param root0 + * @param root0.foo + * @param root1 + * @param root1.bar + */ + function quux ({foo}, {bar}) { + + } + ", None, None), +(" + /** + * @param arg0 + * @param arg0.foo + * @param arg1 + * @param arg1.bar + */ + function quux ({foo}, {bar}) { + + } + ", Some(serde_json::json!([ { "unnamedRootBase": [ "arg", ], }, ])), None), +(" + /** + * @param arg + * @param arg.foo + * @param config0 + * @param config0.bar + * @param config1 + * @param config1.baz + */ + function quux ({foo}, {bar}, {baz}) { + + } + ", Some(serde_json::json!([ { "unnamedRootBase": [ "arg", "config", ], }, ])), None), +(" + /** + * @inheritdoc + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @arg foo + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "tagNamePreference": { "param": "arg", }, }, } }))), +(" + /** + * @override + * @param foo + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @override + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @override + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "overrideReplacesDocs": true, }, } }))), +(" + /** + * @ignore + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "ignoreReplacesDocs": true, }, } }))), +(" + /** + * @implements + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "implementsReplacesDocs": true, }, } }))), +(" + /** + * @implements + * @param foo + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @augments + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "augmentsExtendsReplacesDocs": true, }, } }))), +(" + /** + * @augments + * @param foo + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @extends + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "augmentsExtendsReplacesDocs": true, }, } }))), +(" + /** + * @extends + * @param foo + */ + function quux (foo) { + + } + ", None, None), +(" + /** + * @internal + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "ignoreInternal": true, }, } }))), +(" + /** + * @private + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "ignorePrivate": true, }, } }))), +(" + /** + * @access private + */ + function quux (foo) { + + } + ", None, Some(serde_json::json!({ "settings": { "jsdoc": { "ignorePrivate": true, }, } }))), +(" + // issue 182: optional chaining + /** @const {boolean} test */ + const test = something?.find(_ => _) + ", None, None), // { "parser": babelEslintParser, }, +(" + /** + * @type {MyCallback} + */ + function quux () { + + } + ", Some(serde_json::json!([ { "exemptedBy": [ "type", ], }, ])), None), +(" + export class SomeClass { + /** + * @param property + */ + constructor(private property: string) {} + } + ", None, None), // { "parser": typescriptEslintParser, "sourceType": "module", }, +(" + /** + * Assign the project to an employee. + * + * @param {object} employee - The employee who is responsible for the project. + * @param {string} employee.name - The name of the employee. + * @param {string} employee.department - The employee's department. + */ + function assign({name, department}) { + // ... + } + ", None, None), +(" + export abstract class StephanPlugin { + + /** + * Called right after Stephan loads the plugin file. + * + * @example + *```typescript + * type Options = { + * verbose?: boolean; + * token?: string; + * } + * ``` + * + * Note that your Options type should only have optional properties... + * + * @param args Arguments compiled and provided by StephanClient. + * @param args.options The options as provided by the user, or an empty object if not provided. + * @param args.client The options as provided by the user, or an empty object if not provided. + * @param defaultOptions The default options as provided by the plugin, or an empty object. + */ + public constructor({options, client}: { + options: O; + client: unknown; + }, defaultOptions: D) { + + } + } + ", None, None), // { "parser": typescriptEslintParser }, +(" + export abstract class StephanPlugin { + + /** + * Called right after Stephan loads the plugin file. + * + * @example + *```typescript + * type Options = { + * verbose?: boolean; + * token?: string; + * } + * ``` + * + * Note that your Options type should only have optional properties... + * + * @param args Arguments compiled and provided by StephanClient. + * @param args.options The options as provided by the user, or an empty object if not provided. + * @param args.client The options as provided by the user, or an empty object if not provided. + * @param args.client.name The name of the client. + * @param defaultOptions The default options as provided by the plugin, or an empty object. + */ + public constructor({ options, client: { name } }: { + options: O; + client: { name: string }; + }, defaultOptions: D) { + + } + } + ", None, None), // { "parser": typescriptEslintParser }, +(" + /** + * @param {string} cb + */ + function createGetter (cb) { + return function (...args) { + cb(); + }; + } + ", None, None), +(" + /** + * @param cfg + * @param cfg.num + */ + function quux ({num, ...extra}) { + } + ", None, None), +(" + /** + * Converts an SVGRect into an object. + * @param {SVGRect} bbox - a SVGRect + */ + const bboxToObj = function ({x, y, width, height}) { + return {x, y, width, height}; + }; + ", None, None), +(" + /** + * Converts an SVGRect into an object. + * @param {object} bbox - a SVGRect + */ + const bboxToObj = function ({x, y, width, height}) { + return {x, y, width, height}; + }; + ", Some(serde_json::json!([ { "checkTypesPattern": "SVGRect", }, ])), None), +(" + class CSS { + /** + * Set one or more CSS properties for the set of matched elements. + * + * @param {Object} propertyObject - An object of property-value pairs to set. + */ + setCssObject(propertyObject: {[key: string]: string | number}): void { + } + } + ", None, None), // { "parser": typescriptEslintParser }, +(" + /** + * @param foo + * @param bar + * @param cfg + */ + function quux (foo, bar, {baz}) { + + } + ", Some(serde_json::json!([ { "checkDestructured": false, }, ])), None), +(" + /** + * @param foo + * @param bar + */ + function quux (foo, bar, {baz}) { + + } + ", Some(serde_json::json!([ { "checkDestructuredRoots": false, }, ])), None), +(r#" + /** + * @param root + * @param root.foo + */ + function quux ({"foo": bar}) { + + } + "#, None, None), +(r#" + /** + * @param root + * @param root."foo" + */ + function quux ({foo: bar}) { + + } + "#, None, None), +(" + /** + * Description. + * @param {string} b Description `/**`. + */ + module.exports = function a(b) { + console.info(b); + }; + ", None, None), +(" + /** + * Description. + * @param {Object} options Options. + * @param {FooBar} options.foo foo description. + */ + function quux ({ foo: { bar } }) {} + ", None, None), +(" + /** + * Description. + * @param {FooBar} options + * @param {Object} options.foo + */ + function quux ({ foo: { bar } }) {} + ", Some(serde_json::json!([ { "checkTypesPattern": "FooBar", }, ])), None), +(r#" + /** + * @param obj + * @param obj.data + * @param obj.data."0" + * @param obj.data."1" + * @param obj.data."2" + * @param obj.defaulting + * @param obj.defaulting."0" + * @param obj.defaulting."1" + */ + function Item({ + data: [foo, bar, ...baz], + defaulting: [quux, xyz] = [] + }) { + } + "#, None, None), +// (" +// /** +// * Returns a number. +// * @param {Object} props Props. +// * @param {Object} props.prop Prop. +// * @return {number} A number. +// */ +// export function testFn1 ({ prop = { a: 1, b: 2 } }) { +// } +// ", Some(serde_json::json!([ { "useDefaultObjectProperties": false, }, ])), None), // { "sourceType": "module", }, +(" + /** + * @param this The this object + * @param bar number to return + * @returns number returned back to caller + */ + function foo(this: T, bar: number): number { + console.log(this.name); + return bar; + } + ", None, None), // { "parser": typescriptEslintParser }, +(" + /** + * @param bar number to return + * @returns number returned back to caller + */ + function foo(this: T, bar: number): number { + console.log(this.name); + return bar; + } + ", None, None), // { "parser": typescriptEslintParser }, +(" + /** + * Returns the sum of two numbers + * @param options Object to destructure + * @param options.a First value + * @param options.b Second value + * @returns Sum of a and b + */ + function sumDestructure(this: unknown, { a, b }: { a: number, b: number }) { + return a + b; + } + ", None, None), // { "parser": typescriptEslintParser, } + ]; + + let fail = vec![ + ( + " + /** + * + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + " + /** + * + */ + function quux ({foo}) { + + } + ", + None, + None, + ), + ( + " + /** + * @param foo + */ + function quux (foo, bar, {baz}) { + + } + ", + Some( + serde_json::json!([ { "checkDestructured": false, }, ]), + ), + None, + ), + ( + " + /** + * @param foo + */ + function quux (foo, bar, {baz}) { + + } + ", + Some( + serde_json::json!([ { "checkDestructuredRoots": false, }, ]), + ), + None, + ), + ( + " + /** + * + */ + function quux ({foo}) { + + } + ", + Some(serde_json::json!([ { "enableFixer": false, }, ])), + None, + ), + ( + " + /** + * + */ + function quux ({foo: bar = 5} = {}) { + + } + ", + None, + None, + ), + ( + " + /** + * @param + */ + function quux ({foo}) { + + } + ", + None, + None, + ), + ( + " + /** + * @param + */ + function quux ({foo}) { + + } + ", + Some(serde_json::json!([ { "autoIncrementBase": 1, }, ])), + None, + ), + ( + " + /** + * @param options + */ + function quux ({foo}) { + + } + ", + None, + None, + ), + ( + " + /** + * @param + */ + function quux ({ foo, bar: { baz }}) { + + } + ", + None, + None, + ), + ( + " + /** + * + */ + function quux ({foo}, {bar}) { + + } + ", + Some( + serde_json::json!([ { "unnamedRootBase": [ "arg", ], }, ]), + ), + None, + ), + ( + " + /** + * + */ + function quux ({foo}, {bar}) { + + } + ", + Some( + serde_json::json!([ { "unnamedRootBase": [ "arg", "config", ], }, ]), + ), + None, + ), + ( + " + /** + * + */ + function quux ({foo}, {bar}) { + + } + ", + Some( + serde_json::json!([ { "enableRootFixer": false, "unnamedRootBase": [ "arg", "config", ], }, ]), + ), + None, + ), + ( + " + /** + * + */ + function quux (foo, bar) { + + } + ", + None, + None, + ), + ( + " + /** + * @param foo + */ + function quux (foo, bar) { + + } + ", + None, + None, + ), + ( + " + /** + * @param bar + */ + function quux (foo, bar, baz) { + + } + ", + None, + None, + ), + ( + " + /** + * @param foo + * @param bar + */ + function quux (foo, bar, baz) { + + } + ", + None, + None, + ), + ( + " + /** + * @param baz + */ + function quux (foo, bar, baz) { + + } + ", + None, + None, + ), + ( + " + /** + * @param + */ + function quux (foo) { + + } + ", + None, + Some( + serde_json::json!({ "settings": { "jsdoc": { "tagNamePreference": { "param": "arg", }, }, } }), + ), + ), + ( + " + /** + * @override + */ + function quux (foo) { + + } + ", + None, + Some( + serde_json::json!({ "settings": { "jsdoc": { "overrideReplacesDocs": false, }, } }), + ), + ), + ( + " + /** + * @ignore + */ + function quux (foo) { + + } + ", + None, + Some( + serde_json::json!({ "settings": { "jsdoc": { "ignoreReplacesDocs": false, }, } }), + ), + ), + ( + " + /** + * @implements + */ + function quux (foo) { + + } + ", + None, + Some( + serde_json::json!({ "settings": { "jsdoc": { "implementsReplacesDocs": false, }, } }), + ), + ), + ( + " + /** + * @augments + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + " + /** + * @extends + */ + function quux (foo) { + + } + ", + None, + None, + ), + ( + " + /** + * + */ + function quux ({bar, baz}, foo) { + } + ", + None, + None, + ), + ( + " + /** + * + */ + function quux (foo, {bar, baz}) { + } + ", + None, + None, + ), + ( + " + /** + * + */ + function quux ([bar, baz], foo) { + } + ", + None, + None, + ), + ( + " + /** + * + */ + function quux (foo) { + } + ", + Some( + serde_json::json!([ { "exemptedBy": [ "notPresent", ], }, ]), + ), + None, + ), + ( + " + /** + * @inheritdoc + */ + function quux (foo) { + + } + ", + Some(serde_json::json!([ { "exemptedBy": [], }, ])), + None, + ), + ( + " + /** + * Assign the project to a list of employees. + * @param {object[]} employees - The employees who are responsible for the project. + * @param {string} employees[].name - The name of an employee. + * @param {string} employees[].department - The employee's department. + */ + function assign (employees, name) { + + }; + ", + None, + None, + ), + ( + " + /** + * @param baz + * @param options + */ + function quux (baz, {foo: bar}) { + + } + ", + None, + None, + ), + ( + " + /** + * + */ + function quux (foo) { + + } + ", + Some(serde_json::json!([ { "enableFixer": false, }, ])), + None, + ), + ( + " + class Client { + /** + * Set collection data. + * @return {Promise} + */ + async setData( + data: { last_modified?: number } + ) {} + } + ", + None, + None, + ), // { "parser": typescriptEslintParser }, + ( + " + /** + * @param cfg + * @param cfg.num + */ + function quux ({num, ...extra}) { + } + ", + Some( + serde_json::json!([ { "checkRestProperty": true, }, ]), + ), + None, + ), + ( + " + /** + * @param cfg + * @param cfg.opts + * @param cfg.opts.num + */ + function quux ({opts: {num, ...extra}}) { + } + ", + Some( + serde_json::json!([ { "checkRestProperty": true, }, ]), + ), + None, + ), + ( + r#" + /** + * @param {GenericArray} cfg + * @param {number} cfg."0" + */ + function baar ([a, ...extra]) { + // + } + "#, + Some( + serde_json::json!([ { "checkRestProperty": true, }, ]), + ), + None, + ), + ( + " + /** + * @param a + */ + function baar (a, ...extra) { + // + } + ", + Some( + serde_json::json!([ { "checkRestProperty": true, }, ]), + ), + None, + ), + ( + " + /** + * Converts an SVGRect into an object. + * @param {SVGRect} bbox - a SVGRect + */ + const bboxToObj = function ({x, y, width, height}) { + return {x, y, width, height}; + }; + ", + Some( + serde_json::json!([ { "checkTypesPattern": "SVGRect", }, ]), + ), + None, + ), + ( + " + /** + * Converts an SVGRect into an object. + * @param {object} bbox - a SVGRect + */ + const bboxToObj = function ({x, y, width, height}) { + return {x, y, width, height}; + }; + ", + None, + None, + ), + ( + " + module.exports = class GraphQL { + /** + * @param fetchOptions + * @param cacheKey + */ + fetch = ({ url, ...options }, cacheKey) => { + } + }; + ", + Some( + serde_json::json!([ { "checkRestProperty": true, }, ]), + ), + None, + ), // { "parser": babelEslintParser, }, + ( + " + (function() { + /** + * A function. + */ + function f(param) { + return !param; + } + })(); + ", + None, + None, + ), + ( + " + /** + * Description. + * @param {Object} options + * @param {Object} options.foo + */ + function quux ({ foo: { bar } }) {} + ", + None, + None, + ), + ( + " + /** + * Description. + * @param {FooBar} options + * @param {FooBar} options.foo + */ + function quux ({ foo: { bar } }) {} + ", + Some( + serde_json::json!([ { "checkTypesPattern": "FooBar", }, ]), + ), + None, + ), + ( + " + /** + * Description. + * @param {Object} options + * @param {FooBar} foo + */ + function quux ({ foo: { bar } }) {} + ", + None, + None, + ), + ( + " + /** + * Description. + * @param {Object} options + * @param options.foo + */ + function quux ({ foo: { bar } }) {} + ", + None, + None, + ), + ( + " + /** + * Description. + * @param {object} options Options. + * @param {object} options.foo A description. + * @param {object} options.foo.bar + */ + function foo({ foo: { bar: { baz } }}) {} + ", + None, + None, + ), + // ( + // " + // /** + // * Returns a number. + // * @param {Object} props Props. + // * @param {Object} props.prop Prop. + // * @return {number} A number. + // */ + // export function testFn1 ({ prop = { a: 1, b: 2 } }) { + // } + // ", + // Some( + // serde_json::json!([ { "useDefaultObjectProperties": true, }, ]), + // ), + // None, + // ), // { "sourceType": "module", }, + ( + " + /** Foo. */ + function foo(a, b, c) {} + ", + None, + None, + ), + ]; + + Tester::new(RequireParam::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/require_param.snap b/crates/oxc_linter/src/snapshots/require_param.snap new file mode 100644 index 0000000000000..e5ae5c9c257db --- /dev/null +++ b/crates/oxc_linter/src/snapshots/require_param.snap @@ -0,0 +1,426 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: require_param +--- + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:30] + 4 │ */ + 5 │ function quux ({foo}) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:34] + 4 │ */ + 5 │ function quux (foo, bar, {baz}) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:34] + 4 │ */ + 5 │ function quux (foo, bar, {baz}) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:30] + 4 │ */ + 5 │ function quux ({foo}) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:35] + 4 │ */ + 5 │ function quux ({foo: bar = 5} = {}) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:30] + 4 │ */ + 5 │ function quux ({foo}) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:30] + 4 │ */ + 5 │ function quux ({foo}) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:30] + 4 │ */ + 5 │ function quux ({foo}) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:31] + 4 │ */ + 5 │ function quux ({ foo, bar: { baz }}) { + · ─── ───────────── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:30] + 4 │ */ + 5 │ function quux ({foo}, {bar}) { + · ─── ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:30] + 4 │ */ + 5 │ function quux ({foo}, {bar}) { + · ─── ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:30] + 4 │ */ + 5 │ function quux ({foo}, {bar}) { + · ─── ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo, bar) { + · ─── ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:34] + 4 │ */ + 5 │ function quux (foo, bar) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo, bar, baz) { + · ─── ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:6:39] + 5 │ */ + 6 │ function quux (foo, bar, baz) { + · ─── + 7 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo, bar, baz) { + · ─── ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:26] + 4 │ */ + 5 │ function quux ({bar, baz}, foo) { + · ─── ─── ─── + 6 │ } + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:25] + 4 │ */ + 5 │ function quux (foo, {bar, baz}) { + · ─── ─── ─── + 6 │ } + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:26] + 4 │ */ + 5 │ function quux ([bar, baz], foo) { + · ─── ─── ─── + 6 │ } + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ } + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:8:42] + 7 │ */ + 8 │ function assign (employees, name) { + · ──── + 9 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:6:40] + 5 │ */ + 6 │ function quux (baz, {foo: bar}) { + · ─── + 7 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:29] + 4 │ */ + 5 │ function quux (foo) { + · ─── + 6 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:8:14] + 7 │ async setData( + 8 │ data: { last_modified?: number } + · ──────────────────────────────── + 9 │ ) {} + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:6:34] + 5 │ */ + 6 │ function quux ({num, ...extra}) { + · ───── + 7 │ } + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:7:41] + 6 │ */ + 7 │ function quux ({opts: {num, ...extra}}) { + · ───── + 8 │ } + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:6:32] + 5 │ */ + 6 │ function baar ([a, ...extra]) { + · ───── + 7 │ // + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:5:31] + 4 │ */ + 5 │ function baar (a, ...extra) { + · ───── + 6 │ // + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:6:39] + 5 │ */ + 6 │ const bboxToObj = function ({x, y, width, height}) { + · ─ ─ ───── ────── + 7 │ return {x, y, width, height}; + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:6:39] + 5 │ */ + 6 │ const bboxToObj = function ({x, y, width, height}) { + · ─ ─ ───── ────── + 7 │ return {x, y, width, height}; + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:7:23] + 6 │ */ + 7 │ fetch = ({ url, ...options }, cacheKey) => { + · ─── ─────── + 8 │ } + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:6:16] + 5 │ */ + 6 │ function f(param) { + · ───── + 7 │ return !param; + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:7:34] + 6 │ */ + 7 │ function quux ({ foo: { bar } }) {} + · ─── + 8 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:7:34] + 6 │ */ + 7 │ function quux ({ foo: { bar } }) {} + · ─── + 8 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:7:27] + 6 │ */ + 7 │ function quux ({ foo: { bar } }) {} + · ───────────── + 8 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:7:34] + 6 │ */ + 7 │ function quux ({ foo: { bar } }) {} + · ─── + 8 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:8:39] + 7 │ */ + 8 │ function foo({ foo: { bar: { baz } }}) {} + · ─── + 9 │ + ╰──── + help: Add `@param` tag with name. + + ⚠ eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters. + ╭─[require_param.tsx:3:25] + 2 │ /** Foo. */ + 3 │ function foo(a, b, c) {} + · ─ ─ ─ + 4 │ + ╰──── + help: Add `@param` tag with name. diff --git a/crates/oxc_linter/src/utils/jsdoc.rs b/crates/oxc_linter/src/utils/jsdoc.rs index 4b14cbe28a771..3d54d3877903b 100644 --- a/crates/oxc_linter/src/utils/jsdoc.rs +++ b/crates/oxc_linter/src/utils/jsdoc.rs @@ -38,7 +38,11 @@ pub fn get_function_nearest_jsdoc_node<'a, 'b>( | AstKind::CallExpression(_) // /** This JSDoc should NOT found for `ArrowFunctionExpression` callback */ // new Promise(() => {}) - | AstKind::NewExpression(_) => { + | AstKind::NewExpression(_) + // /** This JSDoc should NOT found for inner `Function` */ + // function outer() { return function inner() {} } + | AstKind::ReturnStatement(_) + => { // /** This JSDoc should NOT found for `VariableDeclaration` */ // export const foo = () => {} let parent_node = ctx.nodes().parent_node(current_node.id())?; From db4fd69f33cba6e7eee09d4fc9972ad20fadeefc Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 17:02:21 +0900 Subject: [PATCH 2/9] Intro spellchecker:disable-line --- .typos.toml | 3 +++ crates/oxc_linter/src/rules/jsdoc/require_param.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.typos.toml b/.typos.toml index de1e00299ee7e..392526d1fa5ab 100644 --- a/.typos.toml +++ b/.typos.toml @@ -18,6 +18,9 @@ extend-exclude = [ "tasks/prettier_conformance/prettier", ] +[default] +extend-ignore-re = ["(?Rm)^.*(#|//)\\s*spellchecker:disable-line$"] + [default.extend-words] trivias = "trivias" trivia = "trivia" diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index 48b9ae75c3b72..6537c7edb5cdc 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -87,7 +87,7 @@ fn default_true() -> bool { true } fn default_check_types_pattern() -> String { - "^(?:[oO]bject|[aA]rray|PlainObject|Generic(?:Object|Array))$".to_string() + "^(?:[oO]bject|[aA]rray|PlainObject|Generic(?:Object|Array))$".to_string() // spellchecker:disable-line } impl Rule for RequireParam { From 3d0d58fe3e202c5127a151b3e01ff024791add49 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 17:15:57 +0900 Subject: [PATCH 3/9] Fix perf? --- crates/oxc_linter/src/rules/jsdoc/require_param.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index 6537c7edb5cdc..e5eea33ce7fce 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -102,7 +102,7 @@ impl Rule for RequireParam { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { // Collected targets from `FormalParameters` let params_to_check = match node.kind() { - AstKind::Function(func) => collect_params(&func.params), + AstKind::Function(func) if !func.is_typescript_syntax() => collect_params(&func.params), AstKind::ArrowFunctionExpression(arrow_func) => collect_params(&arrow_func.params), // If not a function, skip _ => return, @@ -117,8 +117,6 @@ impl Rule for RequireParam { }; let config = &self.0; - let check_types_regex = Regex::new(config.check_types_pattern.as_str()) - .expect("`config.checkTypesPattern` should be a valid regex pattern"); let settings = &ctx.settings().jsdoc; // If config disabled checking, skip @@ -161,6 +159,9 @@ impl Rule for RequireParam { let shallow_tags = tags_to_check.iter().filter(|(name, _)| !name.contains('.')).collect::>(); + let check_types_regex = Regex::new(config.check_types_pattern.as_str()) + .expect("`config.checkTypesPattern` should be a valid regex pattern"); + let mut violations = vec![]; for (idx, param) in params_to_check.iter().enumerate() { match param { From d6c54ac0ff424cc864b459c34c09aed7f797ac6d Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 17:35:14 +0900 Subject: [PATCH 4/9] Fix perf?: avoid format --- .../src/rules/jsdoc/require_param.rs | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index e5eea33ce7fce..a67478ef73385 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -202,7 +202,10 @@ impl Rule for RequireParam { continue; } - let full_param_name = format!("{root_name}.{}", param.name); + let mut full_param_name = root_name.to_string(); + full_param_name.push('.'); + full_param_name.push_str(¶m.name); + for (name, type_part) in &tags_to_check { if !is_name_equal(name, &full_param_name) { continue; @@ -281,20 +284,20 @@ fn collect_params(params: &FormalParameters) -> Vec { match get_param_name(&prop.value, false) { ParamKind::Single(param) => { - collected.push(Param { name: format!("{name}"), ..param }); + collected.push(Param { name: name.to_string(), ..param }); } ParamKind::Nested(params) => { collected.push(Param { span: prop.span, - name: format!("{name}"), + name: name.to_string(), is_rest: false, }); for param in params { - collected.push(Param { - name: format!("{name}.{}", param.name), - ..param - }); + let mut name = name.to_string(); + name.push('.'); + name.push_str(¶m.name); + collected.push(Param { name, ..param }); } } } @@ -313,7 +316,10 @@ fn collect_params(params: &FormalParameters) -> Vec { let mut collected = vec![]; for (idx, elm) in arr_pat.elements.iter().enumerate() { - let name = format!("\"{idx}\""); + let mut name = String::new(); + name.push('"'); + name.push_str(&idx.to_string()); + name.push('"'); if let Some(pat) = elm { match get_param_name(pat, false) { From 2cdf6c0861979c7d15d092e3e3766b032b4d244e Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 17:45:21 +0900 Subject: [PATCH 5/9] Revert "Fix perf?: avoid format" This reverts commit d6c54ac0ff424cc864b459c34c09aed7f797ac6d. --- .../src/rules/jsdoc/require_param.rs | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index a67478ef73385..e5eea33ce7fce 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -202,10 +202,7 @@ impl Rule for RequireParam { continue; } - let mut full_param_name = root_name.to_string(); - full_param_name.push('.'); - full_param_name.push_str(¶m.name); - + let full_param_name = format!("{root_name}.{}", param.name); for (name, type_part) in &tags_to_check { if !is_name_equal(name, &full_param_name) { continue; @@ -284,20 +281,20 @@ fn collect_params(params: &FormalParameters) -> Vec { match get_param_name(&prop.value, false) { ParamKind::Single(param) => { - collected.push(Param { name: name.to_string(), ..param }); + collected.push(Param { name: format!("{name}"), ..param }); } ParamKind::Nested(params) => { collected.push(Param { span: prop.span, - name: name.to_string(), + name: format!("{name}"), is_rest: false, }); for param in params { - let mut name = name.to_string(); - name.push('.'); - name.push_str(¶m.name); - collected.push(Param { name, ..param }); + collected.push(Param { + name: format!("{name}.{}", param.name), + ..param + }); } } } @@ -316,10 +313,7 @@ fn collect_params(params: &FormalParameters) -> Vec { let mut collected = vec![]; for (idx, elm) in arr_pat.elements.iter().enumerate() { - let mut name = String::new(); - name.push('"'); - name.push_str(&idx.to_string()); - name.push('"'); + let name = format!("\"{idx}\""); if let Some(pat) = elm { match get_param_name(pat, false) { From b024c9544353ae9d574c99b75c9466391160c885 Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 17:50:34 +0900 Subject: [PATCH 6/9] Fix perf?: Early return on diagnostic --- .../src/rules/jsdoc/require_param.rs | 40 ++++++++++++------- .../src/snapshots/require_param.snap | 30 +++++++------- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index e5eea33ce7fce..34d5b4674c6c4 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -2,7 +2,7 @@ use oxc_ast::{ ast::{BindingPattern, BindingPatternKind, Expression, FormalParameters, MethodDefinitionKind}, AstKind, }; -use oxc_diagnostics::LabeledSpan; +// use oxc_diagnostics::LabeledSpan; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_semantic::{AstNode, JSDoc}; @@ -20,6 +20,12 @@ use crate::{ }, }; +fn require_param_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters.") + .with_help("Add `@param` tag with name.") + .and_label(span) +} + #[derive(Debug, Default, Clone)] pub struct RequireParam(Box); @@ -162,7 +168,7 @@ impl Rule for RequireParam { let check_types_regex = Regex::new(config.check_types_pattern.as_str()) .expect("`config.checkTypesPattern` should be a valid regex pattern"); - let mut violations = vec![]; + // let mut violations = vec![]; for (idx, param) in params_to_check.iter().enumerate() { match param { ParamKind::Single(param) => { @@ -171,7 +177,9 @@ impl Rule for RequireParam { } if !tags_to_check.iter().any(|(name, _)| **name == param.name) { - violations.push(param.span); + // violations.push(param.span); + ctx.diagnostic(require_param_diagnostic(param.span)); + return; } } ParamKind::Nested(params) => { @@ -226,24 +234,26 @@ impl Rule for RequireParam { .iter() .any(|(name, _)| is_name_equal(name, &full_param_name)) { - violations.push(param.span); + // violations.push(param.span); + ctx.diagnostic(require_param_diagnostic(param.span)); + return; } } } } } - if !violations.is_empty() { - let labels = violations - .iter() - .map(|span| LabeledSpan::new_with_span(None, *span)) - .collect::>(); - ctx.diagnostic( - OxcDiagnostic::warn("eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters.") - .with_help("Add `@param` tag with name.") - .with_labels(labels), - ); - } + // if !violations.is_empty() { + // let labels = violations + // .iter() + // .map(|span| LabeledSpan::new_with_span(None, *span)) + // .collect::>(); + // ctx.diagnostic( + // OxcDiagnostic::warn("eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters.") + // .with_help("Add `@param` tag with name.") + // .with_labels(labels), + // ); + // } } } diff --git a/crates/oxc_linter/src/snapshots/require_param.snap b/crates/oxc_linter/src/snapshots/require_param.snap index e5ae5c9c257db..22da5ff83986a 100644 --- a/crates/oxc_linter/src/snapshots/require_param.snap +++ b/crates/oxc_linter/src/snapshots/require_param.snap @@ -87,7 +87,7 @@ expression: require_param ╭─[require_param.tsx:5:31] 4 │ */ 5 │ function quux ({ foo, bar: { baz }}) { - · ─── ───────────── + · ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -96,7 +96,7 @@ expression: require_param ╭─[require_param.tsx:5:30] 4 │ */ 5 │ function quux ({foo}, {bar}) { - · ─── ─── + · ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -105,7 +105,7 @@ expression: require_param ╭─[require_param.tsx:5:30] 4 │ */ 5 │ function quux ({foo}, {bar}) { - · ─── ─── + · ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -114,7 +114,7 @@ expression: require_param ╭─[require_param.tsx:5:30] 4 │ */ 5 │ function quux ({foo}, {bar}) { - · ─── ─── + · ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -123,7 +123,7 @@ expression: require_param ╭─[require_param.tsx:5:29] 4 │ */ 5 │ function quux (foo, bar) { - · ─── ─── + · ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -141,7 +141,7 @@ expression: require_param ╭─[require_param.tsx:5:29] 4 │ */ 5 │ function quux (foo, bar, baz) { - · ─── ─── + · ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -159,7 +159,7 @@ expression: require_param ╭─[require_param.tsx:5:29] 4 │ */ 5 │ function quux (foo, bar, baz) { - · ─── ─── + · ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -222,7 +222,7 @@ expression: require_param ╭─[require_param.tsx:5:26] 4 │ */ 5 │ function quux ({bar, baz}, foo) { - · ─── ─── ─── + · ─── 6 │ } ╰──── help: Add `@param` tag with name. @@ -231,7 +231,7 @@ expression: require_param ╭─[require_param.tsx:5:25] 4 │ */ 5 │ function quux (foo, {bar, baz}) { - · ─── ─── ─── + · ─── 6 │ } ╰──── help: Add `@param` tag with name. @@ -240,7 +240,7 @@ expression: require_param ╭─[require_param.tsx:5:26] 4 │ */ 5 │ function quux ([bar, baz], foo) { - · ─── ─── ─── + · ─── 6 │ } ╰──── help: Add `@param` tag with name. @@ -339,7 +339,7 @@ expression: require_param ╭─[require_param.tsx:6:39] 5 │ */ 6 │ const bboxToObj = function ({x, y, width, height}) { - · ─ ─ ───── ────── + · ─ 7 │ return {x, y, width, height}; ╰──── help: Add `@param` tag with name. @@ -348,7 +348,7 @@ expression: require_param ╭─[require_param.tsx:6:39] 5 │ */ 6 │ const bboxToObj = function ({x, y, width, height}) { - · ─ ─ ───── ────── + · ─ 7 │ return {x, y, width, height}; ╰──── help: Add `@param` tag with name. @@ -357,7 +357,7 @@ expression: require_param ╭─[require_param.tsx:7:23] 6 │ */ 7 │ fetch = ({ url, ...options }, cacheKey) => { - · ─── ─────── + · ─── 8 │ } ╰──── help: Add `@param` tag with name. @@ -393,7 +393,7 @@ expression: require_param ╭─[require_param.tsx:7:27] 6 │ */ 7 │ function quux ({ foo: { bar } }) {} - · ───────────── + · ──────────── 8 │ ╰──── help: Add `@param` tag with name. @@ -420,7 +420,7 @@ expression: require_param ╭─[require_param.tsx:3:25] 2 │ /** Foo. */ 3 │ function foo(a, b, c) {} - · ─ ─ ─ + · ─ 4 │ ╰──── help: Add `@param` tag with name. From b907e87936e78be33f7cde29ab1559df25006e0b Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 17:57:02 +0900 Subject: [PATCH 7/9] Revert "Fix perf?: Early return on diagnostic" This reverts commit b024c9544353ae9d574c99b75c9466391160c885. --- .../src/rules/jsdoc/require_param.rs | 40 +++++++------------ .../src/snapshots/require_param.snap | 30 +++++++------- 2 files changed, 30 insertions(+), 40 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index 34d5b4674c6c4..e5eea33ce7fce 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -2,7 +2,7 @@ use oxc_ast::{ ast::{BindingPattern, BindingPatternKind, Expression, FormalParameters, MethodDefinitionKind}, AstKind, }; -// use oxc_diagnostics::LabeledSpan; +use oxc_diagnostics::LabeledSpan; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_semantic::{AstNode, JSDoc}; @@ -20,12 +20,6 @@ use crate::{ }, }; -fn require_param_diagnostic(span: Span) -> OxcDiagnostic { - OxcDiagnostic::warn("eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters.") - .with_help("Add `@param` tag with name.") - .and_label(span) -} - #[derive(Debug, Default, Clone)] pub struct RequireParam(Box); @@ -168,7 +162,7 @@ impl Rule for RequireParam { let check_types_regex = Regex::new(config.check_types_pattern.as_str()) .expect("`config.checkTypesPattern` should be a valid regex pattern"); - // let mut violations = vec![]; + let mut violations = vec![]; for (idx, param) in params_to_check.iter().enumerate() { match param { ParamKind::Single(param) => { @@ -177,9 +171,7 @@ impl Rule for RequireParam { } if !tags_to_check.iter().any(|(name, _)| **name == param.name) { - // violations.push(param.span); - ctx.diagnostic(require_param_diagnostic(param.span)); - return; + violations.push(param.span); } } ParamKind::Nested(params) => { @@ -234,26 +226,24 @@ impl Rule for RequireParam { .iter() .any(|(name, _)| is_name_equal(name, &full_param_name)) { - // violations.push(param.span); - ctx.diagnostic(require_param_diagnostic(param.span)); - return; + violations.push(param.span); } } } } } - // if !violations.is_empty() { - // let labels = violations - // .iter() - // .map(|span| LabeledSpan::new_with_span(None, *span)) - // .collect::>(); - // ctx.diagnostic( - // OxcDiagnostic::warn("eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters.") - // .with_help("Add `@param` tag with name.") - // .with_labels(labels), - // ); - // } + if !violations.is_empty() { + let labels = violations + .iter() + .map(|span| LabeledSpan::new_with_span(None, *span)) + .collect::>(); + ctx.diagnostic( + OxcDiagnostic::warn("eslint-plugin-jsdoc(require-param): Missing JSDoc `@param` declaration for function parameters.") + .with_help("Add `@param` tag with name.") + .with_labels(labels), + ); + } } } diff --git a/crates/oxc_linter/src/snapshots/require_param.snap b/crates/oxc_linter/src/snapshots/require_param.snap index 22da5ff83986a..e5ae5c9c257db 100644 --- a/crates/oxc_linter/src/snapshots/require_param.snap +++ b/crates/oxc_linter/src/snapshots/require_param.snap @@ -87,7 +87,7 @@ expression: require_param ╭─[require_param.tsx:5:31] 4 │ */ 5 │ function quux ({ foo, bar: { baz }}) { - · ─── + · ─── ───────────── 6 │ ╰──── help: Add `@param` tag with name. @@ -96,7 +96,7 @@ expression: require_param ╭─[require_param.tsx:5:30] 4 │ */ 5 │ function quux ({foo}, {bar}) { - · ─── + · ─── ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -105,7 +105,7 @@ expression: require_param ╭─[require_param.tsx:5:30] 4 │ */ 5 │ function quux ({foo}, {bar}) { - · ─── + · ─── ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -114,7 +114,7 @@ expression: require_param ╭─[require_param.tsx:5:30] 4 │ */ 5 │ function quux ({foo}, {bar}) { - · ─── + · ─── ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -123,7 +123,7 @@ expression: require_param ╭─[require_param.tsx:5:29] 4 │ */ 5 │ function quux (foo, bar) { - · ─── + · ─── ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -141,7 +141,7 @@ expression: require_param ╭─[require_param.tsx:5:29] 4 │ */ 5 │ function quux (foo, bar, baz) { - · ─── + · ─── ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -159,7 +159,7 @@ expression: require_param ╭─[require_param.tsx:5:29] 4 │ */ 5 │ function quux (foo, bar, baz) { - · ─── + · ─── ─── 6 │ ╰──── help: Add `@param` tag with name. @@ -222,7 +222,7 @@ expression: require_param ╭─[require_param.tsx:5:26] 4 │ */ 5 │ function quux ({bar, baz}, foo) { - · ─── + · ─── ─── ─── 6 │ } ╰──── help: Add `@param` tag with name. @@ -231,7 +231,7 @@ expression: require_param ╭─[require_param.tsx:5:25] 4 │ */ 5 │ function quux (foo, {bar, baz}) { - · ─── + · ─── ─── ─── 6 │ } ╰──── help: Add `@param` tag with name. @@ -240,7 +240,7 @@ expression: require_param ╭─[require_param.tsx:5:26] 4 │ */ 5 │ function quux ([bar, baz], foo) { - · ─── + · ─── ─── ─── 6 │ } ╰──── help: Add `@param` tag with name. @@ -339,7 +339,7 @@ expression: require_param ╭─[require_param.tsx:6:39] 5 │ */ 6 │ const bboxToObj = function ({x, y, width, height}) { - · ─ + · ─ ─ ───── ────── 7 │ return {x, y, width, height}; ╰──── help: Add `@param` tag with name. @@ -348,7 +348,7 @@ expression: require_param ╭─[require_param.tsx:6:39] 5 │ */ 6 │ const bboxToObj = function ({x, y, width, height}) { - · ─ + · ─ ─ ───── ────── 7 │ return {x, y, width, height}; ╰──── help: Add `@param` tag with name. @@ -357,7 +357,7 @@ expression: require_param ╭─[require_param.tsx:7:23] 6 │ */ 7 │ fetch = ({ url, ...options }, cacheKey) => { - · ─── + · ─── ─────── 8 │ } ╰──── help: Add `@param` tag with name. @@ -393,7 +393,7 @@ expression: require_param ╭─[require_param.tsx:7:27] 6 │ */ 7 │ function quux ({ foo: { bar } }) {} - · ──────────── + · ───────────── 8 │ ╰──── help: Add `@param` tag with name. @@ -420,7 +420,7 @@ expression: require_param ╭─[require_param.tsx:3:25] 2 │ /** Foo. */ 3 │ function foo(a, b, c) {} - · ─ + · ─ ─ ─ 4 │ ╰──── help: Add `@param` tag with name. From c203ac450c2dbc9a1c7b7ec3627e79c958c7c25f Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 17:59:49 +0900 Subject: [PATCH 8/9] Fix perf --- crates/oxc_linter/src/rules/jsdoc/require_param.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index e5eea33ce7fce..93bb05bbcc6f4 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -393,8 +393,16 @@ fn should_ignore_as_custom_skip(jsdoc: &JSDoc) -> bool { /// Compare to string param names without quotes /// e.g. `foo."bar"` fn is_name_equal(a: &str, b: &str) -> bool { - a.chars().filter(|c| *c != '"').collect::>() - == b.chars().filter(|c| *c != '"').collect::>() + let mut a_chars = a.chars().filter(|&c| c != '"'); + let mut b_chars = b.chars().filter(|&c| c != '"'); + + loop { + match (a_chars.next(), b_chars.next()) { + (Some(ac), Some(bc)) if ac == bc => continue, + (None, None) => return true, // Both done + _ => return false, // Either one is done + } + } } #[test] From d825ff26ed4667c3c9c1bf63b369e36a3faeff5d Mon Sep 17 00:00:00 2001 From: Yuji Sugiura Date: Thu, 6 Jun 2024 23:17:02 +0900 Subject: [PATCH 9/9] Fix perf: use cache for regex --- .../src/rules/jsdoc/require_param.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index 93bb05bbcc6f4..28d3a64fb7d21 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -1,3 +1,4 @@ +use lazy_static::lazy_static; use oxc_ast::{ ast::{BindingPattern, BindingPatternKind, Expression, FormalParameters, MethodDefinitionKind}, AstKind, @@ -8,8 +9,9 @@ use oxc_macros::declare_oxc_lint; use oxc_semantic::{AstNode, JSDoc}; use oxc_span::Span; use regex::Regex; -use rustc_hash::FxHashSet; +use rustc_hash::{FxHashMap, FxHashSet}; use serde::Deserialize; +use std::sync::Mutex; use crate::{ context::LintContext, @@ -90,6 +92,11 @@ fn default_check_types_pattern() -> String { "^(?:[oO]bject|[aA]rray|PlainObject|Generic(?:Object|Array))$".to_string() // spellchecker:disable-line } +// For perf, cache regex is needed +lazy_static! { + static ref REGEX_CACHE: Mutex> = Mutex::new(FxHashMap::default()); +} + impl Rule for RequireParam { fn from_configuration(value: serde_json::Value) -> Self { value @@ -159,8 +166,12 @@ impl Rule for RequireParam { let shallow_tags = tags_to_check.iter().filter(|(name, _)| !name.contains('.')).collect::>(); - let check_types_regex = Regex::new(config.check_types_pattern.as_str()) - .expect("`config.checkTypesPattern` should be a valid regex pattern"); + let mut regex_cache = REGEX_CACHE.lock().unwrap(); + let check_types_regex = + regex_cache.entry(config.check_types_pattern.clone()).or_insert_with(|| { + Regex::new(config.check_types_pattern.as_str()) + .expect("`config.checkTypesPattern` should be a valid regex pattern") + }); let mut violations = vec![]; for (idx, param) in params_to_check.iter().enumerate() { @@ -400,7 +411,7 @@ fn is_name_equal(a: &str, b: &str) -> bool { match (a_chars.next(), b_chars.next()) { (Some(ac), Some(bc)) if ac == bc => continue, (None, None) => return true, // Both done - _ => return false, // Either one is done + _ => return false, // Either one is done, or not equal } } }