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

Bug: (Exception occurs in markdown/no-missing-label-refs when links from the second onwards have undefined labels) #289

Closed
1 task
Ymmy833y opened this issue Oct 10, 2024 · 7 comments · Fixed by #290
Labels
accepted bug repro:yes Issues with a reproducible example

Comments

@Ymmy833y
Copy link

Ymmy833y commented Oct 10, 2024

Environment

ESLint version: 9.12.0
@eslint/markdown version: 6.2.0
Node version: 20.9.0
npm version: 10.8.3
Operating System: Windows 11

Which language are you using?

commonmark

What did you do?

Configuration
import markdown from "@eslint/markdown";

export default [
    {
        files: ["**/*.md"],
        plugins: {
            markdown
        },
        language: "markdown/commonmark",
        rules: {
            "markdown/no-missing-label-refs": "error",
        },
    }
];
  1. Create a Markdown file with the following content:
    [Foo][foo]
    [Bar][]
  2. Enable the @eslint/markdown plugin and the markdown/no-missing-label-refs rule in your ESLint configuration.
  3. Run ESLint on the Markdown file.

What did you expect to happen?

  1:7  error  Label reference 'foo' not found  markdown/no-missing-label-refs
  2:2  error  Label reference 'Bar' not found  markdown/no-missing-label-refs

What actually happened?

ESLint throws the following exception and stops execution:

Oops! Something went wrong! :(

ESLint: 9.12.0

TypeError: Cannot read properties of undefined (reading 'length')
Occurred while linting ...\eslint-markdown-sample\docs\sample.md:1
Rule: "markdown/no-missing-label-refs"
    at findMissingReferences (.../eslint-markdown-sample/node_modules/@eslint/markdown/dist/esm/index.js:1509:25)
    at text (.../eslint-markdown-sample/node_modules/@eslint/markdown/dist/esm/index.js:1580:9)
    at ruleErrorHandler (...\eslint-markdown-sample\node_modules\eslint\lib\linter\linter.js:1084:48)
    at ...\eslint-markdown-sample\node_modules\eslint\lib\linter\safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (...\eslint-markdown-sample\node_modules\eslint\lib\linter\safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (...\eslint-markdown-sample\node_modules\eslint\lib\linter\node-event-generator.js:297:26)     
    at NodeEventGenerator.applySelectors (...\eslint-markdown-sample\node_modules\eslint\lib\linter\node-event-generator.js:326:22)    
    at NodeEventGenerator.enterNode (...\eslint-markdown-sample\node_modules\eslint\lib\linter\node-event-generator.js:337:14)
    at runRules (...\eslint-markdown-sample\node_modules\eslint\lib\linter\linter.js:1128:40)

Link to Minimal Reproducible Example

https://stackblitz.com/edit/stackblitz-starters-bhmkmm?file=package.json

Participation

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

Additional comments

Failing Test Case

The following test case results in the exception being thrown:

// tests/rules/no-missing-label-refs.test.js

invalid: [
    {
        code: "[Foo][foo]\n[Bar][]",
        errors: [
            {
                messageId: "notFound",
                data: { label: "foo" },
                line: 1,
                column: 6,
                endLine: 1,
                endColumn: 9,
            },
            {
                messageId: "notFound",
                data: { label: "Bar" },
                line: 2,
                column: 2,
                endLine: 2,
                endColumn: 5,
            },
        ],
    },
],

Other Cases Where the Error Occurs

  1. Case 1
    [Foo][]
    [Bar][]
  2. Case 2
    [Foo][foo]
    [Bar][bar]
    [Hoge][]
  3. Case 3
    [Foo][foo]
    [Bar][]
    [Hoge][hoge]

Cases Where No Error Occurs

  1. Case 1
    [Foo][]
    [Bar][bar]
  2. Case 2
    [Foo][foo]
    [Bar][bar]
  3. Case 3
    [Foo][]
    [Bar][bar]
    [Hoge][hoge]
@Ymmy833y Ymmy833y added bug repro:needed This issue should include a reproducible example labels Oct 10, 2024
@eslintbot eslintbot added this to Triage Oct 10, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Oct 10, 2024
@salazarr-js
Copy link

I am not sure if it's related but a similar error happens when using this plugin alongside other plugins like @eslint/js

This works perfectly

import markdown from '@eslint/markdown'

export default [
  ...markdown.configs.recommended,
]

but this throws an error message

import jsEslint from '@eslint/js'
import markdown from '@eslint/markdown'

export default [
  ...markdown.configs.recommended,
  jsEslint.configs.recommended,
]
Oops! Something went wrong! :(

ESLint: 9.12.0

TypeError: Error while loading rule 'no-irregular-whitespace': sourceCode.getAllComments is not a function
Occurred while linting C:\Users\slzr\Dev\Personal\three-app\packages\core\readme.md
    at Object.create (C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\rules\no-irregular-whitespace.js:87:41)
    at createRuleListeners (C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\linter\linter.js:943:21)
    at C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\linter\linter.js:1068:84
    at Array.forEach (<anonymous>)
    at runRules (C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\linter\linter.js:999:34)
    at #flatVerifyWithoutProcessors (C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\linter\linter.js:1914:31)
    at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\linter\linter.js:1995:49)
    at Linter._verifyWithFlatConfigArray (C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\linter\linter.js:2084:21)
    at Linter.verify (C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\linter\linter.js:1528:61)
    at Linter.verifyAndFix (C:\Users\slzr\Dev\Personal\three-app\node_modules\eslint\lib\linter\linter.js:2322:29)

@mdjermanovic mdjermanovic moved this from Needs Triage to Triaging in Triage Oct 11, 2024
@mdjermanovic mdjermanovic added accepted repro:yes Issues with a reproducible example and removed repro:needed This issue should include a reproducible example labels Oct 11, 2024
@mdjermanovic
Copy link
Member

@Ymmy833y thanks for the report and all the details. I can reproduce this.

@mdjermanovic mdjermanovic moved this from Triaging to Ready to Implement in Triage Oct 11, 2024
@mdjermanovic
Copy link
Member

@salazarr-js it isn't related. jsEslint.configs.recommended doesn't specify files, so in your configuration JS recommended rules apply to all files. You can fix this in your config file by adding files like this:

import jsEslint from '@eslint/js'
import markdown from '@eslint/markdown'

export default [
  ...markdown.configs.recommended,
  {
    ...jsEslint.configs.recommended,
    files: ["**/*.js", "**/*.mjs", "**/*.cjs"]
  }  
]

@mdjermanovic
Copy link
Member

I'm working on this.

@salazarr-js
Copy link

@mdjermanovic thanks for that tip, it helped me a lot and made me understand a little more how eslint and markdown plugins work, one thing that has bothered me is that @eslint/js can work perfectly alongside other plugins like typescript-eslint inclusive with eslint-plugin-vue, but when I add the @eslint/markdown it starts to throws those errors unless I made the change that you showed me before

This is the eslint-plugin-vue recommended config setup

import js from '@eslint/js'
import ts from 'typescript-eslint'
import vue from 'eslint-plugin-vue'

export default ts.config(
  js.configs.recommended,
  ...ts.configs.recommended,
  ...vue.configs['flat/recommended'],
  {
    files: ['*.vue', '**/*.vue'],
    languageOptions: {
      parserOptions: {
        parser: '@typescript-eslint/parser'
      }
    }
  }
)

but to be able to use @eslint/markdown the only way to make everything work is with this workaround

import md from '@eslint/markdown'
import js from '@eslint/js'
import ts from 'typescript-eslint'
import vue from 'eslint-plugin-vue'

export default ts.config(
  { ignores: ['**/.vitepress'] },

  {
    ...md.configs.recommended[0],
    language: 'markdown/gfm'
  },
  {
    files: ['**/*.ts'],
    extends: [
      js.configs.recommended,
      ...ts.configs.recommended,
    ],
  },
  {
    files: ['*.vue', '**/*.vue'],
    extends: [
      ...vue.configs['flat/recommended'],
    ],
  },
  {
    files: ['*.vue', '**/*.vue'],
    languageOptions: {
      parserOptions: {
        parser: '@typescript-eslint/parser'
      }
    }
  },
)

@mdjermanovic
Copy link
Member

one thing that has bothered me is that @eslint/js can work perfectly alongside other plugins like typescript-eslint inclusive with eslint-plugin-vue, but when I add the @eslint/markdown it starts to throws those errors unless I made the change that you showed me before

Those plugins provide parsers that produce ASTs compatible with the AST specification expected by the core JS rules (ESTree) and/or processors that extract JS (or JS-like) code from the files. So, in these cases, core JS rules get a JS-like source code and a JS-like AST to work with. In most cases, that works perfectly fine.

Markdown source code is very different from JS source code, and their ASTs are very different, so core JS rules cannot work with it. When you add jsEslint.configs.recommended without files, that means core JS rules are enabled on all files, including .md files (for which the linting is enabled by including markdown.configs.recommended). Then, a core JS rule gets a markdown file to work with and crashes due to an unexpected source code structure or AST.

nzakas pushed a commit that referenced this issue Oct 16, 2024
* fix: no-missing-label-refs should not crash on undefined labels

Fixes #289

* fix locations

* add a note to function `findOffsets`

* add a comment for the loop

* add a comment for the regex
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Oct 16, 2024
@salazarr-js
Copy link

@mdjermanovic that was a masterclass! Thanks for taking the time to explain it and share it with us 🙋🏽‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted bug repro:yes Issues with a reproducible example
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants