Make the crate conform to the API guidelines

[tcharding]

Rust API Guidelines Checklist

• Naming (crate aligns with Rust naming conventions)
  • [ ] Casing conforms to RFC 430 (C-CASE)
  • Ad-hoc conversions follow as_, to_, into_ conventions (C-CONV)
  • Getter names follow Rust convention (C-GETTER)
  • Methods on collections that produce iterators follow iter, iter_mut, into_iter (C-ITER)
  • Iterator type names match the methods that produce them (C-ITER-TY)
  • Feature names are free of placeholder words (C-FEATURE)
  • Names use a consistent word order (C-WORD-ORDER)
• Interoperability (crate interacts nicely with other library functionality)
  • Types eagerly implement common traits (C-COMMON-TRAITS)
    • Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug,
      Display, Default
  • Conversions use the standard traits From, AsRef, AsMut (C-CONV-TRAITS)
  • Collections implement FromIterator and Extend (C-COLLECT)
  • Data structures implement Serde's Serialize, Deserialize (C-SERDE)
  • Types are Send and Sync where possible (C-SEND-SYNC)
  • Error types are meaningful and well-behaved (C-GOOD-ERR)
  • Binary number types provide Hex, Octal, Binary formatting (C-NUM-FMT)
  • Generic reader/writer functions take R: Read and W: Write by value (C-RW-VALUE)
• Macros (crate presents well-behaved macros)
  • Input syntax is evocative of the output (C-EVOCATIVE)
  • Macros compose well with attributes (C-MACRO-ATTR)
  • Item macros work anywhere that items are allowed (C-ANYWHERE)
  • Item macros support visibility specifiers (C-MACRO-VIS)
  • Type fragments are flexible (C-MACRO-TY)
• Documentation (crate is abundantly documented)
  • Crate level docs are thorough and include examples (C-CRATE-DOC)
  • All items have a rustdoc example (C-EXAMPLE)
  • Examples use ?, not try!, not unwrap (C-QUESTION-MARK)
  • Function docs include error, panic, and safety considerations (C-FAILURE)
  • Prose contains hyperlinks to relevant things (C-LINK)
  • Cargo.toml includes all common metadata (C-METADATA)
    • authors, description, license, homepage, documentation, repository,
      keywords, categories
  • Release notes document all significant changes (C-RELNOTES)
  • Rustdoc does not show unhelpful implementation details (C-HIDDEN)
• Predictability (crate enables legible code that acts how it looks)
  • Smart pointers do not add inherent methods (C-SMART-PTR)
  • Conversions live on the most specific type involved (C-CONV-SPECIFIC)
  • Functions with a clear receiver are methods (C-METHOD)
  • Functions do not take out-parameters (C-NO-OUT)
  • Operator overloads are unsurprising (C-OVERLOAD)
  • Only smart pointers implement Deref and DerefMut (C-DEREF)
  • Constructors are static, inherent methods (C-CTOR)
• Flexibility (crate supports diverse real-world use cases)
  • Functions expose intermediate results to avoid duplicate work (C-INTERMEDIATE)
  • Caller decides where to copy and place data (C-CALLER-CONTROL)
  • Functions minimize assumptions about parameters by using generics (C-GENERIC)
  • Traits are object-safe if they may be useful as a trait object (C-OBJECT)
• Type safety (crate leverages the type system effectively)
  • Newtypes provide static distinctions (C-NEWTYPE)
  • Arguments convey meaning through types, not bool or Option (C-CUSTOM-TYPE)
  • Types for a set of flags are bitflags, not enums (C-BITFLAG)
  • Builders enable construction of complex values (C-BUILDER)
• Dependability (crate is unlikely to do the wrong thing)
  • Functions validate their arguments (C-VALIDATE)
  • Destructors never fail (C-DTOR-FAIL)
  • Destructors that may block have alternatives (C-DTOR-BLOCK)
• Debuggability (crate is conducive to easy debugging)
  • All public types implement Debug (C-DEBUG)
  • Debug representation is never empty (C-DEBUG-NONEMPTY)
• Future proofing (crate is free to improve without breaking users' code)
  • Sealed traits protect against downstream implementations (C-SEALED)
  • Structs have private fields (C-STRUCT-PRIVATE)
  • Newtypes encapsulate implementation details (C-NEWTYPE-HIDE)
  • Data structures do not duplicate derived trait bounds (C-STRUCT-BOUNDS)
• Necessities (to whom they matter, they really matter)
  • Public dependencies of a stable crate are stable (C-STABLE)
  • [x] Crate and its dependencies have a permissive license (C-PERMISSIVE)

Extra items (not from official guidelines)

• non_exhaustive C-like enums should opt-out of integer casting unless they have well-defined associated constants
  • #4127

[tcharding]

This is to do if/when #206 merges and we want to release 1.1 from master.

[mpbagot]

Naming:

• C-CONV: DisplayHex::as_hex should be to_hex. Type is Copy and function takes owned, not ref. DisplayHex should no longer require Copy with the use of a GAT, and thus as_hex will be correct after: Convert DisplayHex to use GAT #215
• C-GETTER: All getters follow the convention.
• C-ITER: No functions return iterators.
• C-ITER-TY: No functions return iterators.
• C-FEATURE: The list of features is: std, alloc, serde, newer-rust-version. These are all fine.
• C-WORD-ORDER: All names use consistent word order.

[mpbagot]

Macros:

• C-EVOCATIVE: The hex! and fmt_hex_exact! macros expect syntax similar to function calls, matching their behaviour. The impl_fmt_traits! macro uses pseudo-trait-impl syntax, which mostly matches the resulting code.
• C-MACRO-ATTR: Only relevant for impl_fmt_traits!, which defines multiple trait implementations. These are all defined at once with no independent attribute control.
• C-ANYWHERE: impl_fmt_traits! can be invoked in a function or module scope and applies the trait to the scoped type. Test cases in display.rs confirm this. No other macros define types/trait impls.
• C-MACRO-VIS: Not relevant. No public macros generate types. impl_fmt_traits! trait impls are always public.
• C-MACRO-TY: Both the fmt_hex_exact! and impl_fmt_traits! (the only two that take a type) correctly handle types with paths and/or generics.

C-MACRO-ATTR is not satisfied by impl_fmt_traits!. The individual trait impls can't be independently controlled by the macro invocation. Do we want to change this?

[tcharding]

Way to go Mitch, lets crank.

[tcharding]

C-ITER-TY: No functions return iterators.

There are a few that do in iter module. This module likely needs the most attention out of any in the crate.

[tcharding]

Do we want to change this?

Not sure right now, I can have a look at all the macro ones as well.

[mpbagot]

There are a few that do in iter module.

My understanding is that this guideline refers to methods on types that yield an iterator. I am fairly sure that we don't have any such functions in the public API, other than the new constructors on the iterator types themselves.

[tcharding]

Oh ok, sounds reasonable. FTR in checking over your work I'm just sanity checking what you post not actually clicking on the docs for each one.

[mpbagot]

Interoperability:

• C-COMMON-TRAITS: Many types missing Clone, Eq, PartialEq, and Hash. All of those are added here: Derive standard traits on all types #221. Ord and PartialOrd are skipped as no types are orderable. All types that have a meaningful default value already have Default. DisplayArray, DisplayByteSlice and error types are the only types with Display.
• C-CONV-TRAITS: No types use Into for conversions. Aside from errors, no types have any conversion methods.
• C-COLLECT: Only BufEncoder represents a collection type. FromIterator and Extend are implemented for it here: Add FromIterator and Extend to BufEncoder #223
• C-SERDE: As mentioned in 8e77606, there's no intention to include serde traits in v1.1.
• C-SEND-SYNC: No types use pointers. All types implement Send and Sync automatically.
• C-GOOD-ERR: All error types implement Error, Send, Sync and Display with meaningful, lowercase messages with no trailing punctuation.
• C-NUM-FMT: There are no numeric types in the crate, so this is not applicable.
• C-RW-VALUE: The only function in the crate that takes a Read/Write is the constructor for HexWriter, which takes by value.

[mpbagot]

Debuggability:

• C-DEBUG: All public types have a Debug implementation.
• C-DEBUG-NONEMPTY: All public types have non-empty Debug implementations.

Case and HexToBytesIter were missing from the api test. To prevent regressions and guarantee the above, these two types have been added to the API test in: #225

[mpbagot]

Future proofing:

• C-SEALED: The only trait in the crate is DisplayHex. In Convert DisplayHex to use GAT #215 , it was explicitly suggested to unseal the trait after the introduction of the GAT Display type.
• C-STRUCT-PRIVATE: No structs in the crate have public fields.
• C-NEWTYPE-HIDE: The HexSliceToBytesIter type alias doesn't hide the representation details with a newtype as expected. This has been converted to a newtype around the previous alias in: Convert HexSliceToBytesIter to a newtype instead of an alias #227
• C-STRUCT-BOUNDS: No bounds on generic types include derivable traits, with the sole exception being DisplayHex's Copy until Convert DisplayHex to use GAT #215 merges.

[mpbagot]

Necessities

• C-STABLE: This crate has no public dependencies. if_rust_version is only used internally for the gating macro, and arrayvec is only used on a private field in BufEncoder.
• C-PERMISSIVE: Even though this is crossed out, just to be complete: the crate is CC0-1.0. The Cargo.toml and all src/ all show this with the relevant SPDX license string.

[mpbagot]

Dependability

• C-VALIDATE: Input validation is present throughout the crate, mainly dynamic enforcement with Result return values. BufEncoder uses panics as the error state for put_byte{,s}. Maybe this would be better as a Result with a new error type instead?
• C-DTOR-FAIL: No types in this crate have destructors.
• C-DTOR-BLOCK: No types in this crate have destructors.

C-VALIDATE: Should BufEncoder::put_byte{,s} be changed to return a Result with a new error type instead of panicking?

[mpbagot]

I've gone through this out of order. For future reference, these are the subheadings yet to be done:

• documentation
• predictability
• flexibility
• type safety