search

time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.06k stars 261 forks source link

Missing panic condition on API docs #623

Open HeeillWang opened 10 months ago

HeeillWang commented 10 months ago

While I executed fuzzing, I faced some panics by expect. Though those are intentional crash, I think it's better to mention on docs as well. Some of APIs are already explicitly mention the panic condition.

I'll just list up the panic-condition-missing APIs, without reproduce condition as it's too obvious.

https://github.com/time-rs/time/blob/6248992b918638bc4f9e06e326c1fdace9c56fee/time/src/date.rs#L1304-L1307

https://github.com/time-rs/time/blob/6248992b918638bc4f9e06e326c1fdace9c56fee/time/src/duration.rs#L515-L535

https://github.com/time-rs/time/blob/6248992b918638bc4f9e06e326c1fdace9c56fee/time/src/duration.rs#L493-L513

https://github.com/time-rs/time/blob/6248992b918638bc4f9e06e326c1fdace9c56fee/time/src/duration.rs#L441-L453

jhpratt commented 10 months ago

Doc improvements are always welcome! It's not a priority for me at the moment.

HeeillWang commented 10 months ago

More cases :

https://github.com/time-rs/time/blob/c96bb1a4474b9af1289edbdf34514fbfe95fa833/time/src/duration.rs#L1311-L1314

https://github.com/time-rs/time/blob/c96bb1a4474b9af1289edbdf34514fbfe95fa833/time/src/duration.rs#L1266-L1269

https://github.com/time-rs/time/blob/c96bb1a4474b9af1289edbdf34514fbfe95fa833/time/src/ext.rs#L239-L279

HeeillWang commented 10 months ago

I'd like to defer adding doc for this at the moment, as I'm still running fuzzers, more cases may come out.

However, anyone is welcomed to create pr for this.

jhpratt commented 10 months ago

For panics that are deliberate, it's probably quickest to just check for assert! and .expect with ripgrep. Any other panics are likely unintentional — I can't think of any edge cases off the top of my head. .unwrap is not used anywhere, and this is enforced by clippy.