Skip to content

Conversation

@schneems
Copy link
Contributor

@schneems schneems commented Mar 7, 2025

No description provided.

When default features are disabled this crate won't be available, so we need to conditionally "use" it.
Uses the trait bounds largely provided by `strum` to write generic logic for constructing `KnownAttribute`-s and for generating a `HashMap` based lookup for `ParseAttribute`.
The new (shared) logic also ensures that the same attribute isn't defined more than once by mistake with a nice error.
Added `::` to prevent someone from doing something like `use other as cache_diff`.
Also makes a nicer error
The current type pattern will guarantee that every attribute known to the system is parsable, but it cannot guarantee that it's ever pulled from the HashMap. There's a slim chance someone could add an attribute, but forget to look it up. 

It's not likely with our current test suite, but I'm exploring ways to bake in more safety on edge cases where I can't lean more on types.

This code will check to ensure that all entries are removed from the attribute lookup, so if one is parsed but not used, they'll get a nice error message.

Worth noting: I experimented with a "drop guard" style of check that would automatically perform this logic on drop, but that leaves me with only the ability to panic in the error scenario which doesn't give us nice formatted compile errors.
Error before this change:

```
test tests/pass/struct_with_generics.rs ... error
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
error[E0107]: missing generics for struct `Example`
 --> tests/pass/struct_with_generics.rs:4:8
  |
4 | struct Example<T>
  |        ^^^^^^^ expected 1 generic argument
  |
note: struct defined here, with 1 generic parameter: `T`
 --> tests/pass/struct_with_generics.rs:4:8
  |
4 | struct Example<T>
  |        ^^^^^^^ -
help: add missing generic argument
  |
4 | struct Example<T><T>
  |               +++
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
```

After this change: It's passing.
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.

2 participants