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

New schema and lints for const and static values #366

Closed
Tracked by #241
obi1kenobi opened this issue Feb 10, 2023 · 5 comments · Fixed by #768
Closed
Tracked by #241

New schema and lints for const and static values #366

obi1kenobi opened this issue Feb 10, 2023 · 5 comments · Fixed by #768
Labels
A-lint Area: new or existing lint E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: Mentorship is available for this issue.

Comments

@obi1kenobi
Copy link
Owner

obi1kenobi commented Feb 10, 2023

Add schema to represent const and static items. This will require new types for each in our query schema.

For now, ignore the type of the item. Adding type info to the schema is blocked on #149 which is a very hard issue (generics, lifetimes, HRTBs, effects, oh my!).

New lints enabled by this:

  • pub module-level const missing
  • pub module-level static missing
  • const inside type's impl block missing
  • mutable static became immutable static

All semver-major changes, because the item cannot be imported (it must have been renamed or deleted) and downstream is broken.

EDIT: Some of these have already been implemented, please check the rest of the thread to see what's left 👇

@obi1kenobi obi1kenobi changed the title expose constants: const and static values New schema and lints for const and static values Feb 10, 2023
@obi1kenobi obi1kenobi added A-lint Area: new or existing lint E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: Mentorship is available for this issue. labels May 20, 2023
@obi1kenobi
Copy link
Owner Author

Almost completely implemented, only remaining missing piece is associated constants. Waiting to hear back in rust-lang Zulip for that: https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Rustdoc.20JSON.3A.20How.20.60const.60.20items.20are.20represented

@obi1kenobi
Copy link
Owner Author

obi1kenobi commented Aug 2, 2023

Remaining lints here:

@eugenesvk
Copy link

Should a change in the value of a pub constant be tracked in a lint in addition to a change in name/visibility?

@obi1kenobi
Copy link
Owner Author

I wouldn't make it a hard error like the other kinds of breaking changes, but definitely could be a very reasonable warn-level lint.

We don't currently have the means to allow users to control error/warn/ignore levels, or enable/disable lints, so I wouldn't add it right away. There are also situations where the value of the pub constant might not have changed but its rustdoc value might change from a literal value to a _ which means something like "undetermined," so there's some nuance to worry about.

If this is something you might be interested in working toward, I'd be happy to mentor for PRs and point you in the right direction!

@eugenesvk
Copy link

Thanks for the offer, but I was only interested in this crate as a way to extract a list of constants and their types/values, and this seems to be based in your lower-level trustfall ecosystem, so to that end I've tried to patch it like obi1kenobi/trustfall-rustdoc-adapter#280, maybe that could be a helpful base for the semver checks if you could take at look there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: new or existing lint E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: Mentorship is available for this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants