diff --git a/clippy.toml b/clippy.toml index 77573105d86a..0a7724bbe4e6 100644 --- a/clippy.toml +++ b/clippy.toml @@ -7,11 +7,14 @@ lint-commented-code = true [[disallowed-methods]] path = "rustc_lint::context::LintContext::lint" reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint*` functions instead" +allow-invalid = true [[disallowed-methods]] path = "rustc_lint::context::LintContext::span_lint" reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint*` functions instead" +allow-invalid = true [[disallowed-methods]] path = "rustc_middle::ty::context::TyCtxt::node_span_lint" reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint_hir*` functions instead" +allow-invalid = true diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index bd20ee1fda4a..34c62318aad3 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -120,12 +120,7 @@ impl ConfError { Self { message: message.into(), suggestion, - span: Span::new( - file.start_pos + BytePos::from_usize(span.start), - file.start_pos + BytePos::from_usize(span.end), - SyntaxContext::root(), - None, - ), + span: span_from_toml_range(file, span), } } } @@ -176,11 +171,61 @@ macro_rules! default_text { }; } +macro_rules! deserialize { + ($map:expr, $ty:ty, $errors:expr, $file:expr) => {{ + let raw_value = $map.next_value::>()?; + let value_span = raw_value.span(); + let value = match <$ty>::deserialize(raw_value.into_inner()) { + Err(e) => { + $errors.push(ConfError::spanned( + $file, + e.to_string().replace('\n', " ").trim(), + None, + value_span, + )); + continue; + }, + Ok(value) => value, + }; + (value, value_span) + }}; + + ($map:expr, $ty:ty, $errors:expr, $file:expr, $replacements_allowed:expr) => {{ + let array = $map.next_value::>>()?; + let mut disallowed_paths_span = Range { + start: usize::MAX, + end: usize::MIN, + }; + let mut disallowed_paths = Vec::new(); + for raw_value in array { + let value_span = raw_value.span(); + let mut disallowed_path = match DisallowedPath::<$replacements_allowed>::deserialize(raw_value.into_inner()) + { + Err(e) => { + $errors.push(ConfError::spanned( + $file, + e.to_string().replace('\n', " ").trim(), + None, + value_span, + )); + continue; + }, + Ok(disallowed_path) => disallowed_path, + }; + disallowed_paths_span = union(&disallowed_paths_span, &value_span); + disallowed_path.set_span(span_from_toml_range($file, value_span)); + disallowed_paths.push(disallowed_path); + } + (disallowed_paths, disallowed_paths_span) + }}; +} + macro_rules! define_Conf { ($( $(#[doc = $doc:literal])+ $(#[conf_deprecated($dep:literal, $new_conf:ident)])? $(#[default_text = $default_text:expr])? + $(#[disallowed_paths_allow_replacements = $replacements_allowed:expr])? $(#[lints($($for_lints:ident),* $(,)?)])? $name:ident: $ty:ty = $default:expr, )*) => { @@ -219,7 +264,7 @@ macro_rules! define_Conf { let mut errors = Vec::new(); let mut warnings = Vec::new(); - // Declare a local variable for each field field available to a configuration file. + // Declare a local variable for each field available to a configuration file. $(let mut $name = None;)* // could get `Field` here directly, but get `String` first for diagnostics @@ -237,15 +282,8 @@ macro_rules! define_Conf { $(Field::$name => { // Is this a deprecated field, i.e., is `$dep` set? If so, push a warning. $(warnings.push(ConfError::spanned(self.0, format!("deprecated field `{}`. {}", name.get_ref(), $dep), None, name.span()));)? - let raw_value = map.next_value::>()?; - let value_span = raw_value.span(); - let value = match <$ty>::deserialize(raw_value.into_inner()) { - Err(e) => { - errors.push(ConfError::spanned(self.0, e.to_string().replace('\n', " ").trim(), None, value_span)); - continue; - } - Ok(value) => value - }; + let (value, value_span) = + deserialize!(map, $ty, errors, self.0 $(, $replacements_allowed)?); // Was this field set previously? if $name.is_some() { errors.push(ConfError::spanned(self.0, format!("duplicate field `{}`", name.get_ref()), None, name.span())); @@ -286,6 +324,22 @@ macro_rules! define_Conf { }; } +fn union(x: &Range, y: &Range) -> Range { + Range { + start: cmp::min(x.start, y.start), + end: cmp::max(x.end, y.end), + } +} + +fn span_from_toml_range(file: &SourceFile, span: Range) -> Span { + Span::new( + file.start_pos + BytePos::from_usize(span.start), + file.start_pos + BytePos::from_usize(span.end), + SyntaxContext::root(), + None, + ) +} + define_Conf! { /// Which crates to allow absolute paths from #[lints(absolute_paths)] @@ -472,6 +526,7 @@ define_Conf! { )] avoid_breaking_exported_api: bool = true, /// The list of types which may not be held across an await point. + #[disallowed_paths_allow_replacements = false] #[lints(await_holding_invalid_type)] await_holding_invalid_types: Vec = Vec::new(), /// DEPRECATED LINT: BLACKLISTED_NAME. @@ -517,9 +572,11 @@ define_Conf! { #[conf_deprecated("Please use `cognitive-complexity-threshold` instead", cognitive_complexity_threshold)] cyclomatic_complexity_threshold: u64 = 25, /// The list of disallowed macros, written as fully qualified paths. + #[disallowed_paths_allow_replacements = true] #[lints(disallowed_macros)] disallowed_macros: Vec = Vec::new(), /// The list of disallowed methods, written as fully qualified paths. + #[disallowed_paths_allow_replacements = true] #[lints(disallowed_methods)] disallowed_methods: Vec = Vec::new(), /// The list of disallowed names to lint about. NB: `bar` is not here since it has legitimate uses. The value @@ -528,6 +585,7 @@ define_Conf! { #[lints(disallowed_names)] disallowed_names: Vec = DEFAULT_DISALLOWED_NAMES.iter().map(ToString::to_string).collect(), /// The list of disallowed types, written as fully qualified paths. + #[disallowed_paths_allow_replacements = true] #[lints(disallowed_types)] disallowed_types: Vec = Vec::new(), /// The list of words this lint should not consider as identifiers needing ticks. The value diff --git a/clippy_config/src/lib.rs b/clippy_config/src/lib.rs index 5d6e8b875166..c227b8900b74 100644 --- a/clippy_config/src/lib.rs +++ b/clippy_config/src/lib.rs @@ -13,6 +13,7 @@ rustc::untranslatable_diagnostic )] +extern crate rustc_data_structures; extern crate rustc_errors; extern crate rustc_hir; extern crate rustc_middle; diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index 8faac9ecffea..5949eaca7bc1 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -1,5 +1,7 @@ -use clippy_utils::def_path_def_ids; +use rustc_data_structures::fx::FxHashMap; use rustc_errors::{Applicability, Diag}; +use rustc_hir::PrimTy; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefIdMap; use rustc_middle::ty::TyCtxt; use rustc_span::Span; @@ -21,6 +23,17 @@ pub struct DisallowedPath { path: String, reason: Option, replacement: Option, + /// Setting `allow_invalid` to true suppresses a warning if `path` does not refer to an existing + /// definition. + /// + /// This could be useful when conditional compilation is used, or when a clippy.toml file is + /// shared among multiple projects. + allow_invalid: bool, + /// The span of the `DisallowedPath`. + /// + /// Used for diagnostics. + #[serde(skip_serializing)] + span: Span, } impl<'de, const REPLACEMENT_ALLOWED: bool> Deserialize<'de> for DisallowedPath { @@ -36,6 +49,8 @@ impl<'de, const REPLACEMENT_ALLOWED: bool> Deserialize<'de> for DisallowedPath, replacement: Option, + #[serde(rename = "allow-invalid")] + allow_invalid: Option, }, } @@ -58,7 +75,7 @@ impl DisallowedPath { &self.path } - pub fn diag_amendment(&self, span: Span) -> impl FnOnce(&mut Diag<'_, ()>) + use<'_, REPLACEMENT_ALLOWED> { + pub fn diag_amendment(&self, span: Span) -> impl FnOnce(&mut Diag<'_, ()>) { move |diag| { if let Some(replacement) = &self.replacement { diag.span_suggestion( @@ -72,6 +89,14 @@ impl DisallowedPath { } } } + + pub fn span(&self) -> Span { + self.span + } + + pub fn set_span(&mut self, span: Span) { + self.span = span; + } } impl DisallowedPathEnum { @@ -94,20 +119,87 @@ impl DisallowedPathEnum { Self::Simple(_) => None, } } + + fn allow_invalid(&self) -> bool { + match &self { + Self::WithReason { allow_invalid, .. } => allow_invalid.unwrap_or_default(), + Self::Simple(_) => false, + } + } } /// Creates a map of disallowed items to the reason they were disallowed. +#[allow(clippy::type_complexity)] pub fn create_disallowed_map( tcx: TyCtxt<'_>, - disallowed: &'static [DisallowedPath], -) -> DefIdMap<(&'static str, &'static DisallowedPath)> { - disallowed - .iter() - .map(|x| (x.path(), x.path().split("::").collect::>(), x)) - .flat_map(|(name, path, disallowed_path)| { - def_path_def_ids(tcx, &path).map(move |id| (id, (name, disallowed_path))) - }) - .collect() + disallowed_paths: &'static [DisallowedPath], + def_kind_predicate: impl Fn(DefKind) -> bool, + predicate_description: &str, + allow_prim_tys: bool, +) -> ( + DefIdMap<(&'static str, &'static DisallowedPath)>, + FxHashMap)>, +) { + let mut def_ids: DefIdMap<(&'static str, &'static DisallowedPath)> = DefIdMap::default(); + let mut prim_tys: FxHashMap)> = + FxHashMap::default(); + for disallowed_path in disallowed_paths { + let path = disallowed_path.path(); + let mut resolutions = clippy_utils::def_path_res(tcx, &path.split("::").collect::>()); + + let mut found_def_id = None; + let mut found_prim_ty = false; + resolutions.retain(|res| match res { + Res::Def(def_kind, def_id) => { + found_def_id = Some(*def_id); + def_kind_predicate(*def_kind) + }, + Res::PrimTy(_) => { + found_prim_ty = true; + allow_prim_tys + }, + _ => false, + }); + + if resolutions.is_empty() { + let span = disallowed_path.span(); + + if let Some(def_id) = found_def_id { + tcx.sess.dcx().span_warn( + span, + format!( + "expected a {predicate_description}, found {} {}", + tcx.def_descr_article(def_id), + tcx.def_descr(def_id) + ), + ); + } else if found_prim_ty { + tcx.sess.dcx().span_warn( + span, + format!("expected a {predicate_description}, found a primitive type",), + ); + } else if !disallowed_path.allow_invalid { + tcx.sess.dcx().span_warn( + span, + format!("`{path}` does not refer to an existing {predicate_description}"), + ); + } + } + + for res in resolutions { + match res { + Res::Def(_, def_id) => { + def_ids.insert(def_id, (path, disallowed_path)); + }, + Res::PrimTy(ty) => { + prim_tys.insert(ty, (path, disallowed_path)); + }, + _ => unreachable!(), + } + } + } + + (def_ids, prim_tys) } #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index d600aec8c9d6..52d1d5b4c67a 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -179,9 +179,14 @@ pub struct AwaitHolding { impl AwaitHolding { pub(crate) fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { - Self { - def_ids: create_disallowed_map(tcx, &conf.await_holding_invalid_types), - } + let (def_ids, _) = create_disallowed_map( + tcx, + &conf.await_holding_invalid_types, + crate::disallowed_types::def_kind_predicate, + "type", + false, + ); + Self { def_ids } } } diff --git a/clippy_lints/src/disallowed_macros.rs b/clippy_lints/src/disallowed_macros.rs index 22310bc0798f..fa33fef23062 100644 --- a/clippy_lints/src/disallowed_macros.rs +++ b/clippy_lints/src/disallowed_macros.rs @@ -3,6 +3,7 @@ use clippy_config::types::{DisallowedPath, create_disallowed_map}; use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; use clippy_utils::macros::macro_backtrace; use rustc_data_structures::fx::FxHashSet; +use rustc_hir::def::DefKind; use rustc_hir::def_id::DefIdMap; use rustc_hir::{ AmbigArg, Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty, @@ -71,8 +72,15 @@ pub struct DisallowedMacros { impl DisallowedMacros { pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf, earlies: AttrStorage) -> Self { + let (disallowed, _) = create_disallowed_map( + tcx, + &conf.disallowed_macros, + |def_kind| matches!(def_kind, DefKind::Macro(_)), + "macro", + false, + ); Self { - disallowed: create_disallowed_map(tcx, &conf.disallowed_macros), + disallowed, seen: FxHashSet::default(), derive_src: None, earlies, diff --git a/clippy_lints/src/disallowed_methods.rs b/clippy_lints/src/disallowed_methods.rs index 149cf1cf2def..1382dafa931e 100644 --- a/clippy_lints/src/disallowed_methods.rs +++ b/clippy_lints/src/disallowed_methods.rs @@ -63,9 +63,19 @@ pub struct DisallowedMethods { impl DisallowedMethods { pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { - Self { - disallowed: create_disallowed_map(tcx, &conf.disallowed_methods), - } + let (disallowed, _) = create_disallowed_map( + tcx, + &conf.disallowed_methods, + |def_kind| { + matches!( + def_kind, + DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::AssocFn + ) + }, + "function", + false, + ); + Self { disallowed } } } @@ -74,12 +84,7 @@ impl_lint_pass!(DisallowedMethods => [DISALLOWED_METHODS]); impl<'tcx> LateLintPass<'tcx> for DisallowedMethods { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { let (id, span) = match &expr.kind { - ExprKind::Path(path) - if let Res::Def(DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::AssocFn, id) = - cx.qpath_res(path, expr.hir_id) => - { - (id, expr.span) - }, + ExprKind::Path(path) if let Res::Def(_, id) = cx.qpath_res(path, expr.hir_id) => (id, expr.span), ExprKind::MethodCall(name, ..) if let Some(id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) => { (id, name.ident.span) }, diff --git a/clippy_lints/src/disallowed_types.rs b/clippy_lints/src/disallowed_types.rs index 38903596414c..2bae82648ac7 100644 --- a/clippy_lints/src/disallowed_types.rs +++ b/clippy_lints/src/disallowed_types.rs @@ -1,8 +1,8 @@ use clippy_config::Conf; -use clippy_config::types::DisallowedPath; +use clippy_config::types::{DisallowedPath, create_disallowed_map}; use clippy_utils::diagnostics::span_lint_and_then; use rustc_data_structures::fx::FxHashMap; -use rustc_hir::def::Res; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefIdMap; use rustc_hir::{AmbigArg, Item, ItemKind, PolyTraitRef, PrimTy, Ty, TyKind, UseKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -60,22 +60,7 @@ pub struct DisallowedTypes { impl DisallowedTypes { pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { - let mut def_ids = DefIdMap::default(); - let mut prim_tys = FxHashMap::default(); - for disallowed_path in &conf.disallowed_types { - let path: Vec<_> = disallowed_path.path().split("::").collect::>(); - for res in clippy_utils::def_path_res(tcx, &path) { - match res { - Res::Def(_, id) => { - def_ids.insert(id, (disallowed_path.path(), disallowed_path)); - }, - Res::PrimTy(ty) => { - prim_tys.insert(ty, (disallowed_path.path(), disallowed_path)); - }, - _ => {}, - } - } - } + let (def_ids, prim_tys) = create_disallowed_map(tcx, &conf.disallowed_types, def_kind_predicate, "type", true); Self { def_ids, prim_tys } } @@ -95,6 +80,19 @@ impl DisallowedTypes { } } +pub fn def_kind_predicate(def_kind: DefKind) -> bool { + matches!( + def_kind, + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::Trait + | DefKind::TyAlias + | DefKind::ForeignTy + | DefKind::AssocTy + ) +} + impl_lint_pass!(DisallowedTypes => [DISALLOWED_TYPES]); impl<'tcx> LateLintPass<'tcx> for DisallowedTypes { diff --git a/tests/ui-toml/await_holding_invalid_type_with_replacement/await_holding_invalid_type.stderr b/tests/ui-toml/await_holding_invalid_type_with_replacement/await_holding_invalid_type.stderr index 86e30409af06..d0fce3614a14 100644 --- a/tests/ui-toml/await_holding_invalid_type_with_replacement/await_holding_invalid_type.stderr +++ b/tests/ui-toml/await_holding_invalid_type_with_replacement/await_holding_invalid_type.stderr @@ -1,11 +1,8 @@ error: error reading Clippy's configuration file: replacement not allowed for this configuration - --> $DIR/tests/ui-toml/await_holding_invalid_type_with_replacement/clippy.toml:1:31 + --> $DIR/tests/ui-toml/await_holding_invalid_type_with_replacement/clippy.toml:2:5 | -LL | await-holding-invalid-types = [ - | _______________________________^ -LL | | { path = "std::string::String", replacement = "std::net::Ipv4Addr" }, -LL | | ] - | |_^ +LL | { path = "std::string::String", replacement = "std::net::Ipv4Addr" }, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 1 previous error diff --git a/tests/ui-toml/toml_invalid_path/clippy.toml b/tests/ui-toml/toml_invalid_path/clippy.toml new file mode 100644 index 000000000000..6d0d732a9223 --- /dev/null +++ b/tests/ui-toml/toml_invalid_path/clippy.toml @@ -0,0 +1,14 @@ +[[disallowed-types]] +path = "std::result::Result::Err" + +[[disallowed-macros]] +path = "bool" + +[[disallowed-methods]] +path = "std::process::current_exe" + +# negative test + +[[disallowed-methods]] +path = "std::current_exe" +allow-invalid = true diff --git a/tests/ui-toml/toml_invalid_path/conf_invalid_path.rs b/tests/ui-toml/toml_invalid_path/conf_invalid_path.rs new file mode 100644 index 000000000000..c15203827034 --- /dev/null +++ b/tests/ui-toml/toml_invalid_path/conf_invalid_path.rs @@ -0,0 +1,5 @@ +//@error-in-other-file: expected a macro, found a primitive type +//@error-in-other-file: `std::process::current_exe` does not refer to an existing function +//@error-in-other-file: expected a type, found a tuple variant + +fn main() {} diff --git a/tests/ui-toml/toml_invalid_path/conf_invalid_path.stderr b/tests/ui-toml/toml_invalid_path/conf_invalid_path.stderr new file mode 100644 index 000000000000..82550108eba5 --- /dev/null +++ b/tests/ui-toml/toml_invalid_path/conf_invalid_path.stderr @@ -0,0 +1,23 @@ +warning: expected a macro, found a primitive type + --> $DIR/tests/ui-toml/toml_invalid_path/clippy.toml:4:1 + | +LL | / [[disallowed-macros]] +LL | | path = "bool" + | |_____________^ + +warning: `std::process::current_exe` does not refer to an existing function + --> $DIR/tests/ui-toml/toml_invalid_path/clippy.toml:7:1 + | +LL | / [[disallowed-methods]] +LL | | path = "std::process::current_exe" + | |__________________________________^ + +warning: expected a type, found a tuple variant + --> $DIR/tests/ui-toml/toml_invalid_path/clippy.toml:1:1 + | +LL | / [[disallowed-types]] +LL | | path = "std::result::Result::Err" + | |_________________________________^ + +warning: 3 warnings emitted +