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

Make size, decode and encode derives work with non-suffixed literals #115

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

CaioSym
Copy link
Contributor

@CaioSym CaioSym commented Mar 15, 2024

Issues:

Resolves #114

Description of changes:

Implemented parsing for the repr attribute when deriving mls codec and used it to infer the type of the enum discriminants that don't have explicit suffixes.

Call-outs:

I'm very much a noob when it comes to rust macros. The code works but it genuinely feels like there should be an easier way to do this: Specifically, these two areas could use some insight for someone who knows what they are doing:

  • Presumably there is a way to grab the repr values in FromDeriveInput directly but nothing I tried was working in a way that supported optionality and I did not want to force all declarations to define a repr.
  • The code creating the actual suffixed literal is using some cheeky string interpolation because I could not for the love of me figure out a better way to simply add a suffix to the Literal being parsed.

Testing:

I've added a TestEnumWithoutSuffixedLiterals to the macro usage to ensure this works. The TestEnum case is also still working.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT license.

@CaioSym CaioSym requested a review from a team as a code owner March 15, 2024 11:17
Copy link
Contributor

@tomleavy tomleavy left a comment

Choose a reason for hiding this comment

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

Thanks @CaioSym , just need to fix the clippy warnings

@CaioSym CaioSym force-pushed the repr-aware-derives branch from 15c8156 to c73c8e7 Compare March 18, 2024 10:31
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 94.11765% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 89.34%. Comparing base (650ed8d) to head (ef607a6).

Files Patch % Lines
mls-rs-codec-derive/src/lib.rs 94.11% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #115   +/-   ##
=======================================
  Coverage   89.34%   89.34%           
=======================================
  Files         173      173           
  Lines       31333    31372   +39     
=======================================
+ Hits        27993    28029   +36     
- Misses       3340     3343    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tomleavy tomleavy merged commit 0a52ca1 into awslabs:main Mar 18, 2024
12 checks passed
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.

Allow MlsSize, Decode and Encode to work with unsuffixed literals
4 participants