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

feat(rules): validate the option examples for rules #1442

Merged
merged 8 commits into from
Nov 18, 2024

Conversation

cr7pt0gr4ph7
Copy link
Contributor

@cr7pt0gr4ph7 cr7pt0gr4ph7 commented Nov 14, 2024

Summary

Extend the rules_check to:

(1) Ensure that configuration fragments are actually valid (and do not contain syntax or schema errors):

    /// ```json,options
    /// {
    ///     "options": { ... }
    /// }
    /// ```

(2) Validate rule examples that use a non-default configuration:

    /// ```json,options
    /// {
    ///     "options": { ... }
    /// }
    /// ```
    ///
    /// Invalid example for this configuration:
    ///
    /// ```js,expect_diagnostic,use_options
    /// import "invalid-import";
    /// ```
    ///
    /// Valid example for this configuration:
    ///
    /// ```json,use_options
    /// import "valid-import";
    /// ```

(3) Allow hiding lines from the output like rustdoc does by prefixing them with # .

The corrresponding PR for biomejs/website is biomejs/biome#4542.

Test Plan

  • pnpm codegen:rules
  • I manually inspected the documentation output generated for the website to ensure it is as intended.

Copy link

netlify bot commented Nov 14, 2024

Deploy Preview for biomejs ready!

Name Link
🔨 Latest commit 33d07f7
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/673b444ceac1790008f6e302
😎 Deploy Preview https://deploy-preview-1442--biomejs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@cr7pt0gr4ph7 cr7pt0gr4ph7 changed the title Check rule option examples feat(rules): validate the option examples for rules Nov 14, 2024
@cr7pt0gr4ph7
Copy link
Contributor Author

Note: This PR has merge conflicts because it needs to update Cargo.toml with new references to biome packages via specific Git commit SHA references. I'd recommend the following course of action:

  1. Review feat(rules_check): validate the option examples for rules biome#4542 first until it is ready to merge
  2. Let me integrate the requested changes into this PR
  3. Review & merge feat(rules): validate the option examples for rules #1442 so it supports the newly added doc features
  4. Merge feat(rules_check): validate the option examples for rules biome#4542
  5. Let the CI job update the commit hashes

@cr7pt0gr4ph7 cr7pt0gr4ph7 force-pushed the check-rule-option-examples branch from 8091cf9 to 92aefd2 Compare November 15, 2024 14:53
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

You should run pnpm codegen:rules to check end result, and commit those files

@cr7pt0gr4ph7
Copy link
Contributor Author

You should run pnpm codegen:rules to check end result, and commit those files

Done 👍 Though the changes actually caused by this commit will only show up after biomejs/biome#4542 has been merged, because no json,options blocks exist before that.

@cr7pt0gr4ph7
Copy link
Contributor Author

I re-ran pnpm codegen:rules after merging in the current changes from main. Should be ready for merge now!

@cr7pt0gr4ph7
Copy link
Contributor Author

Just as a heads-up, there are problems in the output that are related to/caused by #1440. They are not caused by the changes in this PR, but this PR manages to trigger some edge cases not hit before.

See for example https://deploy-preview-1442--biomejs.netlify.app/linter/rules/use-sorted-classes/#code-related:

image

@ematipico
Copy link
Member

Yeah that's an issue of MDX and the use of JS templates inside the source code. I still have no idea how to fix it properly, but for now we need to live it with it

@ematipico ematipico merged commit f598888 into biomejs:main Nov 18, 2024
6 of 7 checks passed
@cr7pt0gr4ph7 cr7pt0gr4ph7 deleted the check-rule-option-examples branch November 19, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants