Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
pasha-zayko opened this issue Aug 12, 2024 · 9 comments · Fixed by #1309
Closed

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

pasha-zayko opened this issue Aug 12, 2024 · 9 comments · Fixed by #1309

Comments

@pasha-zayko
Copy link

pasha-zayko commented Aug 12, 2024

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

  • Node version: 20.16.0
  • ESLint version 9.8.0
  • eslint-plugin-jsdoc version: 48.5.0+
@brettz9
Copy link
Collaborator

brettz9 commented Aug 12, 2024

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

@brettz9
Copy link
Collaborator

brettz9 commented Aug 12, 2024

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

@pasha-zayko
Copy link
Author

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
Copy link
Collaborator

brettz9 commented Aug 12, 2024

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
Copy link

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
Copy link
Collaborator

brettz9 commented Aug 14, 2024

@sugat009 : Can you show your config?

@sugat009
Copy link

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 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Aug 14, 2024
…ion nodes so that non-function global contexts do not err; fixes gajus#1303

Also adds warning in docs for use of global settings contexts.
@brettz9
Copy link
Collaborator

brettz9 commented Aug 14, 2024

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.

Copy link

🎉 This issue has been resolved in version 50.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment