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(utils): add parser name to thrown parser error message #8484

Merged
merged 6 commits into from Mar 16, 2024

Conversation

disaacson
Copy link
Contributor

@disaacson disaacson commented Feb 15, 2024

PR Checklist

Overview

An error is thrown if there is an issue with the detected parser. This change adds information on the parser that is causing the error to be thrown.

Current message:

Error: Error while loading rule '@typescript-eslint/no-base-to-string': You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser.
Note: detected a parser other than @typescript-eslint/parser. Make sure the parser is configured to forward

This change adds the following detail to the error message:
Parser: <detected parser causing the error>

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @disaacson!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

Copy link

netlify bot commented Feb 15, 2024

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit cee264e
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/65f4f603c96b250008a0067d
😎 Deploy Preview https://deploy-preview-8484--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 99 (🟢 up 6 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 98 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@disaacson disaacson closed this Feb 15, 2024
@disaacson
Copy link
Contributor Author

Apologies. Let me squash the commits.

Copy link

nx-cloud bot commented Feb 15, 2024

@JoshuaKGoldberg
Copy link
Member

@disaacson no need to rebase or squash 🙂. We prefer the raw, unedited history. https://typescript-eslint.io/contributing/pull-requests

@disaacson disaacson reopened this Feb 15, 2024
@disaacson
Copy link
Contributor Author

Using the example in the automated test for that error, it would now output

You have used a rule which requires parserServices to be generated. You must therefore provide a value for the \"parserOptions.project\" property for @typescript-eslint/parser.
    Parser: @babel/parser.js
    Note: detected a parser other than @typescript-eslint/parser. Make sure the parser is configured to forward \"parserOptions.project\" to @typescript-eslint/parser.

The information that the parser it found was @babel/parser.js can help track down the cause of the error.
If you think it would be better, I could change the PR to only include that information if the second Note error is thrown.

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.13%. Comparing base (609a000) to head (d0350ae).
Report is 2 commits behind head on main.

❗ Current head d0350ae differs from pull request most recent head cee264e. Consider uploading reports for the commit cee264e to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8484      +/-   ##
==========================================
- Coverage   87.23%   87.13%   -0.10%     
==========================================
  Files         251      251              
  Lines       12319    12270      -49     
  Branches     3884     3868      -16     
==========================================
- Hits        10746    10691      -55     
- Misses       1303     1307       +4     
- Partials      270      272       +2     
Flag Coverage Δ
unittest 87.13% <100.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ckages/utils/src/eslint-utils/getParserServices.ts 93.75% <100.00%> (+0.89%) ⬆️

... and 65 files with indirect coverage changes

Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg 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! 🙌

Will wait for @bradzacher a bit as Brad was also commenting on the issue.

@JoshuaKGoldberg JoshuaKGoldberg added the 1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready label Feb 27, 2024
@JoshuaKGoldberg JoshuaKGoldberg changed the title chore: add detail to thrown parser error - resolves #8482 feat: add parser name to thrown parser error Feb 27, 2024
@disaacson
Copy link
Contributor Author

I looked into the failed check for yarn lint. It passes locally on my machine. Also, the error appears to be related to a lock (see below). I wonder if rerunning the check will work. From what I read, it requires someone with write permissions on the repo to rerun the check. Or, I could trigger one by pushing an empty commit.

I'll leave it alone for now, but let me know if you'd like me to trigger it with an empty commit.

Error in check:

❌ > nx run typescript-eslint:lint
  
  
  Linting "typescript-eslint"...
  
   >  NX   _
  
  
  AtomicsWaitError: _
      at _Lock.lock (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:128:11)
      at request (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:161:8)
      at #sendActionToWorker (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:218:61)
      at ThreadsWorker.sendAction (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:177:36)
      at /home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:273:26
      at cacheResult (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:242:25)
      at cachePathResult (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:246:52)
      at _Synchronizer.getInformation (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:270:12)
      at _Synchronizer.get (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:277:30)
      at Module.get [as resolveConfig] (/home/runner/work/typescript-eslint/typescript-eslint/node_modules/make-synchronized/index.cjs:350:29)
      ```

Copy link
Member

@auvred auvred left a comment

Choose a reason for hiding this comment

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

Looks great! 🔥


CI failures are not related to this PR. See #8373

@disaacson
Copy link
Contributor Author

Is there anything else you'd like me to change @bradzacher or @JoshuaKGoldberg?

@auvred
Copy link
Member

auvred commented Mar 4, 2024

@disaacson, As per our contributing guidelines this PR is in the queue of PRs to reviewed, and will be reviewed when we are able.

This project is run by volunteer maintainers, so it might be a bit before we can find the time to get to it.

Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

@bradzacher bradzacher changed the title feat: add parser name to thrown parser error feat(utils): add parser name to thrown parser error message Mar 16, 2024
@bradzacher bradzacher merged commit df00134 into typescript-eslint:main Mar 16, 2024
54 of 55 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 approval PR that a maintainer has LGTM'd - any maintainer can merge this when ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Improve error message reported by getParserServices
4 participants