From eb5ce85932e4b9fb7f7a1765f3760c3363a282d9 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 9 Mar 2024 19:40:39 +0100 Subject: [PATCH] mention `span_lint_hir` in `span_lint` and add a reason to disallowed_methods --- clippy.toml | 13 ++-- clippy_utils/src/diagnostics.rs | 81 +++++++++++++++++---- tests/ui-internal/disallow_span_lint.stderr | 3 + 3 files changed, 77 insertions(+), 20 deletions(-) diff --git a/clippy.toml b/clippy.toml index 3a68fa1670d..62ed55beb1f 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,7 +1,10 @@ avoid-breaking-exported-api = false -# use the various `clippy_utils::diagnostics::span_lint_*` functions instead, which also add a link to the docs -disallowed-methods = [ - "rustc_lint::context::LintContext::span_lint", - "rustc_middle::ty::context::TyCtxt::node_span_lint", -] +[[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" + + +[[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" diff --git a/clippy_utils/src/diagnostics.rs b/clippy_utils/src/diagnostics.rs index 303b23ba3a0..df551495993 100644 --- a/clippy_utils/src/diagnostics.rs +++ b/clippy_utils/src/diagnostics.rs @@ -36,6 +36,15 @@ fn docs_link(diag: &mut Diag<'_, ()>, lint: &'static Lint) { /// Usually it's nicer to provide more context for lint messages. /// Be sure the output is understandable when you use this method. /// +/// NOTE: only lint-level attributes at the `LintPass::check_*` node from which you are calling this +/// will be considered. +/// This can be confusing if the given span is at a different place, because users won't know where +/// `#[allow]` or `#[expect]` attributes need to be placed. +/// +/// This can happen if, for example, you are in `LintPass::check_block` and you are emitting a lint +/// for a particular expression within that block. +/// In those cases, consider using [`span_lint_hir`], and pass the `HirId` of that expression. +/// /// # Example /// /// ```ignore @@ -61,6 +70,15 @@ pub fn span_lint(cx: &T, lint: &'static Lint, sp: impl Into( /// /// If you change the signature, remember to update the internal lint `CollapsibleCalls` /// +/// NOTE: only lint-level attributes at the `LintPass::check_*` node from which you are calling this +/// will be considered. +/// This can be confusing if the given span is at a different place, because users won't know where +/// `#[allow]` or `#[expect]` attributes need to be placed. +/// +/// This can happen if, for example, you are in `LintPass::check_block` and you are emitting a lint +/// for a particular expression within that block. +/// In those cases, consider using [`span_lint_hir`], and pass the `HirId` of that expression. +/// /// # Example /// /// ```text @@ -139,6 +166,15 @@ pub fn span_lint_and_note( /// /// If you need to customize your lint output a lot, use this function. /// If you change the signature, remember to update the internal lint `CollapsibleCalls` +/// +/// NOTE: only lint-level attributes at the `LintPass::check_*` node from which you are calling this +/// will be considered. +/// This can be confusing if the given span is at a different place, because users won't know where +/// `#[allow]` or `#[expect]` attributes need to be placed. +/// +/// This can happen if, for example, you are in `LintPass::check_block` and you are emitting a lint +/// for a particular expression within that block. +/// In those cases, consider using [`span_lint_hir`], and pass the `HirId` of that expression. pub fn span_lint_and_then(cx: &C, lint: &'static Lint, sp: S, msg: &str, f: F) where C: LintContext, @@ -155,22 +191,25 @@ pub fn span_lint_and_then(cx: &C, lint: &'static Lint, sp: S, msg: &str /// Like [`span_lint`], but allows providing the `HirId` of the node that caused us to emit this /// lint. /// -/// The `HirId` is used for checking lint level attributes. +/// The `HirId` is used for checking lint level attributes and to fulfill lint expectations defined +/// via the `#[expect]` attribute. /// /// For example: /// ```ignore -/// fn f() { /* '1 */ +/// fn f() { /* */ /// /// #[allow(clippy::some_lint)] -/// let _x = /* '2 */; +/// let _x = /* */; /// } /// ``` -/// If `some_lint` does its analysis in `LintPass::check_fn` (at `'1`) and emits a lint at `'2` -/// using [`span_lint`], then allowing the lint at `'2` as attempted in the snippet will not work! +/// If `some_lint` does its analysis in `LintPass::check_fn` (at ``) and emits a lint at +/// `` using [`span_lint`], then allowing the lint at `` as attempted in the snippet +/// will not work! /// Even though that is where the warning points at, which would be confusing to users. /// -/// Instead, use this function and also pass the `HirId` of the node at `'2`, which will let the -/// compiler check lint level attributes at `'2` as one would expect, and the `#[allow]` will work. +/// Instead, use this function and also pass the `HirId` of ``, which will let +/// the compiler check lint level attributes at the place of the expression and +/// the `#[allow]` will work. pub fn span_lint_hir(cx: &LateContext<'_>, lint: &'static Lint, hir_id: HirId, sp: Span, msg: &str) { #[expect(clippy::disallowed_methods)] cx.tcx.node_span_lint(lint, hir_id, sp, msg.to_string(), |diag| { @@ -181,22 +220,25 @@ pub fn span_lint_hir(cx: &LateContext<'_>, lint: &'static Lint, hir_id: HirId, s /// Like [`span_lint_and_then`], but allows providing the `HirId` of the node that caused us to emit /// this lint. /// -/// The `HirId` is used for checking lint level attributes. +/// The `HirId` is used for checking lint level attributes and to fulfill lint expectations defined +/// via the `#[expect]` attribute. /// /// For example: /// ```ignore -/// fn f() { /* '1 */ +/// fn f() { /* */ /// /// #[allow(clippy::some_lint)] -/// let _x = /* '2 */; +/// let _x = /* */; /// } /// ``` -/// If `some_lint` does its analysis in `LintPass::check_fn` (at `'1`) and emits a lint at `'2` -/// using [`span_lint_and_then`], then allowing the lint at `'2` as attempted in the snippet will -/// not work! Even though that is where the warning points at, which would be confusing to users. +/// If `some_lint` does its analysis in `LintPass::check_fn` (at ``) and emits a lint at +/// `` using [`span_lint`], then allowing the lint at `` as attempted in the snippet +/// will not work! +/// Even though that is where the warning points at, which would be confusing to users. /// -/// Instead, use this function and also pass the `HirId` of the node at `'2`, which will let the -/// compiler check lint level attributes at `'2` as one would expect, and the `#[allow]` will work. +/// Instead, use this function and also pass the `HirId` of ``, which will let +/// the compiler check lint level attributes at the place of the expression and +/// the `#[allow]` will work. pub fn span_lint_hir_and_then( cx: &LateContext<'_>, lint: &'static Lint, @@ -220,6 +262,15 @@ pub fn span_lint_hir_and_then( /// /// If you change the signature, remember to update the internal lint `CollapsibleCalls` /// +/// NOTE: only lint-level attributes at the `LintPass::check_*` node from which you are calling this +/// will be considered. +/// This can be confusing if the given span is at a different place, because users won't know where +/// `#[allow]` or `#[expect]` attributes need to be placed. +/// +/// This can happen if, for example, you are in `LintPass::check_block` and you are emitting a lint +/// for a particular expression within that block. +/// In those cases, consider using [`span_lint_hir`], and pass the `HirId` of that expression. +/// /// # Example /// /// ```text diff --git a/tests/ui-internal/disallow_span_lint.stderr b/tests/ui-internal/disallow_span_lint.stderr index ae5d6843406..cfc590bed36 100644 --- a/tests/ui-internal/disallow_span_lint.stderr +++ b/tests/ui-internal/disallow_span_lint.stderr @@ -4,6 +4,7 @@ error: use of a disallowed method `rustc_lint::context::LintContext::span_lint` LL | cx.span_lint(lint, span, msg, |_| {}); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | + = note: this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint*` functions instead (from clippy.toml) = note: `-D clippy::disallowed-methods` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::disallowed_methods)]` @@ -12,6 +13,8 @@ error: use of a disallowed method `rustc_middle::ty::context::TyCtxt::node_span_ | LL | tcx.node_span_lint(lint, hir_id, span, msg, |_| {}); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint_hir*` functions instead (from clippy.toml) error: aborting due to 2 previous errors