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.

Sign up for GitHub

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

Jump to bottom

Fixes a panic caused by unwrapping an option #105

Merged
merged 2 commits into from Aug 6, 2020
Merged

Fixes a panic caused by unwrapping an option #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47eda75
Fixes a panic caused by unwrapping an option
peter-glotfelty Aug 6, 2020
520b2e6
updating to 0.19.2 to fix regression 104
peter-glotfelty Aug 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
@@ -1,8 +1,14 @@
# Changelog

## 0.20.0
## 0.19.2

* Fixes [#104](https://github.com/Peternator7/strum/issues/104). PR [#105](https://github.com/Peternator7/strum/issues/105)

## 0.19.1

* **Breaking Change**: EnumVariantNames now properly adjusts to the `to_string` and `serialize` attributes.
* There's a regression in this release that may make strum imcompatible with other plugins if those
plugins use non-rust syntax in their attributes. [#104](https://github.com/Peternator7/strum/issues/104)

## 0.19.0

Expand Down
6 changes: 3 additions & 3 deletions strum/Cargo.toml
View file
Open in desktop
@@ -1,6 +1,6 @@
[package]
name = "strum"
version = "0.19.1"
version = "0.19.2"
authors = ["Peter Glotfelty <peter.glotfelty@microsoft.com>"]
license = "MIT"

Expand All @@ -14,10 +14,10 @@ repository = "https://github.com/Peternator7/strum"
readme = "../README.md"

[dependencies]
strum_macros = { path = "../strum_macros", optional = true, version = "0.19.0" }
strum_macros = { path = "../strum_macros", optional = true, version = "0.19.2" }

[dev-dependencies]
strum_macros = { path = "../strum_macros", version = "0.19.0" }
strum_macros = { path = "../strum_macros", version = "0.19.2" }

[badges]
travis-ci = { repository = "Peternator7/strum" }
Expand Down
2 changes: 1 addition & 1 deletion strum_macros/Cargo.toml
View file
Open in desktop
@@ -1,6 +1,6 @@
[package]
name = "strum_macros"
version = "0.19.1"
version = "0.19.2"
authors = ["Peter Glotfelty <peter.glotfelty@microsoft.com>"]
license = "MIT"

Expand Down
2 changes: 1 addition & 1 deletion strum_macros/src/helpers/has_metadata.rs
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@ fn get_metadata_inner<'a>(
it: impl IntoIterator<Item = &'a syn::Attribute>,
) -> Vec<syn::Meta> {
it.into_iter()
.map(|attr| attr.parse_meta().unwrap())
.filter_map(|attr| attr.parse_meta().ok())
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICT this is still wrong. This will now silently ignore malformed strum attributes. Instead, you should filter by attr.path.is_ident(ident) and then use parse_meta() without discarding errors (the is_ident check below can then be skipped).

Copy link
Contributor

Choose a reason for hiding this comment

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

I can also recommend setting up trybuild to test expected proc-macro errors.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Happy to take a PR fixing this. Historically strum ignored improperly formatted/tagged attributes. Version 0.19 took some steps in the right direction in terms of failing on bad inputs, but not all of them.

.filter_map(|meta| match meta {
syn::Meta::List(syn::MetaList { path, nested, .. }) => {
if path.is_ident(ident) {
Expand Down