autumnai / leaf

Open Machine Intelligence Framework for Hackers. (GPU/CPU)
Apache License 2.0
5.56k stars 272 forks source link

Clippy is very angry #106

Closed ehiggs closed 5 years ago

ehiggs commented 8 years ago

If clippy runs into docstring errors then it will report those errors and bail. This might give the impression that the code is pretty sound since the only issues are docstring things. Well, that wouldn't be correct! If we fix these 36 docstring issues then a cascade of >450 errors on the actual code is the result.

For the lints that refer to a wiki page we can use the following grep:

$ cargo build --features=clippy 2>&1 | tee build.log
$ cat build.log | grep http | cut -f2 -d'#' | sort | uniq -c
   1 assign_op_pattern
  31 cast_possible_truncation
   1 cast_possible_wrap
   8 cast_precision_loss
   2 cast_sign_loss
   1 explicit_iter_loop
 116 indexing_slicing
   3 map_clone
   2 match_bool
   3 match_ref_pats
   1 match_same_arms
   2 mut_mut
   7 needless_borrow
  37 option_unwrap_used
 194 result_unwrap_used
   2 shadow_reuse
   1 shadow_unrelated
   7 single_match_else
   4 too_many_arguments
   9 toplevel_ref_arg
  10 type_complexity
  13 use_debug
   3 used_underscore_binding
   3 useless_vec

This is 461 errors, but there are 481 errors in all. The ones that don't refer to a page are mostly about unneeded use of & when calling functions and using match on boolean expressions (clippy prefers if/else).

Obviously not all of these need to fixed in a humungous PR, but what should be done?

FWIW, here's a patch that fixes the docstring issues: docstring.patch.zip

Patch is a zip since github doesn't want any files with .patch as a suffix. 😞

NB: this required updating clippy to 0.0.67 in Cargo.toml.

ehiggs commented 8 years ago

If clippy runs into docstring errors then it will report those errors and bail. This might give the impression that the code is pretty sound since the only issues are docstring things. Well, that wouldn't be correct! If we fix these 36 docstring issues then a cascade of >450 errors on the actual code is the result.

This is because clippy has two passes. Early and late. If you have 'deny' set on something in an early phase (e.g. docstring stuff) then it will flag errors and bail as described. This isn't an issue with clippy or anything since users should be aware that they set clippy up to report errors when it finds them.

(noting this so when I read my old issues in 3 months I don't bother the clippy team asking if this is a clippy bug again).

ehiggs commented 5 years ago

As leaf is no longer maintained, this will never be fixed.