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

False positive with no-multi-asterisks #773

Closed
Drarig29 opened this issue Jul 23, 2021 · 6 comments · Fixed by #790
Closed

False positive with no-multi-asterisks #773

Drarig29 opened this issue Jul 23, 2021 · 6 comments · Fixed by #790

Comments

@Drarig29
Copy link
Contributor

Drarig29 commented Jul 23, 2021

Expected behavior

With the default rules (no-multi-asterisks enabled), the following JSDoc should pass the linting:

/**
 * Some description here.
 *
 * **Example:** An example here.
 */

The bold text in Markdown should stay.

Actual behavior

The plugin auto-fixes the JSDoc like the following:

/**
 * Some description here.
 *
 * Example:** An example here.
 */

ESLint Config

Just the defaults.

module.exports = {
  parser: '@typescript-eslint/parser', // Specifies the ESLint parser
  parserOptions: {
    ecmaVersion: 2020, // Allows for the parsing of modern ECMAScript features
    sourceType: 'module', // Allows for the use of imports
  },
  ignorePatterns: ['dist/', 'webpack.config.js'],
  extends: [
    'plugin:@typescript-eslint/recommended',
    'plugin:jsdoc/recommended',
  ],
}

ESLint sample

/**
 * Some description here.
 *
 * **Example:** An example here.
 */
function foo() {

}

Environment

  • Node version: v14.15.5
  • ESLint version: v7.31.0
  • eslint-plugin-jsdoc version: 35.5.1
@brettz9
Copy link
Collaborator

brettz9 commented Jul 23, 2021

We can look to overcome this for cases where the asterisk count is even, e.g.,: 4 (extra asterisks) % 2 = 0.

/**
 * **bold** 
 */

However, I'm not sure we want to get into the harder and more error prone area of trying to fix cases like this:

/**
 * Here's a list:
 * * item 1
 * * item 2 
 */

I think in the latter cases, we should just recommend people use the equally valid Markdown operator - in place of *. (Getting in the habit of using this operator also has the advantage that naive syntax highlighters do not consider the asterisk as the beginning of a bold statement.)

@alvis
Copy link

alvis commented Oct 7, 2021

Can we simply only count leading/ending asterisks without spaces?
e.g.

// OK
/**
 * Here's a list:
 * * item 1
 * * item 2 
 *  **BOLD**
 */

// FAIL
/**
 ** Here's a list:
 ** * item 1
 ** * item 2 
 ** **BOLD**
 */

@Drarig29
Copy link
Contributor Author

Drarig29 commented Oct 7, 2021

I will work on this issue.

@Drarig29
Copy link
Contributor Author

Drarig29 commented Oct 7, 2021

Can we simply only count leading/ending asterisks without spaces? e.g.

// OK
/**
 * Here's a list:
 * * item 1
 * * item 2 
 *  **BOLD**
 */

// FAIL
/**
 ** Here's a list:
 ** * item 1
 ** * item 2 
 ** **BOLD**
 */

You are missing the case (one of my use) where you have a line not ending with the end of the bold block. For example:

/**
 ** Here's a list:
 ** * item 1
 ** * item 2
 ** **Note:** Hello there.
 */

@alvis
Copy link

alvis commented Oct 7, 2021

Aren't they all fail?

/**
 ** Here's a list:
 ** * item 1
 ** * item 2
 ** **Note:** Hello there.
 */

Actually out use case is about a file header like the following

/*
 *                           *** INTERNAL ONLY ***
 * -------------------------------------------------------------------------
 * Unauthorized copying of this file, via any medium is strictly prohibited.
 * -------------------------------------------------------------------------
 *
 * @summary   ...
 *
 * @license   Proprietary
 * @copyright Copyright (c) 2021 - All Rights Reserved.
 * -------------------------------------------------------------------------
 */

The easiest way to fix it of cause is us changing the format, but it wasn't a problem and I don't see it should be a problem.
The current rule is just too sensitive.

@gajus
Copy link
Owner

gajus commented Oct 26, 2021

🎉 This issue has been resolved in version 37.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gajus gajus added the released label Oct 26, 2021
Drarig29 added a commit to Drarig29/brackets-manager.js that referenced this issue Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants