# Command spring cleaning

**Goal:** systematically check all commands for basic bugs and documentation flaws
**Sideeffect:** familiarize more people with our command internals, identify inconsistent semantics, find opportunities to simplify

## Checklist

- [ ] The documented input output types contain all the accepted or returned `Value` types in the implementation.
- Due to limitations in our type check, it may be more correct to use a supertype before more specific types. The parse-time type check can not handle the situation of two different specific types being returned from one common input type
- [ ] Examples are up to date and useful
- [ ] The examples get tested (there is a test invoking `test_examples`)
- [ ] (if no return type given and thus not auto-tested) all flags or other commands in the example are valid and not deprecated
- [ ] Examples for which the return value can be safely tested (no dependence on the surrounding environment, no side-effect) actually provide their return value
- [ ] All relevant flags and usages have an example with explanation (**Up for debate:** should all possible broadcasting rules be covered explicitly: runs on single value, runs on list, runs on table?; Covering all types may be superfluous when the allowed type pairings are explicitly documented and otherwise trivial)
- [ ] Examples which don't further the users understanding but rather try to purely test a past bug/edge case, are moved into into a traditional test.
- [ ] Input checking
- [ ] flags/options that are incompatible or ineffective together raise a [`ShellError::IncompatibleParameters`](https://docs.rs/nu-protocol/latest/nu_protocol/enum.ShellError.html#variant.IncompatibleParameters) (or similar error if appropriate)
- [ ] runtime type checking (remember that we can receive `Type::Any`) will return an appropriate error documenting which *types* are supported (**Be careful** we need to update shell error variants to be consistent: https://github.com/nushell/nushell/issues/10698)
- [ ] Invalid values (of correct type but e.g. out of bounds, not matching a schema) also provide a helpful error message. (Distinction with type errors should be helpful)
- [ ] sound use of spans in those errors (**Core team discussion:** do we have a span guideline for when to pass the value span and when to update to the command head span)
- [ ] Error strings that are generated make sense in combination of the command provided string and the format string on `ShellError` (-> adjust locally or open an issue about inconsistent use of `ShellError` format strings to not conflict)
- [ ] Correctness
- [ ] If working on tables, make sure that it doesn't make the assumptions that all records have the same columns in the same order, this has to be explicitly checked otherwise incorrect results are produced (semantics are up for debate for the non rectangular/regular tables)
- [ ] If generating records, do not create records with duplicate column names, `Record::insert` will replace existing entries and is safe to use but an error to the user may still be warranted, `Record::from_iterator`/`Record::push` currently don't perform such a check and need to be used carefully
- [ ] Semantics
- [ ] The command only collects if needed for sound operations.
- [ ] Type coercions are only done if well documented and in line with the rest of the commands, preferrably use common helpers on `Value` if you have to do so.
- [ ] Handling of missing data is consistent, if in doubt raise an error (**TODO:** collect consistent rules on what we are doing here across the commands and try to write a guideline)
- [ ] Behavior across `PipelineData` variants is consistent if they are manually handled.