From d6370f19fe770d667d2fb0c4dffc6863da693caa Mon Sep 17 00:00:00 2001
From: Yuji Sugiura <6259812+leaysgur@users.noreply.github.com>
Date: Mon, 10 Jun 2024 18:23:41 +0900
Subject: [PATCH] feat(linter/jsdoc): Implement require-param-type rule (#3601)
Part of #1170
>
https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/require-param-type.md
---
crates/oxc_linter/src/rules.rs | 2 +
.../src/rules/jsdoc/require_param.rs | 122 +--------
.../src/rules/jsdoc/require_param_type.rs | 255 ++++++++++++++++++
.../src/snapshots/require_param_type.snap | 48 ++++
crates/oxc_linter/src/utils/jsdoc.rs | 118 +++++++-
5 files changed, 425 insertions(+), 120 deletions(-)
create mode 100644 crates/oxc_linter/src/rules/jsdoc/require_param_type.rs
create mode 100644 crates/oxc_linter/src/snapshots/require_param_type.snap
diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs
index db30b32abc919..134610583b627 100644
--- a/crates/oxc_linter/src/rules.rs
+++ b/crates/oxc_linter/src/rules.rs
@@ -391,6 +391,7 @@ mod jsdoc {
pub mod implements_on_classes;
pub mod no_defaults;
pub mod require_param;
+ pub mod require_param_type;
pub mod require_property;
pub mod require_property_description;
pub mod require_property_name;
@@ -759,6 +760,7 @@ oxc_macros::declare_all_lint_rules! {
jsdoc::implements_on_classes,
jsdoc::no_defaults,
jsdoc::require_param,
+ jsdoc::require_param_type,
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
index 9dd8f40397c5a..e786060b11efd 100644
--- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs
+++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs
@@ -1,13 +1,9 @@
use lazy_static::lazy_static;
-use oxc_ast::{
- ast::{BindingPattern, BindingPatternKind, Expression, FormalParameters, MethodDefinitionKind},
- AstKind,
-};
+use oxc_ast::{ast::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::{FxHashMap, FxHashSet};
use serde::Deserialize;
@@ -17,8 +13,8 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
- get_function_nearest_jsdoc_node, should_ignore_as_avoid, should_ignore_as_internal,
- should_ignore_as_private,
+ collect_params, get_function_nearest_jsdoc_node, should_ignore_as_avoid,
+ should_ignore_as_internal, should_ignore_as_private, ParamKind,
},
};
@@ -258,118 +254,6 @@ impl Rule for RequireParam {
}
}
-#[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 Some(name) = prop.key.name() else { continue };
-
- 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,
diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param_type.rs b/crates/oxc_linter/src/rules/jsdoc/require_param_type.rs
new file mode 100644
index 0000000000000..576bccce7d37a
--- /dev/null
+++ b/crates/oxc_linter/src/rules/jsdoc/require_param_type.rs
@@ -0,0 +1,255 @@
+use crate::{
+ context::LintContext,
+ rule::Rule,
+ utils::{
+ collect_params, get_function_nearest_jsdoc_node, should_ignore_as_internal,
+ should_ignore_as_private, ParamKind,
+ },
+ AstNode,
+};
+use oxc_ast::AstKind;
+use oxc_diagnostics::OxcDiagnostic;
+use oxc_macros::declare_oxc_lint;
+use oxc_span::Span;
+
+fn missing_type_diagnostic(span0: Span) -> OxcDiagnostic {
+ OxcDiagnostic::warn("eslint-plugin-jsdoc(require-param-type): Missing JSDoc `@param` type.")
+ .with_help("Add {type} to `@param` tag.")
+ .with_labels([span0.into()])
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct RequireParamType;
+
+declare_oxc_lint!(
+ /// ### What it does
+ /// Requires that each `@param` tag has a type value (within curly brackets).
+ ///
+ /// ### Why is this bad?
+ /// The type of a parameter should be documented.
+ ///
+ /// ### Example
+ /// ```javascript
+ /// // Passing
+ /// /** @param {SomeType} foo */
+ /// function quux (foo) {}
+ ///
+ /// // Failing
+ /// /** @param foo */
+ /// function quux (foo) {}
+ /// ```
+ RequireParamType,
+ pedantic,
+);
+
+impl Rule for RequireParamType {
+ fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+ // Collected targets from `FormalParameters`
+ let params_to_check = match node.kind() {
+ 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,
+ };
+
+ // If no JSDoc is found, skip
+ let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx)
+ .and_then(|node| ctx.jsdoc().get_all_by_node(node))
+ else {
+ return;
+ };
+
+ let settings = &ctx.settings().jsdoc;
+ let resolved_param_tag_name = settings.resolve_tag_name("param");
+
+ let mut root_count = 0;
+ for jsdoc in jsdocs
+ .iter()
+ .filter(|jsdoc| !should_ignore_as_internal(jsdoc, settings))
+ .filter(|jsdoc| !should_ignore_as_private(jsdoc, settings))
+ {
+ for tag in jsdoc.tags() {
+ if tag.kind.parsed() != resolved_param_tag_name {
+ continue;
+ }
+
+ let (type_part, name_part, _) = tag.type_name_comment();
+
+ if name_part.is_some_and(|name_part| !name_part.parsed().contains('.')) {
+ root_count += 1;
+ }
+ if settings.exempt_destructured_roots_from_checks {
+ // -1 for count to idx conversion
+ if let Some(ParamKind::Nested(_)) = params_to_check.get(root_count - 1) {
+ continue;
+ }
+ }
+
+ // If type exists, skip
+ if type_part.is_some() {
+ continue;
+ };
+
+ ctx.diagnostic(missing_type_diagnostic(tag.kind.span));
+ }
+ }
+ }
+}
+
+#[test]
+fn test() {
+ use crate::tester::Tester;
+
+ let pass = vec![
+ (
+ "
+ /**
+ *
+ */
+ function quux (foo) {
+
+ }
+ ",
+ None,
+ None,
+ ),
+ (
+ "
+ /**
+ * @param {number} foo
+ */
+ function quux (foo) {
+
+ }
+ ",
+ None,
+ None,
+ ),
+ (
+ "
+ /**
+ * @function
+ * @param foo
+ */
+ ",
+ None,
+ None,
+ ),
+ (
+ "
+ /**
+ * @callback
+ * @param foo
+ */
+ ",
+ None,
+ None,
+ ),
+ (
+ "
+ /**
+ * @param {number} foo
+ * @param root
+ * @param {boolean} baz
+ */
+ function quux (foo, {bar}, baz) {
+
+ }
+ ",
+ None,
+ Some(
+ serde_json::json!({ "settings": { "jsdoc": { "exemptDestructuredRootsFromChecks": true, }, } }),
+ ),
+ ),
+ (
+ "
+ /**
+ * @param {number} foo
+ * @param root
+ * @param root.bar
+ */
+ function quux (foo, {bar: {baz}}) {
+
+ }
+ ",
+ None,
+ Some(
+ serde_json::json!({ "settings": { "jsdoc": { "exemptDestructuredRootsFromChecks": true, }, } }),
+ ),
+ ),
+ ];
+
+ let fail = vec![
+ (
+ "
+ /**
+ * @param foo
+ */
+ function quux (foo) {
+
+ }
+ ",
+ None,
+ None,
+ ),
+ (
+ "
+ /**
+ * @param {a xxx
+ */
+ function quux () {
+ }
+ ",
+ None,
+ None,
+ ),
+ (
+ "
+ /**
+ * @arg foo
+ */
+ function quux (foo) {
+
+ }
+ ",
+ None,
+ Some(
+ serde_json::json!({ "settings": { "jsdoc": { "tagNamePreference": { "param": "arg", }, }, } }),
+ ),
+ ),
+ (
+ "
+ /**
+ * @param {number} foo
+ * @param root
+ * @param {boolean} baz
+ */
+ function quux (foo, {bar}, baz) {
+
+ }
+ ",
+ Some(
+ serde_json::json!([ { "setDefaultDestructuredRootType": true, }, ]),
+ ),
+ None,
+ ),
+ (
+ "
+ /**
+ * @param {number} foo
+ * @param root
+ * @param {boolean} baz
+ */
+ function quux (foo, {bar}, baz) {
+
+ }
+ ",
+ Some(
+ serde_json::json!([ { "setDefaultDestructuredRootType": false, }, ]),
+ ),
+ None,
+ ),
+ ];
+
+ Tester::new(RequireParamType::NAME, pass, fail).test_and_snapshot();
+}
diff --git a/crates/oxc_linter/src/snapshots/require_param_type.snap b/crates/oxc_linter/src/snapshots/require_param_type.snap
new file mode 100644
index 0000000000000..98e7f68e6beab
--- /dev/null
+++ b/crates/oxc_linter/src/snapshots/require_param_type.snap
@@ -0,0 +1,48 @@
+---
+source: crates/oxc_linter/src/tester.rs
+expression: require_param_type
+---
+ ⚠ eslint-plugin-jsdoc(require-param-type): Missing JSDoc `@param` type.
+ ╭─[require_param_type.tsx:3:17]
+ 2 │ /**
+ 3 │ * @param foo
+ · ──────
+ 4 │ */
+ ╰────
+ help: Add {type} to `@param` tag.
+
+ ⚠ eslint-plugin-jsdoc(require-param-type): Missing JSDoc `@param` type.
+ ╭─[require_param_type.tsx:3:13]
+ 2 │ /**
+ 3 │ * @param {a xxx
+ · ──────
+ 4 │ */
+ ╰────
+ help: Add {type} to `@param` tag.
+
+ ⚠ eslint-plugin-jsdoc(require-param-type): Missing JSDoc `@param` type.
+ ╭─[require_param_type.tsx:3:17]
+ 2 │ /**
+ 3 │ * @arg foo
+ · ────
+ 4 │ */
+ ╰────
+ help: Add {type} to `@param` tag.
+
+ ⚠ eslint-plugin-jsdoc(require-param-type): Missing JSDoc `@param` type.
+ ╭─[require_param_type.tsx:4:17]
+ 3 │ * @param {number} foo
+ 4 │ * @param root
+ · ──────
+ 5 │ * @param {boolean} baz
+ ╰────
+ help: Add {type} to `@param` tag.
+
+ ⚠ eslint-plugin-jsdoc(require-param-type): Missing JSDoc `@param` type.
+ ╭─[require_param_type.tsx:4:17]
+ 3 │ * @param {number} foo
+ 4 │ * @param root
+ · ──────
+ 5 │ * @param {boolean} baz
+ ╰────
+ help: Add {type} to `@param` tag.
diff --git a/crates/oxc_linter/src/utils/jsdoc.rs b/crates/oxc_linter/src/utils/jsdoc.rs
index 3d54d3877903b..3c3a6a0401c35 100644
--- a/crates/oxc_linter/src/utils/jsdoc.rs
+++ b/crates/oxc_linter/src/utils/jsdoc.rs
@@ -1,6 +1,10 @@
use crate::{config::JSDocPluginSettings, context::LintContext, AstNode};
-use oxc_ast::AstKind;
+use oxc_ast::{
+ ast::{BindingPattern, BindingPatternKind, Expression, FormalParameters},
+ AstKind,
+};
use oxc_semantic::JSDoc;
+use oxc_span::Span;
use rustc_hash::FxHashSet;
/// JSDoc is often attached on the parent node of a function.
@@ -113,3 +117,115 @@ pub fn should_ignore_as_avoid(
jsdoc.tags().iter().any(|tag| ignore_tag_names.contains(tag.kind.parsed()))
}
+
+#[derive(Debug, Clone)]
+pub struct Param {
+ pub span: Span,
+ pub name: String,
+ pub is_rest: bool,
+}
+
+#[derive(Debug, Clone)]
+pub enum ParamKind {
+ Single(Param),
+ Nested(Vec),
+}
+
+pub 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 Some(name) = prop.key.name() else { continue };
+
+ 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
+}