Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
smoelius committed Dec 15, 2024
1 parent 7d99b5d commit 57fb2f1
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 44 deletions.
12 changes: 4 additions & 8 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -563,14 +563,10 @@ A list of paths to types that should be treated as if they do not contain interi


## `initializer-suggestions`
Suggestion behavior when initializers are present. Options are:
Whether to suggest reordering constructor fields when initializers are present.

- "none": do not suggest
- "maybe-incorrect": suggest, but do not apply suggestions with `--fix`
- "machine-applicable": suggest and apply suggestions with `--fix`

The following example [due to @ronnodas] shows why "maybe-incorrect" may be the right choice.
Swapping the fields in the constructor produces incompilable code:
Note that such suggestions are not applied automatically with `--fix`. The following example
[due to @ronnodas] shows why. Swapping the fields in the constructor produces incompilable code:

```rust
struct MyStruct {
Expand All @@ -585,7 +581,7 @@ fn main() {

[due to @ronnodas]: https://github.com/rust-lang/rust-clippy/issues/11846#issuecomment-1820747924

**Default Value:** `"none"`
**Default Value:** `false`

---
**Affected lints:**
Expand Down
2 changes: 1 addition & 1 deletion clippy.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
avoid-breaking-exported-api = false

initializer-suggestions = "machine-applicable"
initializer-suggestions = true

[[disallowed-methods]]
path = "rustc_lint::context::LintContext::lint"
Expand Down
18 changes: 7 additions & 11 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::ClippyConfiguration;
use crate::types::{
DisallowedPath, InitializerSuggestionApplicability, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour,
Rename, SourceItemOrdering, SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings,
SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename, SourceItemOrdering,
SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind,
SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
};
use clippy_utils::msrvs::Msrv;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -526,14 +526,10 @@ define_Conf! {
/// A list of paths to types that should be treated as if they do not contain interior mutability
#[lints(borrow_interior_mutable_const, declare_interior_mutable_const, ifs_same_cond, mutable_key_type)]
ignore_interior_mutability: Vec<String> = Vec::from(["bytes::Bytes".into()]),
/// Suggestion behavior when initializers are present. Options are:
/// Whether to suggest reordering constructor fields when initializers are present.
///
/// - "none": do not suggest
/// - "maybe-incorrect": suggest, but do not apply suggestions with `--fix`
/// - "machine-applicable": suggest and apply suggestions with `--fix`
///
/// The following example [due to @ronnodas] shows why "maybe-incorrect" may be the right choice.
/// Swapping the fields in the constructor produces incompilable code:
/// Note that such suggestions are not applied automatically with `--fix`. The following example
/// [due to @ronnodas] shows why. Swapping the fields in the constructor produces incompilable code:
///
/// ```rust
/// struct MyStruct {
Expand All @@ -548,7 +544,7 @@ define_Conf! {
///
/// [due to @ronnodas]: https://github.com/rust-lang/rust-clippy/issues/11846#issuecomment-1820747924
#[lints(inconsistent_struct_constructor)]
initializer_suggestions: InitializerSuggestionApplicability = InitializerSuggestionApplicability::None,
initializer_suggestions: bool = false,
/// The maximum size of the `Err`-variant in a `Result` returned from a function
#[lints(result_large_err)]
large_error_threshold: u64 = 128,
Expand Down
19 changes: 0 additions & 19 deletions clippy_config/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use clippy_utils::def_path_def_ids;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefIdMap;
use rustc_middle::ty::TyCtxt;
use serde::de::{self, Deserializer, Visitor};
Expand Down Expand Up @@ -47,24 +46,6 @@ pub fn create_disallowed_map(
.collect()
}

#[derive(Clone, Copy, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
pub enum InitializerSuggestionApplicability {
None,
MaybeIncorrect,
MachineApplicable,
}

impl InitializerSuggestionApplicability {
pub fn to_applicability(self) -> Option<Applicability> {
match self {
InitializerSuggestionApplicability::None => None,
InitializerSuggestionApplicability::MaybeIncorrect => Some(Applicability::MaybeIncorrect),
InitializerSuggestionApplicability::MachineApplicable => Some(Applicability::MachineApplicable),
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]
pub enum MatchLintBehaviour {
AllTypes,
Expand Down
7 changes: 3 additions & 4 deletions clippy_lints/src/inconsistent_struct_constructor.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use clippy_config::Conf;
use clippy_config::types::InitializerSuggestionApplicability;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::fulfill_or_allowed;
use clippy_utils::source::snippet_opt;
Expand Down Expand Up @@ -66,7 +65,7 @@ declare_clippy_lint! {
}

pub struct InconsistentStructConstructor {
initializer_suggestions: InitializerSuggestionApplicability,
initializer_suggestions: bool,
}

impl InconsistentStructConstructor {
Expand All @@ -86,8 +85,8 @@ impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor {
};
let applicability = if fields.iter().all(|f| f.is_shorthand) {
Applicability::MachineApplicable
} else if let Some(applicability) = self.initializer_suggestions.to_applicability() {
applicability
} else if self.initializer_suggestions {
Applicability::MaybeIncorrect
} else {
return;
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
initializer-suggestions = "machine-applicable"
initializer-suggestions = true

0 comments on commit 57fb2f1

Please sign in to comment.