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

no-redeclare loose parser tests #14315

Closed
mdjermanovic opened this issue Apr 11, 2021 · 1 comment · Fixed by #14569
Closed

no-redeclare loose parser tests #14315

mdjermanovic opened this issue Apr 11, 2021 · 1 comment · Fixed by #14569
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 chore This change is not user-facing
Projects

Comments

@mdjermanovic
Copy link
Member

The version of ESLint you are using.

v7.24.0

The problem you want to solve.

We have several tests for the no-redeclare rule where let and const variables are redeclared:

// let/const
{
code: "let a; let a;",
parser: looseParserPath,
parserOptions: { ecmaVersion: 2015 },
errors: [
{ message: "'a' is already defined.", type: "Identifier" }
]
},

This is normally a syntax error reported by the parser, so the tests are using fixture parser tests/tools/loose-parser.js

The fixture parser requires an internal module from espree in order to produce an error-tolerant parser.

const espree = require("espree/lib/espree");

The problem is - after we release eslint/espree#469, that module will be no more available as it isn't defined by exports in espree's package.json. Even if we could bypass this restriction, the module will be ESM so it can't be required.

Your take on the correct solution to problem.

Just remove those tests? Core rules are not generally designed to handle code that has syntax errors, at least for now.

Are you willing to submit a pull request to implement this change?

Yes.

@mdjermanovic mdjermanovic added the chore This change is not user-facing label Apr 11, 2021
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Apr 11, 2021
@mdjermanovic mdjermanovic moved this from Needs Triage to Evaluating in Triage Apr 11, 2021
@nzakas
Copy link
Member

nzakas commented Apr 22, 2021

I agree, let's remove the tests.

@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label May 5, 2021
@mdjermanovic mdjermanovic moved this from Evaluating to Pull Request Opened in Triage May 5, 2021
Triage automation moved this from Pull Request Opened to Complete May 8, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Nov 5, 2021
@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 Nov 5, 2021
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 chore This change is not user-facing
Projects
Archived in project
Triage
Complete
Development

Successfully merging a pull request may close this issue.

2 participants