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

Refactor and unify new attribute parsing #303

Merged
merged 26 commits into from
Oct 6, 2023
Merged

Conversation

tyranron
Copy link
Collaborator

Synopsis

In the recent PRs, reworking AsRef/AsMut, Debug, Display, From and Into macros, the new approach was introduced for attributes parsing: attributes are parsed as dedicated types, representing each attribute. However, due to lack of a single machinery to do so, it brought a lot of boilerplate and code duplication. Utterly, the parse_attrs() logic is fully repeated for each attribute type. This also lead, that the same attribute type (like skip of forward) is implemented twice in different macros, leading to some hardly noticeable behavior differences. This was partially fixed in those previous PRs by extracting some repeated attributes to utils module, but this is not enough.

Solution

This PR introduces utils::attr module, which serves and provides the following:

  • ParseMultiple trait machinery, which represents an extracted and abstracted parse_attrs() approach, and DRYes all the attributes' implementations by eliminating boilerplate. Now, implementing a custom attribute parsing only requires specifying behavior deviating from the default one, everything else is there already.
  • Extracted commonly used attributes, like skip, forward, (<types>), repr, etc, so they can be easily reused in different existing macros and in future implementations.
  • Either implementation for ParseMultiple, which allows simply combine different attributes in Either<OneAttribute, AnotherAttribute> way without introducing new types and implementations for it.

Additionally

Removes parsing::Type. The parsing::Expr was introduced to avoid using syn/full feature for syn::Expr, as it blows compilation times. Later, similar parsing::Type was introduced instead of syn::Type. However, syn::Type doesn't require a syn/full feature to work fully and properly, and so, we can just use syn::Type and don't bother.

Checklist

  • Documentation is updated
  • Tests are added/updated
  • CHANGELOG entry is added (not required)

@tyranron tyranron added this to the 1.0.0 milestone Sep 20, 2023
@tyranron tyranron self-assigned this Sep 20, 2023
@tyranron
Copy link
Collaborator Author

@JelteF ping

1 similar comment
@tyranron
Copy link
Collaborator Author

@JelteF ping

Copy link
Owner

@JelteF JelteF left a comment

Choose a reason for hiding this comment

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

Looks fine to me, only note is that total line count actually got larger with this PR

@tyranron
Copy link
Collaborator Author

tyranron commented Oct 6, 2023

@JelteF yup, because generic code usually requires more lines when declared. However, the actual code duplication was largely reduced, and it should show even more reusage in subsequent PRs.

@tyranron tyranron merged commit da9709d into master Oct 6, 2023
16 checks passed
@tyranron tyranron deleted the refactor-and-unify branch October 6, 2023 11:25
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.

3 participants