Generated schemas are not valid when using rust style names in ref

[waltronix]

#798

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.54%. Comparing base (2d70b70) to head (98bcdb7).
Report is 19 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #799      +/-   ##
==========================================
+ Coverage   67.34%   67.54%   +0.20%     
==========================================
  Files          40       40              
  Lines        2468     2499      +31     
==========================================
+ Hits         1662     1688      +26     
- Misses        806      811       +5     

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

[swlynch99]

I'm not a maintainer of serde-with, just the person who originally wrote the schemars integration. So feel free to ignore this if you want.

With that said, it would be really helpful to document this on the schema_id and schema_name methods of the JsonSchemaAs trait. That the characters need to be valid as a URI path segment for refs to work is very much not apparent unless you are familiar with the json schema spec.

[jonasbb]

Hi, thanks for the PR. The change looks fine overall. Adding the extra validation check to the test makes sense.
Do you have a reference which names are allowed? In #798 you listed the three characters <, >, and ,. The , is still used here and seems to pass the tests.

If the names are invalid anyway, then backwards compatability is not a problem.

[swlynch99]

I think it needs to be a URI path segment, so it would be defined in RFC 3986 section 3.3. That refers back to section 2 and section 2.2 seems to indicate that all 3 of (, ,, and ) are valid.

[waltronix]

Thanks for your fast response!

I had a mistake in my bug report. There is no problem with the , itself, the problem arises from the space after it.

I also extended the doc comment for schema_name(). As schema_id() is not part of the generated schema according to current documentation I don't think have to require it to be a URI path segment.