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

Added deprecated attribute API lint, minor incompatibility level #57

Open
14 of 15 tasks
Tracked by #61 ...
epage opened this issue Aug 9, 2022 · 7 comments
Open
14 of 15 tasks
Tracked by #61 ...

Added deprecated attribute API lint, minor incompatibility level #57

epage opened this issue Aug 9, 2022 · 7 comments
Labels
A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations 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

@epage
Copy link
Collaborator

epage commented Aug 9, 2022

Semver says deprecations are minor changes.

Reference: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute

Will require separate checks for:

@epage epage added A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations E-help-wanted Call for participation: Help is requested to fix this issue. labels Aug 9, 2022
@obi1kenobi obi1kenobi added the E-mentor Call for participation: Mentorship is available for this issue. label Aug 10, 2022
@pksunkara
Copy link
Contributor

@obi1kenobi It hasn't been implemented for union. You might want to separate it out from struct & enum in the issue description.

@obi1kenobi
Copy link
Owner

Done, thanks!

obi1kenobi pushed a commit that referenced this issue Jan 31, 2025
obi1kenobi pushed a commit that referenced this issue Feb 3, 2025
obi1kenobi pushed a commit that referenced this issue Feb 4, 2025
obi1kenobi pushed a commit that referenced this issue Feb 4, 2025
obi1kenobi pushed a commit that referenced this issue Feb 4, 2025
obi1kenobi pushed a commit that referenced this issue Feb 4, 2025
part of #57 

I am actually not very sure when I should use `public_api_eligible` or
`visibility_limit` since we check that for trait method/type/const and
enum, enum variant, but not for trait itself, impl_owner itself?
obi1kenobi pushed a commit that referenced this issue Feb 4, 2025
part of: #57
should I add another lint: `proc_macro_marked_deprecated`
obi1kenobi pushed a commit that referenced this issue Feb 5, 2025
obi1kenobi pushed a commit that referenced this issue Feb 5, 2025
part of: #57 

I think I got the query right, but it seems to get triggered when the
struct is marked deprecated which should not be true?
@Frank-III
Copy link
Contributor

struct fields should be marked done: #1125. the only two missing parts have PR opened.

@obi1kenobi
Copy link
Owner

Sweet! Thank you for all your work, that's a lot of boxes you've checked!

Might you be interested in doing a bit of schema extension next, to unlock even more lints elsewhere? If so, adding "field ordering" information to struct fields and enum tuple/struct variant fields can unlock ~10ish more new lints. This is because changing the order of struct fields (e.g. swapping which field is first and which is second) can be a major breaking change in some circumstances.

@Frank-III
Copy link
Contributor

Sweet! Thank you for all your work, that's a lot of boxes you've checked!

Thank you for your guidance, the more lint I added the more I realized how important c-s-c would be in the rust ecosystem!

Might you be interested in doing a bit of schema extension next, to unlock even more lints elsewhere? If so, adding "field ordering" information to struct fields and enum tuple/struct variant fields can unlock ~10ish more new lints. This is because changing the order of struct fields (e.g. swapping which field is first and which is second) can be a major breaking change in some circumstances.

Definitely yes. Could you explain more about how it would be implemented? like adding a field to the StructField / Variant type ? and for the lints, we need to check if say a struct's field's order is changed, and report something like:

  • "field a in struct order has changed"
  • "field a in struct order has changed"

@obi1kenobi
Copy link
Owner

Here's an issue describing this in more detail and giving one example of a lint: obi1kenobi/trustfall-rustdoc-adapter#758

There are more lints that use order info in #5. I'll try to organize them into a separate issue with individual checkboxes similar to this one.

Off-topic: You are very good at this work! Consider applying for a Rust-themed Google Summer of Code of project, if you don't already have other plans this summer! There are some cargo-semver-checks projects on that list, and you can always pitch your own as well. Happy to chat more if that sounds interesting!

@Frank-III
Copy link
Contributor

Off-topic: You are very good at this work! Consider applying for a Rust-themed Google Summer of Code of project, if you don't already have other plans this summer! There are some cargo-semver-checks projects on that list, and you can always pitch your own as well. Happy to chat more if that sounds interesting!

Wow, I did not know that! would definitely apply for it and see what I could build for the summer, c-s-c seems to be something I wanna work for!

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 C-enhancement Category: raise the bar on expectations 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

No branches or pull requests

4 participants