Uplift lints from clippy to rustc

[oli-obk]

cc @Manishearth @rust-lang/lang

As discussed in the Clippy 1.0 RFC, we would like to uplift some lints from clippy to rustc. Full rationale can be found in https://github.com/rust-lang/rfcs/blob/b9c8471887f308223c226642cad3a8290731b942/text/0000-clippy-uno.md#compiler-uplift

The list of correctness lints in clippy follows. I have checked the boxes of lints that I think should be uplifted

Renamings happening during uplift have been added via -> new_name annotations

• for_loop_over_option: Checks for for loops over Option values.
• eq_op -> same_operands: Checks for equal operands to comparison, logical and
  bitwise, difference and division binary operators (==, >, etc., &&,
  ||, &, |, ^, - and /).
• iter_next_loop -> for_val_in_iter_next: Checks for loops on x.next().
• deprecated_semver -> incorrect_semver_strings: Checks for #[deprecated] annotations with a since
  field that is not a valid semantic version.
• drop_copy -> dropping_copy_types: Checks for calls to std::mem::drop with a value
  that derives the Copy trait
• not_unsafe_ptr_arg_deref -> deref_ptr_arg_in_safe_fns: Checks for public functions that dereferences raw pointer
  arguments but are not marked unsafe.
• logic_bug -> unused_boolean_operands: Checks for boolean expressions that contain terminals that
  can be eliminated.
• clone_double_ref -> cloning_references: Checks for usage of .clone() on an &&T.
• almost_swapped -> incorrect_swaps: Checks for foo = bar; bar = foo sequences.
• possible_missing_comma: Checks for possible missing comma in an array. It lints if
  an array element is a binary operator expression and it lies on two lines.
• wrong_transmute -> undefined_transmutes: Checks for transmutes that can't ever be correct on any
  architecture.
• invalid_regex: Checks regex creation
  (with Regex::new,RegexBuilder::new or RegexSet::new) for correct
  regex syntax.
• bad_bit_mask -> unused_bitmasks: Checks for incompatible bit masks in comparisons.
• drop_ref -> dropping_references: Checks for calls to std::mem::drop with a reference
  instead of an owned value.
• derive_hash_xor_eq: Checks for deriving Hash but implementing PartialEq
  explicitly or vice versa.
• useless_attribute: Checks for extern crate and use items annotated with
  lint attributes
• temporary_cstring_as_ptr: Checks for getting the inner pointer of a temporary
  CString.
• min_max -> incorrect_clamps: Checks for expressions where std::cmp::min and max are
  used to clamp values, but switched so that the result is constant.
• unit_cmp -> unit_comparisons: Checks for comparisons to unit.
• reverse_range_loop -> incorrect_reversed_ranges: Checks for loops over ranges x..y where both x and y
  are constant and x is greater or equal to y, unless the range is
  reversed or has a negative .step_by(_).
• erasing_op: Checks for erasing operations, e.g. x * 0.
• suspicious_op_assign_impl: Lints for suspicious operations in impls of OpAssign, e.g.
  subtracting elements in an AddAssign impl.
• float_cmp: Checks for (in-)equality comparisons on floating-point
  values (apart from zero), except in functions called *eq* (which probably
  implement equality for a type involving floats).
• zero_width_space -> zero_width_spaces: Checks for the Unicode zero-width space in the code.
• fn_to_numeric_cast_with_truncation -> truncating_fn_ptr_to_int_casts: Checks for casts of a function pointer to a numeric type not large enough to store address.
• suspicious_arithmetic_impl: Lints for suspicious operations in impls of arithmetic operators, e.g.
  subtracting elements in an Add impl.
• approx_constant: Checks for floating point literals that approximate
  constants which are defined in
  std::f32::consts
  or
  std::f64::consts,
  respectively, suggesting to use the predefined constant.
• while_immutable_condition -> unused_while_loop_conditions: Checks whether variables used within while loop condition
  can be (and are) mutated in the body.
• never_loop: Checks for loops that will always break, return or
  continue an outer loop.
• nonsensical_open_options -> unused_open_options: Checks for duplicate open options as well as combinations
  that make no sense.
• forget_copy -> forgetting_copy_types: Checks for calls to std::mem::forget with a value that
  derives the Copy trait
• if_same_then_else -> unused_branching: Checks for if/else with the same body as the then part
  and the else part.
• cast_ptr_alignment: Checks for casts from a less-strictly-aligned pointer to a
  more-strictly-aligned pointer
• ifs_same_cond -> unused_branching: Checks for consecutive ifs with the same condition.
• out_of_bounds_indexing: Checks for out of bounds array indexing with a constant
  index.
• modulo_one: Checks for getting the remainder of a division by one.
• inline_fn_without_body -> inline_fns_without_body: Checks for #[inline] on trait methods without bodies
• cmp_nan -> unused_comparisons: Checks for comparisons to NaN.
• ineffective_bit_mask: Checks for bit masks in comparisons which can be removed
  without changing the outcome.
• infinite_iter -> infinite_iterators: Checks for iteration that is guaranteed to be infinite.
• mut_from_ref -> returning_mut_ref_from_ref: This lint checks for functions that take immutable
  references and return
  mutable ones.
• unused_io_amount -> unused_io_amounts: Checks for unused written/read amount.
• invalid_ref -> undefined_references: Checks for creation of references to zeroed or uninitialized memory.
• serde_api_misuse: Checks for mis-uses of the serde API.
• forget_ref -> forgetting_references: Checks for calls to std::mem::forget with a reference
  instead of an owned value.
• absurd_extreme_comparisons -> unused_comparisons: Checks for comparisons where one side of the relation is
  either the minimum or maximum value for its type and warns if it involves a
  case that is always true or always false. Only integer and boolean types are
  checked.
• for_loop_over_result: Checks for for loops over Result values.
• iterator_step_by_zero -> iter_step_by_zero: Checks for calling .step_by(0) on iterators,
  which never terminates.
• enum_clike_unportable_variant: Checks for C-like enumerations that are
  repr(isize/usize) and have values that don't fit into an i32.

[oli-obk]

@rfcbot fcp merge

please raise concerns about specific lints directly with rfcbot

[rfcbot]

Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged teams:

• @Zoxc
• @cramertj
• @eddyb
• @estebank
• @michaelwoerister
• @nagisa
• @nikomatsakis
• @oli-obk
• @petrochenkov
• @pnkfelix
• @varkor

Concerns:

• naming conventions (Uplift lints from clippy to rustc #53224 (comment))
• some lints are opinionated on how the target platform ought to work (Uplift lints from clippy to rustc #53224 (comment))
• the descriptions of lints are a little too concise for decision making. (Uplift lints from clippy to rustc #53224 (comment))
• tying compiler to stdlib implementation details (Uplift lints from clippy to rustc #53224 (comment))

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[Mark-Simulacrum]

https://rust-lang-nursery.github.io/rust-clippy/master/index.html#unused_io_amount seems like it should be replaced with a #[must_use] on those, but I'm not sure if we can do that for trait methods yet.

[nrc]

cc @rust-lang/compiler

[eddyb]

@Mark-Simulacrum You should be able to do it without any special support on the method declaration in the trait, because that's what paths and method calls resolve to, but if you want to stick it on method definitions in impls, that's significantly harder.

[oli-obk]

The problem is not that the return value is unused, it is usually used via ?, because Result is already must_use. The problem is that the Ok value is unused, and I don't think that's solvable with must_use

[nikomatsakis]

for_loop_over_option: Checks for for loops over Option values.

I feel like classifying this as correctness is a bit of a stretch? It seems like type check ought to resolve most of the bugs that would occur here.

We may still want to uplift it, though, I'm not sure.

UPDATE: ok, my mistake, I didn't realize the meaning of the checked boxes. =)

[nagisa]

Checks for transmutes that can't ever be correct on any architecture.

Related: #50842

---

Checks for creation of references to zeroed or uninitialized memory.

IMO description is wrong, as the example shows creation of a zero-reference, not a reference to zeroed memory.

---

@rfcbot concern tying compiler to stdlib implementation details

Many of these lints are libstd specific (mostly pertaining to iterators). What mnemonic does clippy use to figure out what libstd items are what and whether the actual libstd is used at all and not e.g. use my_funky_crate as std;? How would the compiler deal with this without making half of the libstd a language item?

---

@rfcbot concern the descriptions of lints are a little too concise for decision making.

For example, it is unclear from description of eq_op whether it does act on generic code or non-builtin-types, where all of the operators can be overridden to have arbitrary side effects.