gajus / eslint-plugin-jsdoc

JSDoc specific linting rules for ESLint.
Other
1.1k stars 160 forks source link

Matching between @param definition and function parameters no longer works #1303

Closed pasha-zayko closed 3 months ago

pasha-zayko commented 3 months ago

Upgrading from version 48.4.0 to anything later no longer matches @param value with variable in the function definition when trying to use 'ExportNamedDeclaration' to define exclusions in global context. The function affected by this is being exported, and the message indicates "@param "xxx" does not match an existing function parameter jsdoc/check-param-names". This issue appears to be related to the change that introduced "contextDetaults" in checkParamNames for version 48.5.0:

adding these defaults

contextDefaults: [
    'ArrowFunctionExpression', 'FunctionDeclaration', 'FunctionExpression', 'TSDeclareFunction',
    // Add this to above defaults
    'TSMethodSignature'
  ],

as the list of variables from function definition is not getting generated. Removing these lines restores the expected behavior.

/**
 * Description of the function.
 * @param inputId Planned value as integer to be evaluated.
 */
export function validateId(inputIds: number): boolean {

Expected behavior

When there is mismatch on the variable name between @param definition and function definition, the following message would be provided: Expected @param names to be "inputIds". Got "inputId" jsdoc/check-param-names

Actual behavior

Without any change in configuration but using version 48.5.0 or higher results in this message instead: @param "inputId" does not match an existing function parameter jsdoc/check-param-names

As mentioned earlier, the change in behavior can be traced to the addition of the contextDefaults that interfere with calculation of function variables

ESLint Config

What is the minimal config that reproduces the issue? As long as 'ExportNamedDeclaration' present the incorrect error message is returned.

import eslint from '@eslint/js';
import globals from 'globals';
import jsdoc from 'eslint-plugin-jsdoc';
import tseslint from 'typescript-eslint';

export default tseslint.config(
    eslint.configs.recommended,
    jsdoc.configs['flat/recommended-typescript'],
    ...tseslint.configs.strictTypeChecked,
    ...tseslint.configs.stylisticTypeChecked,
    {
        'ignores': [
            'eslint.config.mjs'
        ]
    },
    {
        'plugins': { jsdoc },
        'languageOptions': {
            'globals': {
                ...globals.node,
                ...globals.es2021
            },
            'parserOptions': { 'project': true }
        },
        'settings': {
            'jsdoc': {
                'contexts': [
                    'ExportNamedDeclaration:not(:has(VariableDeclarator[id.type=\'ArrayPattern\'])):not(:has(VariableDeclarator[id.type=\'ObjectPattern\']))'
                ]
            }
        }
    }
);

Not using context for 'ExportNamedDeclaration' would provide expected message about the name mismatch.

Environment

brettz9 commented 3 months ago

Do you recall what rules you were using settings.jsdoc.contexts for?

brettz9 commented 3 months ago

If it was for no-restricted-syntax, can you not use the contexts option with that rule instead of using the setting?

pasha-zayko commented 3 months ago

The intention was to apply the contexts across any and all jsdoc-related rules, and I see this even when no rules defined at all in the minimal config file.

At the moment the list of rules in use is this

'jsdoc/check-alignment': 'warn',
'jsdoc/check-indentation': [
    'warn',
    {
        'excludeTags': [
            'example',
            'returns'
        ]
    }
],
'jsdoc/informative-docs': 'warn',
'jsdoc/no-blank-blocks': 'warn',
'jsdoc/no-multi-asterisks': [
    'warn',
    {
        'allowWhitespace': true
    }
],
'jsdoc/require-asterisk-prefix': 'warn',
'jsdoc/require-description-complete-sentence': [
    'warn',
    {
        'abbreviations': ['etc.', 'e.g.', 'i.e.']
    }
],
'jsdoc/require-jsdoc': [
    'warn',
    {
        'require': {
            'ArrowFunctionExpression': true,
            'ClassDeclaration': true,
            'ClassExpression': true,
            'FunctionDeclaration': true,
            'FunctionExpression': true,
            'MethodDefinition': true
        }
    }
],
'jsdoc/require-param': 'warn',
'jsdoc/valid-types': 'warn',
brettz9 commented 3 months ago

Ok, but what are you trying to do with the export declarations? You said, "trying to use 'ExportNamedDeclaration' to define exclusions in global context". Which rules specifically do you need the contexts on?

Defining exclusions suggests you are using the rule jsdoc/no-restricted-syntax. If so, I think that is probably a rule we shouldn't have granted access to the global settings.jsdoc.contexts and it is better to just use the rule options for that. I don't think it is useful to reuse exclusions because I don't believe we have any other rules which exclude contexts.

If you were trying to apply rules to those contexts, e.g., let's say if you always documented variable declarations, then that would be a good cause to use the global settings. (There might still be a bug with check-param-names in such a case, but I'm just trying to see how things should be before identifying the specific problem.)

sugat009 commented 3 months ago

Not sure if these are related issues but I'm getting the same error for the following code. eslint is raising an issue for the c param stating that:

error  @param "c" does not match an existing function parameter           jsdoc/check-param-names
/**
 * Outer
 * @param a a
 * @param b b
 * @returns something
 */
export const outer = (
  a: number,
  b: string
): typeof inner => {
  console.log(a);
  console.log(b);

  /**
   * Inner
   * @param c c
   * @param d d
   */
  const inner = (c: number, d: string): void => {
    console.log(c);
    console.log(d);
  };
  return inner;
};

Environment:

Node version: v20.15.1
ESLint version: 8.49.0
eslint-plugin-jsdoc version: 48.2.5
brettz9 commented 3 months ago

@sugat009 : Can you show your config?

sugat009 commented 3 months ago

yeah, here you go.

module.exports = {
  overrides: [
    {
      files: ['*.ts'],
      extends: [
        // https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/strict-type-checked.ts
        'plugin:@typescript-eslint/strict-type-checked',
        // https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/stylistic-type-checked.ts
        'plugin:@typescript-eslint/stylistic-type-checked',
        'plugin:jsdoc/recommended-typescript-error',
        'plugin:compat/recommended',
      ],
      parser: '@typescript-eslint/parser',
      plugins: ['@typescript-eslint', 'jsdoc', 'compat'],
      parserOptions: {
        project: 'tsconfig.json',
        tsconfigRootDir: __dirname
      },
      settings: {
        jsdoc: {
          contexts: [
            'VariableDeclaration',
            'TSInterfaceDeclaration',
            'TSTypeAliasDeclaration',
            'TSEnumDeclaration',
            'TSMethodSignature'
          ]
        }
      },
      rules: {
        ['@typescript-eslint/explicit-module-boundary-types']: ['error', { allowedNames: ['getDatasource'] }],
        ['@typescript-eslint/no-confusing-void-expression']: ['error', { ignoreArrowShorthand: true }],
        ['@typescript-eslint/no-empty-interface']: ['error', { allowSingleExtends: true }],
        ['@typescript-eslint/no-namespace']: 'off',
        ['@typescript-eslint/no-non-null-assertion']: 'off',
        ['jsdoc/require-jsdoc']: ['error', {
          require: {
            ArrowFunctionExpression: true,
            ClassDeclaration: true,
            ClassExpression: true,
            FunctionDeclaration: true,
            FunctionExpression: true,
            MethodDefinition: true,
          },
          publicOnly: true,
        }]
      }
    }
  ]
};
brettz9 commented 3 months ago

Ok, there were two problems here.

One is that check-param-names should not have been checking anything except function contexts, so if the user supplies some for their contexts, it should ignore them. PR #1309 fixed this.

But another problem is that I think people are using global settings contexts in a way which is harmful, so I've tried to clarify things in the documentation.

Using global settings contexts means that your global contexts will replace the defaults. If you only have VariableDeclaration in there, and no FunctionDeclaration, function declarations will not be checked! This is a concern for other rules, too, whether require-param or others. Do not use global settings contexts unless you are clear that they replace the defaults across a variety of rules.

github-actions[bot] commented 3 months ago

:tada: This issue has been resolved in version 50.2.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: