Skip to content

Commit

Permalink
Refactor for using config values:
Browse files Browse the repository at this point in the history
* Construct lint passes by taking `Conf` by reference.
* Use `HashSet` configs in less places
* Move some `check_crate` code into the pass constructor when possible.
  • Loading branch information
Jarcho committed Jul 17, 2024
1 parent 0ee9f44 commit e34c6db
Show file tree
Hide file tree
Showing 115 changed files with 824 additions and 994 deletions.
14 changes: 5 additions & 9 deletions book/src/development/adding_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -458,9 +458,8 @@ pub struct ManualStrip {
}

impl ManualStrip {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
pub fn new(conf: &'static Conf) -> Self {
Self { msrv: conf.msrv.clone() }
}
}
```
Expand Down Expand Up @@ -689,7 +688,6 @@ for some users. Adding a configuration is done in the following steps:
]);

// New manual definition struct
#[derive(Copy, Clone)]
pub struct StructName {}

impl_lint_pass!(StructName => [
Expand All @@ -700,17 +698,16 @@ for some users. Adding a configuration is done in the following steps:
2. Next add the configuration value and a corresponding creation method like
this:
```rust
#[derive(Copy, Clone)]
pub struct StructName {
configuration_ident: Type,
}

// ...

impl StructName {
pub fn new(configuration_ident: Type) -> Self {
pub fn new(conf: &'static Conf) -> Self {
Self {
configuration_ident,
configuration_ident: conf.configuration_ident,
}
}
}
Expand All @@ -726,8 +723,7 @@ for some users. Adding a configuration is done in the following steps:
store.register_*_pass(|| box module::StructName);

// New registration with configuration value
let configuration_ident = conf.configuration_ident.clone();
store.register_*_pass(move || box module::StructName::new(configuration_ident));
store.register_*_pass(move || box module::StructName::new(conf));
```

Congratulations the work is almost done. The configuration value can now be
Expand Down
2 changes: 1 addition & 1 deletion book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ default configuration of Clippy. By default, any configuration will replace the
* `doc-valid-idents = ["ClipPy"]` would replace the default list with `["ClipPy"]`.
* `doc-valid-idents = ["ClipPy", ".."]` would append `ClipPy` to the default list.

**Default Value:** `["KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "AccessKit", "CoreFoundation", "CoreGraphics", "CoreText", "DevOps", "Direct2D", "Direct3D", "DirectWrite", "DirectX", "ECMAScript", "GPLv2", "GPLv3", "GitHub", "GitLab", "IPv4", "IPv6", "ClojureScript", "CoffeeScript", "JavaScript", "PostScript", "PureScript", "TypeScript", "WebAssembly", "NaN", "NaNs", "OAuth", "GraphQL", "OCaml", "OpenAL", "OpenDNS", "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenTelemetry", "OpenType", "WebGL", "WebGL2", "WebGPU", "WebRTC", "WebSocket", "WebTransport", "WebP", "OpenExr", "YCbCr", "sRGB", "TensorFlow", "TrueType", "iOS", "macOS", "FreeBSD", "NetBSD", "OpenBSD", "TeX", "LaTeX", "BibTeX", "BibLaTeX", "MinGW", "CamelCase"]`
**Default Value:** `["TiB", "CoreGraphics", "CoffeeScript", "TeX", "Direct2D", "PiB", "DirectX", "NetBSD", "OAuth", "NaN", "OpenType", "WebGL2", "WebTransport", "JavaScript", "OpenSSL", "OpenSSH", "EiB", "PureScript", "OpenAL", "MiB", "WebAssembly", "MinGW", "CoreFoundation", "WebGPU", "ClojureScript", "CamelCase", "OpenDNS", "NaNs", "OpenMP", "GitLab", "KiB", "sRGB", "CoreText", "macOS", "TypeScript", "GiB", "OpenExr", "YCbCr", "OpenTelemetry", "OpenBSD", "FreeBSD", "GPLv2", "PostScript", "WebP", "LaTeX", "TensorFlow", "AccessKit", "TrueType", "OpenStreetMap", "OpenGL", "DevOps", "OCaml", "WebRTC", "WebGL", "BibLaTeX", "GitHub", "GraphQL", "iOS", "Direct3D", "BibTeX", "DirectWrite", "GPLv3", "IPv6", "WebSocket", "IPv4", "ECMAScript"]`

---
**Affected lints:**
Expand Down
14 changes: 9 additions & 5 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ define_Conf! {
///
/// A type, say `SomeType`, listed in this configuration has the same behavior of
/// `["SomeType" , "*"], ["*", "SomeType"]` in `arithmetic_side_effects_allowed_binary`.
(arithmetic_side_effects_allowed: FxHashSet<String> = <_>::default()),
(arithmetic_side_effects_allowed: Vec<String> = <_>::default()),
/// Lint: ARITHMETIC_SIDE_EFFECTS.
///
/// Suppress checking of the passed type pair names in binary operations like addition or
Expand All @@ -265,7 +265,7 @@ define_Conf! {
/// ```toml
/// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
/// ```
(arithmetic_side_effects_allowed_unary: FxHashSet<String> = <_>::default()),
(arithmetic_side_effects_allowed_unary: Vec<String> = <_>::default()),
/// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX, UNNECESSARY_BOX_RETURNS, SINGLE_CALL_FN, NEEDLESS_PASS_BY_REF_MUT.
///
/// Suppress lints whenever the suggested change would cause breakage for other crates.
Expand Down Expand Up @@ -314,7 +314,7 @@ define_Conf! {
/// default configuration of Clippy. By default, any configuration will replace the default value. For example:
/// * `doc-valid-idents = ["ClipPy"]` would replace the default list with `["ClipPy"]`.
/// * `doc-valid-idents = ["ClipPy", ".."]` would append `ClipPy` to the default list.
(doc_valid_idents: Vec<String> = DEFAULT_DOC_VALID_IDENTS.iter().map(ToString::to_string).collect()),
(doc_valid_idents: FxHashSet<String> = DEFAULT_DOC_VALID_IDENTS.iter().map(ToString::to_string).collect()),
/// Lint: TOO_MANY_ARGUMENTS.
///
/// The maximum number of argument a function or method can have
Expand Down Expand Up @@ -550,7 +550,7 @@ define_Conf! {
/// Lint: PATH_ENDS_WITH_EXT.
///
/// Additional dotfiles (files or directories starting with a dot) to allow
(allowed_dotfiles: FxHashSet<String> = FxHashSet::default()),
(allowed_dotfiles: Vec<String> = Vec::default()),
/// Lint: MULTIPLE_CRATE_VERSIONS.
///
/// A list of crate names to allow duplicates of
Expand Down Expand Up @@ -703,7 +703,6 @@ pub fn lookup_conf_file() -> io::Result<(Option<PathBuf>, Vec<String>)> {
fn deserialize(file: &SourceFile) -> TryConf {
match toml::de::Deserializer::new(file.src.as_ref().unwrap()).deserialize_map(ConfVisitor(file)) {
Ok(mut conf) => {
extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS);
extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES);
extend_vec_if_indicator_present(&mut conf.conf.allowed_prefixes, DEFAULT_ALLOWED_PREFIXES);
extend_vec_if_indicator_present(
Expand All @@ -716,6 +715,11 @@ fn deserialize(file: &SourceFile) -> TryConf {
.allowed_idents_below_min_chars
.extend(DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS.iter().map(ToString::to_string));
}
if conf.conf.doc_valid_idents.contains("..") {
conf.conf
.doc_valid_idents
.extend(DEFAULT_DOC_VALID_IDENTS.iter().map(ToString::to_string));
}

conf
},
Expand Down
14 changes: 6 additions & 8 deletions clippy_config/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ use serde::de::{self, Deserializer, Visitor};
use serde::{ser, Deserialize, Serialize};
use std::fmt;

#[derive(Clone, Debug, Deserialize)]
#[derive(Debug, Deserialize)]
pub struct Rename {
pub path: String,
pub rename: String,
}

#[derive(Clone, Debug, Deserialize)]
#[derive(Debug, Deserialize)]
#[serde(untagged)]
pub enum DisallowedPath {
Simple(String),
Expand All @@ -22,12 +22,10 @@ impl DisallowedPath {
path
}

pub fn reason(&self) -> Option<String> {
match self {
Self::WithReason {
reason: Some(reason), ..
} => Some(format!("{reason} (from clippy.toml)")),
_ => None,
pub fn reason(&self) -> Option<&str> {
match &self {
Self::WithReason { reason, .. } => reason.as_deref(),
Self::Simple(_) => None,
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ fn add_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> {

let new_lint = if enable_msrv {
format!(
"store.register_{lint_pass}_pass(move |{ctor_arg}| Box::new({module_name}::{camel_name}::new(msrv())));\n ",
"store.register_{lint_pass}_pass(move |{ctor_arg}| Box::new({module_name}::{camel_name}::new(conf)));\n ",
lint_pass = lint.pass,
ctor_arg = if lint.pass == "late" { "_" } else { "" },
module_name = lint.name,
Expand Down Expand Up @@ -274,6 +274,7 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
formatdoc!(
r#"
use clippy_config::msrvs::{{self, Msrv}};
use clippy_config::Conf;
{pass_import}
use rustc_lint::{{{context_import}, {pass_type}, LintContext}};
use rustc_session::impl_lint_pass;
Expand Down Expand Up @@ -301,9 +302,8 @@ fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String {
}}
impl {name_camel} {{
#[must_use]
pub fn new(msrv: Msrv) -> Self {{
Self {{ msrv }}
pub fn new(conf: &'static Conf) -> Self {{
Self {{ msrv: conf.msrv.clone() }}
}}
}}
Expand Down
12 changes: 11 additions & 1 deletion clippy_lints/src/absolute_paths.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::source::snippet_opt;
use rustc_data_structures::fx::FxHashSet;
Expand Down Expand Up @@ -47,7 +48,16 @@ impl_lint_pass!(AbsolutePaths => [ABSOLUTE_PATHS]);

pub struct AbsolutePaths {
pub absolute_paths_max_segments: u64,
pub absolute_paths_allowed_crates: FxHashSet<String>,
pub absolute_paths_allowed_crates: &'static FxHashSet<String>,
}

impl AbsolutePaths {
pub fn new(conf: &'static Conf) -> Self {
Self {
absolute_paths_max_segments: conf.absolute_paths_max_segments,
absolute_paths_allowed_crates: &conf.absolute_paths_allowed_crates,
}
}
}

impl LateLintPass<'_> for AbsolutePaths {
Expand Down
7 changes: 5 additions & 2 deletions clippy_lints/src/almost_complete_range.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{trim_span, walk_span_to_context};
use rustc_ast::ast::{Expr, ExprKind, LitKind, Pat, PatKind, RangeEnd, RangeLimits};
Expand Down Expand Up @@ -34,8 +35,10 @@ pub struct AlmostCompleteRange {
msrv: Msrv,
}
impl AlmostCompleteRange {
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}
}
impl EarlyLintPass for AlmostCompleteRange {
Expand Down
8 changes: 5 additions & 3 deletions clippy_lints/src/approx_const.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_help;
use rustc_ast::ast::{FloatTy, LitFloatType, LitKind};
use rustc_hir::{Expr, ExprKind};
Expand Down Expand Up @@ -67,9 +68,10 @@ pub struct ApproxConstant {
}

impl ApproxConstant {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}

fn check_lit(&self, cx: &LateContext<'_>, lit: &LitKind, e: &Expr<'_>) {
Expand Down
8 changes: 5 additions & 3 deletions clippy_lints/src/assigning_clones.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::mir::{enclosing_mir, PossibleBorrowerMap};
use clippy_utils::sugg::Sugg;
Expand Down Expand Up @@ -57,9 +58,10 @@ pub struct AssigningClones {
}

impl AssigningClones {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}
}

Expand Down
19 changes: 14 additions & 5 deletions clippy_lints/src/attrs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod useless_attribute;
mod utils;

use clippy_config::msrvs::{self, Msrv};
use clippy_config::Conf;
use rustc_ast::{Attribute, MetaItemKind, NestedMetaItem};
use rustc_hir::{ImplItem, Item, ItemKind, TraitItem};
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
Expand Down Expand Up @@ -499,7 +500,6 @@ declare_clippy_lint! {
"duplicated attribute"
}

#[derive(Clone)]
pub struct Attributes {
msrv: Msrv,
}
Expand All @@ -517,9 +517,10 @@ impl_lint_pass!(Attributes => [
]);

impl Attributes {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}
}

Expand Down Expand Up @@ -589,7 +590,15 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
}

pub struct EarlyAttributes {
pub msrv: Msrv,
msrv: Msrv,
}

impl EarlyAttributes {
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}
}

impl_lint_pass!(EarlyAttributes => [
Expand Down
39 changes: 12 additions & 27 deletions clippy_lints/src/await_holding_invalid.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use clippy_config::types::DisallowedPath;
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{match_def_path, paths};
use rustc_data_structures::fx::FxHashMap;
use clippy_utils::{create_disallowed_map, match_def_path, paths};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::def_id::{DefId, DefIdMap};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::mir::CoroutineLayout;
use rustc_middle::ty::TyCtxt;
use rustc_session::impl_lint_pass;
use rustc_span::{sym, Span};

Expand Down Expand Up @@ -172,31 +172,19 @@ declare_clippy_lint! {

impl_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF, AWAIT_HOLDING_INVALID_TYPE]);

#[derive(Debug)]
pub struct AwaitHolding {
conf_invalid_types: Vec<DisallowedPath>,
def_ids: FxHashMap<DefId, DisallowedPath>,
def_ids: DefIdMap<(&'static str, Option<&'static str>)>,
}

impl AwaitHolding {
pub(crate) fn new(conf_invalid_types: Vec<DisallowedPath>) -> Self {
pub(crate) fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
Self {
conf_invalid_types,
def_ids: FxHashMap::default(),
def_ids: create_disallowed_map(tcx, &conf.await_holding_invalid_types),
}
}
}

impl<'tcx> LateLintPass<'tcx> for AwaitHolding {
fn check_crate(&mut self, cx: &LateContext<'tcx>) {
for conf in &self.conf_invalid_types {
let segs: Vec<_> = conf.path().split("::").collect();
for id in clippy_utils::def_path_def_ids(cx, &segs) {
self.def_ids.insert(id, conf.clone());
}
}
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if let hir::ExprKind::Closure(hir::Closure {
kind: hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, _)),
Expand Down Expand Up @@ -258,25 +246,22 @@ impl AwaitHolding {
);
},
);
} else if let Some(disallowed) = self.def_ids.get(&adt.did()) {
emit_invalid_type(cx, ty_cause.source_info.span, disallowed);
} else if let Some(&(path, reason)) = self.def_ids.get(&adt.did()) {
emit_invalid_type(cx, ty_cause.source_info.span, path, reason);
}
}
}
}
}

fn emit_invalid_type(cx: &LateContext<'_>, span: Span, disallowed: &DisallowedPath) {
fn emit_invalid_type(cx: &LateContext<'_>, span: Span, path: &'static str, reason: Option<&'static str>) {
span_lint_and_then(
cx,
AWAIT_HOLDING_INVALID_TYPE,
span,
format!(
"`{}` may not be held across an await point per `clippy.toml`",
disallowed.path()
),
format!("holding a disallowed type across an await point `{path}`"),
|diag| {
if let Some(reason) = disallowed.reason() {
if let Some(reason) = reason {
diag.note(reason);
}
},
Expand Down
Loading

0 comments on commit e34c6db

Please sign in to comment.