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

Move non_send_fields_in_send_ty to suspicious #7874

Merged
merged 1 commit into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/lib.register_nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN),
LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),
LintId::of(mutex_atomic::MUTEX_INTEGER),
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_suspicious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
LintId::of(loops::MUT_RANGE_BOUND),
LintId::of(methods::SUSPICIOUS_MAP),
LintId::of(mut_key::MUTABLE_KEY_TYPE),
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
])
2 changes: 1 addition & 1 deletion clippy_lints/src/non_send_fields_in_send_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ declare_clippy_lint! {
/// Use thread-safe types like [`std::sync::Arc`](https://doc.rust-lang.org/std/sync/struct.Arc.html)
/// or specify correct bounds on generic type parameters (`T: Send`).
pub NON_SEND_FIELDS_IN_SEND_TY,
nursery,
suspicious,
"there is field that does not implement `Send` in a `Send` struct"
}

Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,10 @@ define_Conf! {
///
/// The list of imports to always rename, a fully qualified path followed by the rename.
(enforced_import_renames: Vec<crate::utils::conf::Rename> = Vec::new()),
/// Lint: RESTRICTED_SCRIPTS.
/// Lint: DISALLOWED_SCRIPT_IDENTS.
///
/// The list of unicode scripts allowed to be used in the scope.
(allowed_scripts: Vec<String> = vec!["Latin".to_string()]),
(allowed_scripts: Vec<String> = ["Latin"].iter().map(ToString::to_string).collect()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the reason for using an iterator here. What was wrong with using vec![_] before?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remembered an old implementation, that would only display the content inside the square brackets to make the documentation a bit cleaner. Now it actually uses format!("{:?}", default_value) which doesn't care about the formatting. The cases are therefore equivalent 😅

I would still suggest to switch it for consistency, as all other string vectors do the same. However, I'm also happy to revert this again 🙃

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps just add a comment that this test case also covers the distinction between vec! and some_iter.collect() so future reviewers don't have to wonder.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't really have any test case for the metadata collecting 😅. However, I can update the "Add configuration" documentation to say that the default value will be formatted with format!("{:?}", default)

/// Lint: NON_SEND_FIELDS_IN_SEND_TY.
///
/// Whether to apply the raw pointer heuristic to determine if a type is `Send`.
Expand Down
3 changes: 2 additions & 1 deletion doc/adding_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,8 @@ in the following steps:
/// <The configuration field doc comment>
(configuration_ident: Type = DefaultValue),
```
The doc comment will be automatically added to the lint documentation.
The doc comment is automatically added to the documentation of the listed lints. The default
value will be formatted using the `Debug` implementation of the type.
2. Adding the configuration value to the lint impl struct:
1. This first requires the definition of a lint impl struct. Lint impl structs are usually
generated with the `declare_lint_pass!` macro. This struct needs to be defined manually
Expand Down