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

Consistently naming lints #2845

Open
clarfonthey opened this issue Jun 14, 2018 · 6 comments
Open

Consistently naming lints #2845

clarfonthey opened this issue Jun 14, 2018 · 6 comments
Labels
S-needs-discussion Status: Needs further discussion before merging or work can be started
Milestone

Comments

@clarfonthey
Copy link
Contributor

clarfonthey commented Jun 14, 2018

RFC 344 mentions a naming convention for lints, and I think that going through the names of the lints clippy offers to be more consistent would be a good idea.

The full list of lints is here: https://rust-lang-nursery.github.io/rust-clippy/master/index.html

I'll be updating this as I go through the lints with what extra conventions on top of what RFC 344 offers should be added to help clippy create better lint names. :)

Lint rules:

  1. We should probably aim for a whitelist of starting words, just like how unused is used across Rust lints. For now, I can think of incorrect, excessive, redundant, temporary, and undefined as a few big keywords.
  2. When describing statements or expressions that are explicitly checked, said constructs should be explained as closely to Rust syntax as possible, using val as a placeholder for a variable name.
  3. When describing values of a particular trait, the trait should be suffixed with types.
  4. When describing things happening in a function that is not properly annotated, these should be listed as construct_in_XXX_function.
  5. When describing lints, describe the lint rather than the intent.
  6. When describing UB, start with undefined.
  7. Use nouns rather than shortenings of words whenever possible.
  8. When describing rust syntax in lints, don't go overboard and separate out lints that should be together.

Lint changes:

  • almost_swapped, following rule 1, should be incorrect_swaps
  • approx_constant should be approximate_known_constants
  • assign_op_pattern definitely should have a different name but I'm not sure what
  • borrowed_box should be renamed to redundant_boxes and include other lints like boxed_local
  • for_loop_over_option and for_loop_over_result, following rules 2 and 8, should be for_val_in_option
  • iter_next_loop, following rule 2, should be for_val_in_iter_next
  • deprecated_semver should be non_semver_deprecations, because rather than
  • drop_copy, following rule 3, should be drop_copy_types
  • not_unsafe_ptr_arg_deref, following rule 4, should be deref_ptr_arg_in_safe_functions
  • logic_bug should explain what it's linting and be renamed to unused_short_circuits
  • possible_missing_comma, by rule 5, should be renamed to multiline_binops_in_array
  • wrong_transmute, by rule 6, should be undefined_transmutes
  • invalid_regex, by rule 1, should be incorrect_regexes
  • min_max, following rule 1, should be incorrect_clamps
  • unit_cmp should be unit_comparisons by 7
  • reverse_range_loop, by 1 should be incorrect_reversed_ranges
  • suspicious_arithmetic_impl by 5 should be incorrect_op_in_impl_op
  • suspicious_op_assign_impl by 5 should be incorrect_op_in_impl_op_assign, or combined with incorrect_op_in_impl_op
  • float_cmp should be float_comparisons by 7
  • zero_width_space should be zero_width_spaces
  • fn_to_numeric_cast_with_truncation should be truncating_fn_ptr_as_int by 2
  • while_immutable_condition should be while_immutable_val by 2
  • never_loop should be infinite_loops
  • infinite_iter should be infinite_loops, unless we're still not sure about the quality of never_loop and want to add an infinite_iter_loops in the meantime
  • nonsensical_open_options should be unused_file_options
  • forget_copy should be forget_copy_types
  • if_same_then_else and ifs_same_cond should be combined into unused_if
  • cast_ptr_alignment should be incorrectly_aligned_ptr_casts
  • modulo_one, erasing_op, ineffective_bit_mask should be combined into unused_operations
  • inline_fn_without_body should be empty_inline_fn
  • mut_from_ref should be returning_mut_ref_from_ref
  • invalid_ref should be undefined_references
  • serde_api_misuse probably needs a different name but idk what it should be
  • match_bool should be match_bool_types
  • cmp_null should be null_comparisons
  • write_with_newline and print_with_newline should be print_val_newline
  • unneeded_field_pattern should be unused_field_patterns
  • new_without_default_derive might need a new name or rule
  • zero_ptr should be zero_as_ptr
  • wrong_self_convention should be incorrect_self_conventions
  • inconsistent_digit_grouping should be inconsistent_digit_groups to match large_digit_groups
  • range_minus_one should be inclusive_ranges_to_val_minus_one (I don't like how long it is, but I can't think of anything better?)
  • regex_macro should be incorrect_regex_macro_calls
  • op_ref should be referencing_comparisons
  • question_mark should be incorrect_tries
  • redundant_closure should be unused_closures
  • ptr_arg should be smart_ptr_arguments
  • chars_last_cmp should be chars_last_eq_char
  • blacklisted_name should be blacklisted_names
  • double_neg should be double_negations
  • unnecessary_fold should be unspecialized_folds
  • let_unit_value should be unit_variables
  • needless_range_loop should be redundant_index_loops
  • excessive_precision should be excessive_precision_float_literals
  • duplicate_underscore_argument should be double_underscore_arguments
  • panic_params should be missing_panic_arguments
  • writeln_empty_string should be part of println_empty_string
  • infallible_destructuring_match should be part of infalliable_matches
  • block_in_if_condition_stmt should be statements_in_if_conditions
  • unreadable_literal should be unreadable_literals
  • unsafe_removed_from_name should be import_unsafe_as_safe
  • builtin_type_shadow should be builtin_type_shadowing
  • neg_multiply should be times_negative_one
  • const_static_lifetime should be unused_static_lifetimes
  • explicit_iter_loop should be explicit_for
  • single_match should be explicit_if_let
  • for_kv_map should be excessive_entry_iterators
@oli-obk
Copy link
Contributor

oli-obk commented Jun 14, 2018

you can use ./util/update-lints.py -n to dump a markdown list of lints

@ghost
Copy link

ghost commented Jun 16, 2018

needless and useless are basically used as synonyms in a few lint names. It would be more consistent to use just one. needless is used more often and useless has negative connotations so I prefer needless.

@oli-obk oli-obk added the S-needs-discussion Status: Needs further discussion before merging or work can be started label Jun 16, 2018
@phansch phansch added this to the 1.0 milestone Jun 20, 2018
@clarfonthey
Copy link
Contributor Author

So you all know, I just looked at a bunch of lints and came up with a fair number of new rules. I'd love feedback on some of these.

I mostly read down @Manishearth's RFC and figured out new names for lints.

@phansch
Copy link
Member

phansch commented Aug 19, 2019

needless and useless are basically used as synonyms in a few lint names. It would be more consistent to use just one. needless is used more often and useless has negative connotations so I prefer needless.

Not a native English speaker, I wonder if the same would be the case for unnecessary? We have a couple of those, too:

  • unnecessary_cast
  • unnecessary_filter_map
  • unnecessary_fold
  • unnecessary_mut_passed
  • unnecessary_operation
  • unnecessary_unwrap

Using needless would make the lint names a bit shorter and easier to write.

@timClicks
Copy link

I believe that we should remove blacklist and whitelist. Perhaps blacklisted_names could be replaced with uninformative_names?

@Manishearth
Copy link
Member

Manishearth commented Jan 19, 2022

The typical replacement for those is allowlist and denylist, we should do that. uninformative_names isn't great because that lint is supposed to be used as a denylist for wider purposes, and is customizeable from the config.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 22, 2023
…lints, r=fee1-dead

Rename `{drop,forget}_{copy,ref}` lints to more consistent naming

This PR renames previous uplifted lints in rust-lang#109732 to more consistent naming.

I followed the renaming done [here](rust-lang#53224) and also advocated in this [clippy issue](rust-lang/rust-clippy#2845):
   - `drop_copy` to `dropping_copy_types`
   - `forget_copy` to `forgetting_copy_types`
   - `drop_ref` to `dropping_references`
   - `forget_ref` to `forgetting_references`
oli-obk pushed a commit to oli-obk/miri that referenced this issue May 23, 2023
…fee1-dead

Rename `{drop,forget}_{copy,ref}` lints to more consistent naming

This PR renames previous uplifted lints in rust-lang/rust#109732 to more consistent naming.

I followed the renaming done [here](rust-lang/rust#53224) and also advocated in this [clippy issue](rust-lang/rust-clippy#2845):
   - `drop_copy` to `dropping_copy_types`
   - `forget_copy` to `forgetting_copy_types`
   - `drop_ref` to `dropping_references`
   - `forget_ref` to `forgetting_references`
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jul 18, 2023
…fee1-dead

Rename `{drop,forget}_{copy,ref}` lints to more consistent naming

This PR renames previous uplifted lints in rust-lang/rust#109732 to more consistent naming.

I followed the renaming done [here](rust-lang/rust#53224) and also advocated in this [clippy issue](rust-lang/rust-clippy#2845):
   - `drop_copy` to `dropping_copy_types`
   - `forget_copy` to `forgetting_copy_types`
   - `drop_ref` to `dropping_references`
   - `forget_ref` to `forgetting_references`
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 2, 2023
…=Nilstrieb

Improve `invalid_reference_casting` lint

This PR is a follow-up to rust-lang#111567 and rust-lang#113422.

This PR does multiple things:
 - First it adds support for deferred de-reference, the goal is to support code like this, where the casting and de-reference are not done on the same expression
    ```rust
    let myself = self as *const Self as *mut Self;
    *myself = Self::Ready(value);
    ```
 - Second it does not lint anymore on SB/TB UB code by only checking assignments (`=`, ` =`, ...) and creation of mutable references `&mut *`
 - Thirdly it greatly improves the diagnostics in particular for cast from `&mut` to `&mut` or assignments
 - ~~And lastly it renames the lint from `cast_ref_to_mut` to `invalid_reference_casting` which is more consistent with the ["rules"](rust-lang/rust-clippy#2845) and also more consistent with what the lint checks~~ *rust-lang#113422

This PR is best reviewed commit by commit.

r? compiler
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Aug 3, 2023
Improve `invalid_reference_casting` lint

This PR is a follow-up to rust-lang/rust#111567 and rust-lang/rust#113422.

This PR does multiple things:
 - First it adds support for deferred de-reference, the goal is to support code like this, where the casting and de-reference are not done on the same expression
    ```rust
    let myself = self as *const Self as *mut Self;
    *myself = Self::Ready(value);
    ```
 - Second it does not lint anymore on SB/TB UB code by only checking assignments (`=`, ` =`, ...) and creation of mutable references `&mut *`
 - Thirdly it greatly improves the diagnostics in particular for cast from `&mut` to `&mut` or assignments
 - ~~And lastly it renames the lint from `cast_ref_to_mut` to `invalid_reference_casting` which is more consistent with the ["rules"](rust-lang/rust-clippy#2845) and also more consistent with what the lint checks~~ *rust-lang/rust#113422

This PR is best reviewed commit by commit.

r? compiler
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Oct 17, 2023
Improve `invalid_reference_casting` lint

This PR is a follow-up to rust-lang/rust#111567 and rust-lang/rust#113422.

This PR does multiple things:
 - First it adds support for deferred de-reference, the goal is to support code like this, where the casting and de-reference are not done on the same expression
    ```rust
    let myself = self as *const Self as *mut Self;
    *myself = Self::Ready(value);
    ```
 - Second it does not lint anymore on SB/TB UB code by only checking assignments (`=`, ` =`, ...) and creation of mutable references `&mut *`
 - Thirdly it greatly improves the diagnostics in particular for cast from `&mut` to `&mut` or assignments
 - ~~And lastly it renames the lint from `cast_ref_to_mut` to `invalid_reference_casting` which is more consistent with the ["rules"](rust-lang/rust-clippy#2845) and also more consistent with what the lint checks~~ *rust-lang/rust#113422

This PR is best reviewed commit by commit.

r? compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-discussion Status: Needs further discussion before merging or work can be started
Projects
None yet
Development

No branches or pull requests

5 participants