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

Generate JSON schema for Ruff options #1329

Merged
merged 14 commits into from Dec 24, 2022

Conversation

edgarrmondragon
Copy link
Contributor

See #1217

@edgarrmondragon edgarrmondragon marked this pull request as draft December 22, 2022 06:30
@@ -251,6 +257,7 @@ pub struct Options {
select = ["E", "F", "B", "Q"]
"#
)]
/// A list of check code prefixes to enable.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's unfortunate if we have to repeat ourselves here. I would look into whether we can adjust the option macro to attach the doc attribute to the field (would probably be nice during development as well), or whether we can make use of the generated ConfigurationOptions struct that exposes all that information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I'll look into it

Copy link
Contributor

@squiddy squiddy Dec 22, 2022

Choose a reason for hiding this comment

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

We can also explore turning this around: see how far we can get with JSON schema & schemars, and perhaps we can build on that as a base for the [option] macros.

Copy link
Member

Choose a reason for hiding this comment

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

Repeating ourselves here isn't the end of the world (it's nice to have these as rust docs too, I guess?) though agree it'd be nice to reuse if there's a straightforward path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, there's a bit of overlap between the two traits so at some point (not necessarily in this PR) a single source of truth should be in place.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I think I have this working in the other direction?

Like we can do this:

#[option(
    default = r#"[]"#,
    value_type = "Vec<char>",
    example = r#"
        # Allow minus-sign (U+2212), greek-small-letter-rho (U+03C1), and the asterisk-operator (U+2217),
        # which could be confused for "-", "p", and "*", respectively.
        allowed-confusables = ["−", "ρ", "∗"]
    "#
)]
/// A list of allowed "confusable" Unicode characters to ignore when enforcing `RUF001`,
/// `RUF002`, and `RUF003`.
pub allowed_confusables: Option<Vec<char>>,
...

...and have it generate the schema, and the documentation, and it works in the IDE.

What do you think @squiddy?

Copy link
Member

Choose a reason for hiding this comment

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

(It's probably possible to generate the rustdoc from the proc macro, but I'm not that strong on macros (and I don't know how that would ever work with the IDE).)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, looks good to me! As you said, that fit's more into standard rust and how IDEs work. I found some hints on how to generate rustdoc from the macro, but than I ran into macro errors that I couldn't resolve.. probably for the best.

(On that topic, I wonder if we can just move everything into rustdoc and have the macro parse that text up into default / value_type and example.)

src/pyupgrade/settings.rs Outdated Show resolved Hide resolved
@squiddy
Copy link
Contributor

squiddy commented Dec 22, 2022

This is nice. I'm keeping an eye on this, because having a schema could come in handy for the wasm playground, where I currently export some custom schema to have this options sidebar. If there was a way for this schema to also have the default values in there, I could use just this. :)

@charliermarsh
Copy link
Member

This is awesome.

@edgarrmondragon edgarrmondragon marked this pull request as ready for review December 23, 2022 04:07
"title": "Options",
"type": "object",
"properties": {
"allowed-confusables": {
Copy link
Member

Choose a reason for hiding this comment

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

Are these missing descriptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there are a bunch of missing descriptions. I can add them :)

Copy link
Member

Choose a reason for hiding this comment

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

It’s ok, I think I can update the macro to read from rustdoc, and then these comments can just be taking the “doc” piece from above and moving them into comments verbatim.

Copy link
Member

Choose a reason for hiding this comment

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

The most helpful thing, in that light, would just be to take the existing doc= text from above and copying it as // docs for each attribute. But I can also do it, either way.

@charliermarsh charliermarsh merged commit 4888afd into astral-sh:main Dec 24, 2022
@charliermarsh
Copy link
Member

Thanks @edgarrmondragon as always!

@edgarrmondragon edgarrmondragon deleted the toml-json-schema branch December 24, 2022 23:33
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.

None yet

3 participants