Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clippy is very angry #106

Closed
ehiggs opened this issue May 14, 2016 · 2 comments
Closed

Clippy is very angry #106

ehiggs opened this issue May 14, 2016 · 2 comments

Comments

@ehiggs
Copy link

ehiggs commented May 14, 2016

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
Copy link
Author

ehiggs commented Aug 31, 2016

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
Copy link
Author

ehiggs commented Apr 23, 2019

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

@ehiggs ehiggs closed this as completed Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant