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

Exceptions when traversing using sibling, adjacent and nth-child selectors and non-standard AST nodes #13639

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 bug ESLint is working incorrectly core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects

Comments

@bradzacher
Copy link
Contributor

Tell us about your environment

  • ESLint Version: v7.7.0
  • Node Version: v12.14.1
  • npm Version: v6.13.4

What parser (default, @babel/eslint-parser, @typescript-eslint/parser, etc.) are you using?

@typescript-eslint/parser

Please show your full configuration:

Configuration
module.exports = {
  "parser": "@typescript-eslint/parser",
  "plugins": [ "eslint-plugin-test" ],
  "rules": { "test/test": "error" }
};

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

eslint-plugin-test source:

exports.rules = {
    test: {
        create(context) {
            return {
                "Program ~ *": node => {
                    context.report({
                        message: "test",
                        node
                    })

                    // this code doesn't get a chance to get called :(
                }
            }
        }
    }
}

lint file source:

const foo: string = "bar"
eslint ./test.ts

What actually happened? Please include the actual, raw output from ESLint.

Oops! Something went wrong! :(

ESLint: 7.7.0

TypeError: Cannot read property 'length' of undefined
Occurred while linting /home/samual/test/index.ts:1
    at u (/home/samual/test/node_modules/esquery/dist/esquery.min.js:1:31904)
    at Function.l [as matches] (/home/samual/test/node_modules/esquery/dist/esquery.min.js:1:30807)
    at NodeEventGenerator.applySelector (/home/samual/test/node_modules/eslint/lib/linter/node-event-generator.js:253:21)
    at NodeEventGenerator.applySelectors (/home/samual/test/node_modules/eslint/lib/linter/node-event-generator.js:281:22)
    at NodeEventGenerator.enterNode (/home/samual/test/node_modules/eslint/lib/linter/node-event-generator.js:297:14)
    at CodePathAnalyzer.enterNode (/home/samual/test/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:673:23)
    at /home/samual/test/node_modules/eslint/lib/linter/linter.js:949:32
    at Array.forEach (<anonymous>)
    at runRules (/home/samual/test/node_modules/eslint/lib/linter/linter.js:944:15)
    at Linter._verifyWithoutProcessors (/home/samual/test/node_modules/eslint/lib/linter/linter.js:1170:31)

Context

Original issue: typescript-eslint/typescript-eslint#2439

Investigation so far:

Our parser (@typescript-eslint/parser) returns an extended list of visitor-keys (as specified by the ESLint docs) right here

However esquery uses its own (hard coded) visitor keys for 3 selectors:

So I think that this is this something we'll have to PR into the underlying esquery library?

@bradzacher bradzacher added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Aug 31, 2020
@mdjermanovic mdjermanovic added blocked This change can't be completed until another issue is resolved core Relates to ESLint's core APIs and features and removed triage An ESLint team member will look at this issue soon labels Aug 31, 2020
@mdjermanovic
Copy link
Member

I think we should treat this as a bug, since our selectors documentation lists those selectors, so it's reasonable to expect that they can work with custom parsers.

Also, they'll crash even with the default parser on any JSX code, as in this demo.

It's true that this needs to be supported in esquery first, so we could pass visitors keys.

Relevant PR: estools/esquery#112

An alternative is to remove those selectors from the documentation. If this doesn't get fixed, it seems best not to use them at all.

samualtnorman added a commit to samualtnorman/eslint-plugin-hackmud2 that referenced this issue Sep 1, 2020
disabled no-closure-siblings rule to prevent crash with typescript plugin (see eslint/eslint#13639)
@brettz9
Copy link
Contributor

brettz9 commented Feb 5, 2021

esquery has just published a new release with the support for custom visitor keys, so I think this should no longer be blocking.

@brettz9
Copy link
Contributor

brettz9 commented Feb 5, 2021

Also, has:() selectors are also supported which perhaps the eslint docs can be updated to indicate (the subject indicator is also supported, but per estools/esquery#61 , that implementation is buggy while the :has() one was working enough to be used as a base for the former–though with one issue on relative keys mentioned there).

ota-meshi added a commit to ota-meshi/eslint that referenced this issue Feb 6, 2021
ota-meshi added a commit to ota-meshi/eslint that referenced this issue Feb 6, 2021
@mdjermanovic mdjermanovic added this to Pull Request Opened in Triage Feb 6, 2021
@mdjermanovic mdjermanovic 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 and removed blocked This change can't be completed until another issue is resolved labels Feb 6, 2021
@mdjermanovic
Copy link
Member

@brettz9 can you please open a separate issue about the new esquery syntax?

ESLint selectors are using esquery, but we're also doing some optimizations and calculating specificity in NodeEventGenerator, so it's possible that some additional work has to be done before we can say that ESLint fully supports new selectors.

@brettz9
Copy link
Contributor

brettz9 commented Feb 7, 2021

@mdjermanovic : Thanks, I've filed #14076 .

ota-meshi added a commit to ota-meshi/eslint that referenced this issue Feb 11, 2021
Triage automation moved this from Pull Request Opened to Complete Feb 12, 2021
mdjermanovic pushed a commit that referenced this issue Feb 12, 2021
* Fix: Crash with esquery when using JSX (fixes #13639)

* Chore: Add and change test cases related to esquery options

* Chore: Add test cases related to esquery options
This was referenced Mar 17, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Aug 12, 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 Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.