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

docs(reference/lint_plugins): Clarify "Testing plugins" example #1518

Merged
merged 4 commits into from
Mar 3, 2025

Conversation

frou
Copy link
Contributor

@frou frou commented Mar 2, 2025

Hi there - I read this page while doing the Lint Rules Contest today and based on that have made a few small changes to fix it / make it clearer:

  1. Used normal type annotations for the example linter's default export, rather than satisfies Deno.lint.Plugin (This is because I noticed when publishing my own plugin that JSR complained about the latter being a "slow type").
  2. Fixed the example test file name to use a _ rather than -, so that deno test picks it up by default.
  3. Mentioned that the filename we give to Deno.lint.runPlugin is just a dummy (we provide the source-code to lint as a string).
  4. Fixed the assert for Deno.lint.Diagnostic.fix in the example test (it is never a function either way).
  5. Fixed the link to the API documentation for Deno.lint.Diagnostic.

Copy link
Contributor

@Aire-One Aire-One left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I'm also having trouble with this example and was preparing another similar PR. It's probably easier if we gather everything in the same place. Here are my additional quick notes 😄

@Aire-One
Copy link
Contributor

Aire-One commented Mar 2, 2025

Deleted a part of the example test which seems broken. It seems that the d.fix property is not yet ready for primetime, because it's not a function even when the Lint Rule has a fix configured, and currently seems to be an object like {range: undefined, text: undefined}.

The typing part was noticed before, and a PR is already merged! denoland/deno#28344

@frou
Copy link
Contributor Author

frou commented Mar 3, 2025

@Aire-One Thanks, well spotted. I noticed a couple of other things needed fixed when updating that, so have pushed those too. See the edited OP.

Copy link
Contributor

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, thanks!

@marvinhagemeister marvinhagemeister merged commit 0b2c84e into denoland:main Mar 3, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants