Open akhi3030 opened 1 year ago
I suggest that we enable the following lints at some point:
implicit_saturating_add
, implicit_saturating_sub
, manual_saturating_arithmetic
, len_zero
, manual_range_contains
, missing_safety_doc
;as_conversions
, clone_on_ref_ptr
, map_err_ignore
, undocumented_unsafe_blocks
;
as_conversions
is likely to be quite contentious, but my personal belief is that all of them should become a from
or try_from
anyway for the same reason we don't allow auto-wrapping arithmetic.map_err_ignore
just enforces a good error propagation hygiene.undocumented_unsafe_blocks
, would complement well with the built-in unsafe-op-in-unsafe-fn
lint, which we should also enable.checked_conversions
, doc_markdown
;
doc_markdown
in particular is interesting as I’ve seen people adding doc-comments that aren't quite markdown.redundant_clone
redundant_clone
is something I keep seeing a lot, though all of these should be considered.As a more general strategy note, I don’t believe wholesale enabling any of the groups outside suspicious
and correctness
makes sense. I think each lint should be denied and listed individually in run_clippy.sh
, even if the end result is denial of the entire category anyway. Doing it that way means that it is easy to add or remove individual lints in the future as our opinions of their benefits changes. It also avoids sustaining as many breaking changes on toolchain upgrades as clippy adds more lints in the existing categories.
Another thing to do is to look through our STYLE.md
document and see if there are any lints in clippy that would force those rules.
This issue tracks clippy adoption as part of nearcore CI.
We've decided to gradually enable clippy starting from the most useful checks (see the discussion here for more context).
clippy is currently executed as part of buildkite
sanity checks
step via run_clippy script.Done:
8931
8930
8925
Next:
8631