[Proposal] Coding Standards

[d0cd]

💥 Proposal

We propose the following coding standards for Leo:

• If code is unreachable for all invocations, use the unreachable! macro.
• Do not use unwraps, use expect with a reason, assert!, or a test-and-panic.
• Every struct, struct member, and function should have a comment explaining what it does. If it's a public item, use a doc comment.
• Minimize default catch alls _ => {}

TODO

Existing Leo needs to be updated to this standard.

[d0cd]

Minimize exposure of the module structure. pub use when needed.

[mikebenfield]

Responding to your comment from the refactor-pass PR:

I think its reasonable to use either but it would be helpful to have a (minimal) set of rules for our codebase. Last we discussed, we decided not to use unreachable! Do we want the rule to be allow either panic! or assert!?

That's not how I recall that discussion. I thought we decided to use unreachable for its proper use case, but that there were many incorrect uses of unreachable in the Leo code base we'd eventually change to panics.

Note that we also have many other functions that are essentially other ways of panicking - like expect, which we clearly are OK with.

In general I'd like to keep any coding standards minimalistic and/or automated when possible, and only make explicit rules when they differ from widely accepted Rust practice and/or documentation. panic and assert are used for logic bugs or unrecoverable errors, that's what everyone uses them for, and that's completely uncontroversial. IMO making a rule about that doesn't make any more sense than making a rule "Use Vec when you need a contiguous growable array" - we all already know that, and if we don't, we should be learning it from docs or other learning materials.

I also find rules that do have a reasonable basis often have so many exceptions that it can be difficult to concisely express what is really meant. For instance I understand the urge to say "no unwraps", but ultimately I think that's not really a good rule and that completely avoiding unwraps will lead to messier code in many cases (see here for a bunch of words about that).