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

feat: languageOptions.parser must be an object. #16985

Merged
merged 3 commits into from Mar 16, 2023
Merged

feat: languageOptions.parser must be an object. #16985

merged 3 commits into from Mar 16, 2023

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Mar 14, 2023

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[x] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Changes languageOptions.parser to disallow string values. Now parsers must always be an object.

Fixes #16930

Is there anything you'd like reviewers to focus on?

Do the test and documentation updates make sense?

Changes languageOptions.parser to disallow string values.

Fixes #16930
@nzakas nzakas requested a review from a team as a code owner March 14, 2023 19:09
@eslint-github-bot eslint-github-bot bot added triage An ESLint team member will look at this issue soon feature This change adds a new feature to ESLint labels Mar 14, 2023
@netlify
Copy link

netlify bot commented Mar 14, 2023

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 1a360e6
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/64111cddd94a5300071b4b3c

@fasttime fasttime added accepted There is consensus among the team that this change meets the criteria for inclusion enhancement This change enhances an existing feature of ESLint core Relates to ESLint's core APIs and features and removed triage An ESLint team member will look at this issue soon labels Mar 14, 2023
Copy link
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I left a comment in lib/config/flat-config-schema.js. And do we still need this?

if (typeof parser === "string") {
const { pluginName, objectName: localParserName } = splitPluginIdentifier(parser);
parserName = parser;
if (!plugins || !plugins[pluginName] || !plugins[pluginName].parsers || !plugins[pluginName].parsers[localParserName]) {
throw new TypeError(`Key "parser": Could not find "${localParserName}" in plugin "${pluginName}".`);
}
languageOptions.parser = plugins[pluginName].parsers[localParserName];

typeof value !== "object" ||
(typeof value.parse !== "function" && typeof value.parseForESLint !== "function")
) {
throw new TypeError("Expected object with parse() or parseForESLint() method.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make sure that this error is also thrown when the parser (value) is null? I'm currently getting Cannot read properties of null (reading 'parse').

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, nice catch! Absolutely.

@@ -192,17 +192,7 @@ class FlatConfigArray extends ConfigArray {
if (languageOptions && languageOptions.parser) {
const { parser } = languageOptions;

if (typeof parser === "string") {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fasttime another nice catch. 👍 This was never being called because the schema errors before it ever gets here.

@mdjermanovic
Copy link
Member

I'll close-reopen to include 69bc0e2

@snitin315
Copy link
Contributor

I believe we will need to rebase.

@mdjermanovic
Copy link
Member

I believe we will need to rebase.

It looks fine now, the tests run on Node 19.7.0?

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! Leaving open for @fasttime to verify.

Copy link
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Will merge this tomorrow if there are no other comments. Thanks again @nzakas.

Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@snitin315 snitin315 merged commit 892e6e5 into main Mar 16, 2023
25 of 29 checks passed
@snitin315 snitin315 deleted the issue16930 branch March 16, 2023 01:06
nzakas added a commit that referenced this pull request Mar 16, 2023
* feat: languageOptions.parser must be an object.

Changes languageOptions.parser to disallow string values.

Fixes #16930

* Add parser:null check

* Remove unnecessary check
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Mar 29, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.36.0` -> `8.37.0`](https://renovatebot.com/diffs/npm/eslint/8.36.0/8.37.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.37.0`](https://github.com/eslint/eslint/releases/tag/v8.37.0)

[Compare Source](eslint/eslint@v8.36.0...v8.37.0)

#### Features

-   [`b6ab8b2`](eslint/eslint@b6ab8b2) feat: `require-unicode-regexp` add suggestions ([#&#8203;17007](eslint/eslint#17007)) (Josh Goldberg)
-   [`10022b1`](eslint/eslint@10022b1) feat: Copy getScope() to SourceCode ([#&#8203;17004](eslint/eslint#17004)) (Nicholas C. Zakas)
-   [`1665c02`](eslint/eslint@1665c02) feat: Use plugin metadata for flat config serialization ([#&#8203;16992](eslint/eslint#16992)) (Nicholas C. Zakas)
-   [`b3634f6`](eslint/eslint@b3634f6) feat: docs license ([#&#8203;17010](eslint/eslint#17010)) (Samuel Roldan)
-   [`892e6e5`](eslint/eslint@892e6e5) feat: languageOptions.parser must be an object. ([#&#8203;16985](eslint/eslint#16985)) (Nicholas C. Zakas)

#### Bug Fixes

-   [`619f3fd`](eslint/eslint@619f3fd) fix: correctly handle `null` default config in `RuleTester` ([#&#8203;17023](eslint/eslint#17023)) (Brad Zacher)
-   [`1fbf118`](eslint/eslint@1fbf118) fix: `getFirstToken`/`getLastToken` on comment-only node ([#&#8203;16889](eslint/eslint#16889)) (Francesco Trotta)
-   [`129e252`](eslint/eslint@129e252) fix: Fix typo in `logical-assignment-operators` rule description ([#&#8203;17000](eslint/eslint#17000)) (Francesco Trotta)

#### Documentation

-   [`75339df`](eslint/eslint@75339df) docs: fix typos and missing info in id-match docs ([#&#8203;17029](eslint/eslint#17029)) (Ed Lucas)
-   [`ec2d830`](eslint/eslint@ec2d830) docs: Fix typos in the `semi` rule docs ([#&#8203;17012](eslint/eslint#17012)) (Andrii Lundiak)
-   [`e39f28d`](eslint/eslint@e39f28d) docs: add back to top button ([#&#8203;16979](eslint/eslint#16979)) (Tanuj Kanti)
-   [`721c717`](eslint/eslint@721c717) docs: Custom Processors cleanup and expansion ([#&#8203;16838](eslint/eslint#16838)) (Ben Perlmutter)
-   [`d049f97`](eslint/eslint@d049f97) docs: 'How ESLint is Maintained' page ([#&#8203;16961](eslint/eslint#16961)) (Ben Perlmutter)
-   [`5251a92`](eslint/eslint@5251a92) docs: Describe guard options for guard-for-in ([#&#8203;16986](eslint/eslint#16986)) (alope107)
-   [`6157d81`](eslint/eslint@6157d81) docs: Add example to guard-for-in docs. ([#&#8203;16983](eslint/eslint#16983)) (alope107)
-   [`fd47998`](eslint/eslint@fd47998) docs: update `Array.prototype.toSorted` specification link ([#&#8203;16982](eslint/eslint#16982)) (Milos Djermanovic)
-   [`3e1cf6b`](eslint/eslint@3e1cf6b) docs: Copy edits on Maintain ESLint docs ([#&#8203;16939](eslint/eslint#16939)) (Ben Perlmutter)

#### Chores

-   [`c67f299`](eslint/eslint@c67f299) chore: upgrade [@&#8203;eslint/js](https://github.com/eslint/js)[@&#8203;8](https://github.com/8).37.0 ([#&#8203;17033](eslint/eslint#17033)) (Milos Djermanovic)
-   [`ee9ddbd`](eslint/eslint@ee9ddbd) chore: package.json update for [@&#8203;eslint/js](https://github.com/eslint/js) release (ESLint Jenkins)
-   [`dddb475`](eslint/eslint@dddb475) chore: upgrade [@&#8203;eslint/eslintrc](https://github.com/eslint/eslintrc)[@&#8203;2](https://github.com/2).0.2 ([#&#8203;17032](eslint/eslint#17032)) (Milos Djermanovic)
-   [`522431e`](eslint/eslint@522431e) chore: upgrade espree@9.5.1 ([#&#8203;17031](eslint/eslint#17031)) (Milos Djermanovic)
-   [`f5f9a88`](eslint/eslint@f5f9a88) chore: upgrade eslint-visitor-keys@3.4.0 ([#&#8203;17030](eslint/eslint#17030)) (Milos Djermanovic)
-   [`4dd8d52`](eslint/eslint@4dd8d52) ci: bump actions/stale from 7 to 8 ([#&#8203;17026](eslint/eslint#17026)) (dependabot\[bot])
-   [`ad9dd6a`](eslint/eslint@ad9dd6a) chore: remove duplicate scss, ([#&#8203;17005](eslint/eslint#17005)) (Strek)
-   [`ada6a3e`](eslint/eslint@ada6a3e) ci: unpin Node 19 ([#&#8203;16993](eslint/eslint#16993)) (Milos Djermanovic)
-   [`c3da975`](eslint/eslint@c3da975) chore: Remove triage label from template ([#&#8203;16990](eslint/eslint#16990)) (Nicholas C. Zakas)
-   [`69bc0e2`](eslint/eslint@69bc0e2) ci: pin Node 19 to 19.7.0 ([#&#8203;16987](eslint/eslint#16987)) (Milos Djermanovic)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4yNC41IiwidXBkYXRlZEluVmVyIjoiMzUuMjQuNSJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1834
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Sep 13, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint feature This change adds a new feature to ESLint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Request: Remove parser plugins from flat config
4 participants