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: False positive markdown/no-missing-label-refs on GFM Alerts #294

Open
1 task
midskyey opened this issue Oct 18, 2024 · 6 comments
Open
1 task

Bug: False positive markdown/no-missing-label-refs on GFM Alerts #294

midskyey opened this issue Oct 18, 2024 · 6 comments
Labels
bug repro:yes Issues with a reproducible example

Comments

@midskyey
Copy link

Environment

ESLint version: 9.12.0
@eslint/markdown version: 6.2.1
Node version: 22.9.0
npm version: 10.8.3
Operating System: macOS 14.7

Which language are you using?

gfm

What did you do?

Configuration
import pluginMd from "@eslint/markdown"

export default [
  // Silly type narrowing for spreading in full-fledged typechecked `eslint.config.js`
  ...[pluginMd.configs?.recommended].filter(Boolean).flat(),
  {
    files: ["**/*.md"],
    language: "markdown/gfm",
  },
]
> [!NOTE]
> Useful information that users should know, even when skimming content.

> [!TIP]
> Helpful advice for doing things better or more easily.

> [!IMPORTANT]
> Key information users need to know to achieve their goal.

> [!WARNING]
> Urgent info that needs immediate user attention to avoid problems.

> [!CAUTION]
> Advises about risks or negative outcomes of certain actions.

Which is being rendered on GH as:

Note

Useful information that users should know, even when skimming content.

Tip

Helpful advice for doing things better or more easily.

Important

Key information users need to know to achieve their goal.

Warning

Urgent info that needs immediate user attention to avoid problems.

Caution

Advises about risks or negative outcomes of certain actions.

What did you expect to happen?

No violations.

What actually happened?

   1:4  error  Label reference '!NOTE' not found       markdown/no-missing-label-refs
   4:4  error  Label reference '!TIP' not found        markdown/no-missing-label-refs
   7:4  error  Label reference '!IMPORTANT' not found  markdown/no-missing-label-refs
  10:4  error  Label reference '!WARNING' not found    markdown/no-missing-label-refs
  13:4  error  Label reference '!CAUTION' not found    markdown/no-missing-label-refs

Link to Minimal Reproducible Example

https://stackblitz.com/edit/118f65b5-5b7b-4190-9fbe-b023b0542a4e?file=package.json

Participation

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

Additional comments

More info on GFM Alerts:

@midskyey midskyey added bug repro:needed This issue should include a reproducible example labels Oct 18, 2024
@eslintbot eslintbot added this to Triage Oct 18, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Oct 18, 2024
@mdjermanovic mdjermanovic added repro:yes Issues with a reproducible example and removed repro:needed This issue should include a reproducible example labels Oct 18, 2024
@mdjermanovic mdjermanovic moved this from Needs Triage to Triaging in Triage Oct 18, 2024
@mdjermanovic
Copy link
Member

Thanks for the issue! I can reproduce this with @eslint/markdown v6.2.1 (and v6.2.0 as well, so this isn't a regression introduced in v6.2.1).

I think this should be fixed in the parser as it treats [!NOTE] and others as just plain text, while it should probably be a new node type. @eslint/eslint-team thoughts?

@mdjermanovic mdjermanovic moved this from Triaging to Feedback Needed in Triage Oct 18, 2024
@nzakas
Copy link
Member

nzakas commented Oct 18, 2024

Yes, this is an issue with the parser not supporting GitHub alerts. It needs to be fixed there. Opened: syntax-tree/mdast-util-gfm#2

@nzakas nzakas moved this from Feedback Needed to Blocked in Triage Oct 18, 2024
@nzakas
Copy link
Member

nzakas commented Oct 18, 2024

So it looks like alerts are not actually part of GFM and are just an HTML transform:
syntax-tree/mdast-util-gfm#2 (comment)

By "HTML transform", they mean that the parsing of the Markdown isn't any different, it's just the way that the Markdown is interpreted while transforming to HTML that changes.

Here's the GFM spec:
https://github.github.com/gfm/

We have a couple options for what to do here:

  1. We could create a plugin that generates a separate AST for this syntax to make it more easily identifiable in the AST.
  2. We could add an option to the rule that filters out known GitHub alert labels.

@JoshuaKGoldberg
Copy link
Contributor

Agreed on 1, this feels like a plugin to me. GitHub alert labels are just one example of a nonstandard extension to GFM. Folks writing .md for other platforms won't want them.

@mdjermanovic
Copy link
Member

  1. We could create a plugin that generates a separate AST for this syntax to make it more easily identifiable in the AST.

We would then always use this plugin when parsing in gfm mode? That seems like a nice solution, though our parser would be doing something unique because other markdown parsers treat this as plain text, so that might be surprising to rule authors?

@nzakas
Copy link
Member

nzakas commented Oct 21, 2024

We would then always use this plugin when parsing in gfm mode? That seems like a nice solution, though our parser would be doing something unique because other markdown parsers treat this as plain text, so that might be surprising to rule authors?

Yes. I think we should maintain the sanctity of what "GFM" means and ensure it matches the spec by default. If we want to create an AST representation of alerts, then I think that should be opt-in as a language option for markdown/gfm.

adamlui added a commit to elonsucks/why.elonsucks.org that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/perplexity-omnibox that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/github-widescreen that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/github-star-history that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/chatgpt-widescreen that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/chatgpt-infinity that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/chatgpt-auto-refresh that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/chatgpt-auto-talk that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/chatgpt-auto-continue that referenced this issue Nov 13, 2024
adamlui added a commit to adamlui/autoclear-chatgpt-history that referenced this issue Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug repro:yes Issues with a reproducible example
Projects
Status: Blocked
Development

No branches or pull requests

4 participants