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

Rule "check-examples" should use parser given in eslint configuration #434

Closed
Elberet opened this issue Nov 19, 2019 · 13 comments · Fixed by #470
Closed

Rule "check-examples" should use parser given in eslint configuration #434

Elberet opened this issue Nov 19, 2019 · 13 comments · Fixed by #470

Comments

@Elberet
Copy link

Elberet commented Nov 19, 2019

Linting source code with certain experimental features (e.g. static class properties via Babel and its proposal-class-properties plugin) can be achieved by using the Babel parser within eslint via babel-eslint. However, the check-examples rule apparently does not use the configured parser and fails to lint @example code fragments.

Contrived and stripped example:

/**
 * @example
 * class Derived extends Base {
 *   static [Base.Prop] = "value"
 * }
 */
class Base {
  static Prop = Symbol()
}

class Derived extends Base {
  static [Base.Prop] = "value"
}

Eslint reports a single warning by jsdoc/check-examples:
@example error: Parsing error: Unexpected token =

.eslintrc.js:

module.exports = {
  parser: "babel-eslint",
  parserOptions: {
    ecmaVersion: 8,
    sourceType: "module",
    babelOptions: {
      configFile: "./babel-eslint-options.js"
    }
  }
}

babel-eslint-options.js:

module.exports = {
  plugins: [
    ["@babel/proposal-class-properties"]
  ],
  presets: [
    ["@babel/env", { targets: "since 2015, ie >= 11" }]
  }
}
@brettz9
Copy link
Collaborator

brettz9 commented Nov 19, 2019

Have you looked yet at the README docs?

@Elberet
Copy link
Author

Elberet commented Nov 19, 2019

I have - but I understood the section on configuring check-examples to allow specifying a linter configuration other than the currently active one. Or in other words, I assumed that in the absence of any configuration options, the eslint config for the file containing the jsdoc comment would also apply to the @example, minus the rules cited in the README.

I'll admit that I didn't read the README word by word, so if there is an explanation that examples are linted against a fixed set of default rules ignoring the ambient eslint configuration, it should probably be more overt.

Additionally, there might be a compounding issue at play here: only a subtree of my project is being linted with the babel parser and thus has a nested eslint config file...

@brettz9
Copy link
Collaborator

brettz9 commented Nov 20, 2019

What check-examples options were you using when you got the error?

@brettz9
Copy link
Collaborator

brettz9 commented Nov 20, 2019

While we could try to pick up the parser config by default, we are picking up the parser config when there is a matchingFileName, so I would suggest your giving it one. This does not need to point to a real file; it is just a dummy so that ESLint will know which rules to apply, based on the file extension and file location (for ESLint configs). Once you have that, you don't need to manually specify the parser, etc. My personal recommendation is to give it a file name with an "md" extension, since the typical rules one may wish for @example are often similar to those if linting one's JavaScript in Markdown (as through eslint-plugin-markdown); i.e., neither tend to have a full context, they do not need 'use strict', do not need to report missing end-of-file line breaks, etc.

(I can't seem to get an error by having a bad babelOptions configFile, so I'm not sure whether all of this is picked up; but since babel-eslint alone can parse class properties, you will avoid the error just by getting babel-eslint to apply.)

@Elberet
Copy link
Author

Elberet commented Nov 20, 2019

I see, thanks for the explanation. I've extended my eslint and json/check-examples config and have gotten rid of the unwanted error message. 👍

Still, this behaviour feels needlessly surprising. I wonder how many users actually code in one dialect but write JSDoc comments from the perspective of a (library?) consumer. Should a Typescript developer really be expected to write examples in ES5 syntax?

@brettz9
Copy link
Collaborator

brettz9 commented Nov 20, 2019

Yes, I think it is a reasonable addition to add. There may be somewhat of a performance cost, however, in always checking the file system (the linter checks the file system, even while the file doesn't need to exist). (I suppose there could be an option to opt-out.)

@Elberet
Copy link
Author

Elberet commented Nov 20, 2019

If the Linter instance can be cached and reused for identical matchingFileName values, the performance cost would be one additional config load per directory that contains any linted file with at least one @example. Doesn't sound too too terrible to me.

@brettz9 brettz9 self-assigned this Dec 29, 2019
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 30, 2019
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 30, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 30, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 30, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority and exits faster if no `example` tag
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 31, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority, exits faster if no `example` tag, and avoids retrieving file config if `eslintrcForExamples` is `false`
@brettz9
Copy link
Collaborator

brettz9 commented Dec 31, 2019

@Elberet : I'd appreciate if you'd take a look at #470 (at least the docs and/or PR explanation) to see what you think of the proposed new behavior/changes.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 31, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority, exits faster if no `example` tag, and avoids retrieving file config if `eslintrcForExamples` is `false`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 31, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority, exits faster if no `example` tag, and avoids retrieving file config if `eslintrcForExamples` is `false`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 31, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority, exits faster if no `example` tag, and avoids retrieving file config if `eslintrcForExamples` is `false`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 31, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority, exits faster if no `example` tag, and avoids retrieving file config if `eslintrcForExamples` is `false`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Dec 31, 2019
…cript file (though with `md` extension for easier automated overriding); fixes gajus#434

Also ensures that `baseConfig` is treated as lower priority, exits faster if no `example` tag, and avoids retrieving file config if `eslintrcForExamples` is `false`
brettz9 added a commit that referenced this issue Jan 2, 2020
…cript file (though with `md` extension for easier automated overriding); fixes #434

Also ensures that `baseConfig` is treated as lower priority, exits faster if no `example` tag, and avoids retrieving file config if `eslintrcForExamples` is `false`
@gajus
Copy link
Owner

gajus commented Jan 2, 2020

🎉 This issue has been resolved in version 20.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gajus gajus added the released label Jan 2, 2020
@Elberet
Copy link
Author

Elberet commented Jan 2, 2020

Hey @brettz9, my apologies. I would have loved to give feedback, but I'm out of the office until the 7th. 😉 But I'll definitely check out the new version! 👍

@brettz9
Copy link
Collaborator

brettz9 commented Jan 2, 2020

No worries! Hope it works well for ya....

@dannyfritz
Copy link

dannyfritz commented Oct 28, 2021

I'm having a really hard time figuring this one out.

I have a TypeScript file that I want to write TypeScript examples for. I always get this error:

/**
 * Extracts the internal data type `T` from a `ThunkHookTuple<T>`
 *
 * @example
 * type RetailStoreHook = ThunkHookTuple<Array<RetailStore>>;
 * type RetailStoreHookType = ThunkHookType<RetailStoreHook>; // Array<RetailStore>
 */
export type ThunkHookType<T> = T extends ThunkHookTuple<infer U> & [unknown, typeof FETCH_SUCCESS, undefined] ? U : never;
08:10:17    0:0  error  @example error: Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
08:10:17  The file does not match your project config: <text>.
08:10:17  The extension for the file () is non-standard. You should add "parserOptions.extraFileExtensions" to your config  jsdoc/check-examples

I've tried messing around with matchingFileName and baseConfig with parserOptions, but I can't seem to figure out a configuration that works.

@brettz9
Copy link
Collaborator

brettz9 commented Nov 3, 2021

@dannyfritz : Please file a new issue.

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