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

Change Request: Allow processors to report syntax errors #16104

Closed
1 task
btmills opened this issue Jul 6, 2022 · 5 comments · Fixed by #16105
Closed
1 task

Change Request: Allow processors to report syntax errors #16104

btmills opened this issue Jul 6, 2022 · 5 comments · Fixed by #16105
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
Projects

Comments

@btmills
Copy link
Member

btmills commented Jul 6, 2022

ESLint version

v8.19.0

What problem do you want to solve?

Originally discussed in #16015, the @graphql-eslint/graphql processor is encountering a syntax error in the file that is being processed (the "physical" file, if code blocks are "virtual" files). That syntax error results in an exception thrown within the processor that currently crashes the entire lint run. It would be better if processors could report syntax errors like other parsers without crashing the whole process.

What do you think is the correct solution?

We should mimic parse()'s behavior by wrapping the preprocess() call in try/catch and transforming the exception thrown by the processor into a lint error.

Participation

  • I am willing to submit a pull request for this change.

Additional comments

In the discussion, @JounQin asked to take this, but I'm also willing to tackle it as a fallback if necessary.

@btmills btmills added enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon core Relates to ESLint's core APIs and features labels Jul 6, 2022
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Jul 6, 2022
@btmills btmills moved this from Needs Triage to Evaluating in Triage Jul 6, 2022
@btmills btmills added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jul 6, 2022
@JounQin
Copy link
Contributor

JounQin commented Jul 6, 2022

Thanks for pinning, I'm going to work on a fix soon, then.

@JounQin
Copy link
Contributor

JounQin commented Jul 6, 2022

@btmills Done in #16105.

@dimaMachina
Copy link

Thanks @JounQin 💪🏻

@mdjermanovic
Copy link
Member

It would be better if processors could report syntax errors like other parsers without crashing the whole process.

Makes sense to me 👍

The only downside I can see is that if the processor is not stateless, continuing the work with other files after the point where throwing an error currently breaks the whole linting process might result in broken processing of subsequent files due to a possibly broken state, depending on how the processor is implemented.

@mdjermanovic
Copy link
Member

Since @nzakas already approved the PR, it seems we have consensus to implement this change, so I'm marking it as accepted.

@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jul 8, 2022
@mdjermanovic mdjermanovic moved this from Evaluating to Pull Request Opened in Triage Jul 8, 2022
Triage automation moved this from Pull Request Opened to Complete Jul 8, 2022
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jan 5, 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 Jan 5, 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
Projects
Archived in project
Triage
Complete
Development

Successfully merging a pull request may close this issue.

4 participants