Skip to content

Fixed raw struct name identifier parsing#402

Merged
juntyr merged 4 commits intoron-rs:masterfrom
juntyr:401-raw-identifiers
Sep 7, 2022
Merged

Fixed raw struct name identifier parsing#402
juntyr merged 4 commits intoron-rs:masterfrom
juntyr:401-raw-identifiers

Conversation

@juntyr
Copy link
Member

@juntyr juntyr commented Aug 20, 2022

Fixes #401

  • adds documentation about valid identifiers to the grammar
  • checks identifier validity upon serialisation
  • fix struct name parsing to support raw identifiers

@torkleyy Deserialisation currently does NOT validate expected identifiers, so one could get an error saying found "World" but expected "Hello World" which has absolutely no user-side fix if you don't control the data structure you're deserialising into. I could specialise the error messages to add a note if an invalid identifier was expected, but that might clutter the code base.

  • I've included my change in CHANGELOG.md

@juntyr juntyr requested a review from torkleyy August 20, 2022 07:47
@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2022

Codecov Report

Merging #402 (37df3d8) into master (75ac75e) will decrease coverage by 0.84%.
The diff coverage is 79.37%.

@@            Coverage Diff             @@
##           master     #402      +/-   ##
==========================================
- Coverage   80.31%   79.46%   -0.85%     
==========================================
  Files          49       50       +1     
  Lines        6247     6565     +318     
==========================================
+ Hits         5017     5217     +200     
- Misses       1230     1348     +118     
Impacted Files Coverage Δ
src/error.rs 36.29% <36.73%> (+8.12%) ⬆️
src/parse.rs 79.53% <65.95%> (-14.23%) ⬇️
src/de/mod.rs 73.91% <72.72%> (-0.55%) ⬇️
src/ser/mod.rs 78.26% <100.00%> (+0.21%) ⬆️
tests/401_raw_identifier.rs 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@juntyr
Copy link
Member Author

juntyr commented Sep 7, 2022

@torkleyy Can I merge this PR?

Copy link
Member

@torkleyy torkleyy left a comment

Choose a reason for hiding this comment

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

Yes, looks good to me 👍

@juntyr juntyr merged commit a63b3c8 into ron-rs:master Sep 7, 2022
@juntyr juntyr deleted the 401-raw-identifiers branch September 7, 2022 16:00
juntyr added a commit to juntyr/ron that referenced this pull request Aug 15, 2023
* Fixed raw struct name identifier parsing

* Added more tests, raw identifier suggestion error, and invalid identifier error formatting

* Even more empty identifier tests
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.

Raw struct names cannot be deserialised

3 participants

Comments