Skip to content

Commit

Permalink
Auto merge of #12445 - y21:document-diagnostic-utils, r=xFrednet
Browse files Browse the repository at this point in the history
add documentation to the `span_lint_hir` functions

As far as I could tell, these weren't documented anywhere, and since this is sometimes needed over `span_lint` for `#[allow]` attrs to work, I thought I would add a little bit of documentation.
When I started with clippy development, I also had no idea what these functions were for.

changelog: none
  • Loading branch information
bors committed Mar 10, 2024
2 parents 81debac + 5b1f95c commit 86717f2
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 5 deletions.
13 changes: 8 additions & 5 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
avoid-breaking-exported-api = false

# use the various `span_lint_*` methods 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"
118 changes: 118 additions & 0 deletions clippy_utils/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ 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: Lint emissions are always bound to a node in the HIR, which is used to determine
/// the lint level.
/// For the `span_lint` function, the node that was passed into the `LintPass::check_*` function is
/// used.
///
/// If you're emitting the lint at the span of a different node than the one provided by the
/// `LintPass::check_*` function, consider using [`span_lint_hir`] instead.
/// This is needed for `#[allow]` and `#[expect]` attributes to work on the node
/// highlighted in the displayed warning.
///
/// If you're unsure which function you should use, you can test if the `#[allow]` attribute works
/// where you would expect it to.
/// If it doesn't, you likely need to use [`span_lint_hir`] instead.
///
/// # Example
///
/// ```ignore
Expand All @@ -61,6 +75,20 @@ pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<Mult
///
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
///
/// NOTE: Lint emissions are always bound to a node in the HIR, which is used to determine
/// the lint level.
/// For the `span_lint_and_help` function, the node that was passed into the `LintPass::check_*`
/// function is used.
///
/// If you're emitting the lint at the span of a different node than the one provided by the
/// `LintPass::check_*` function, consider using [`span_lint_hir_and_then`] instead.
/// This is needed for `#[allow]` and `#[expect]` attributes to work on the node
/// highlighted in the displayed warning.
///
/// If you're unsure which function you should use, you can test if the `#[allow]` attribute works
/// where you would expect it to.
/// If it doesn't, you likely need to use [`span_lint_hir_and_then`] instead.
///
/// # Example
///
/// ```text
Expand Down Expand Up @@ -99,6 +127,20 @@ pub fn span_lint_and_help<T: LintContext>(
///
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
///
/// NOTE: Lint emissions are always bound to a node in the HIR, which is used to determine
/// the lint level.
/// For the `span_lint_and_note` function, the node that was passed into the `LintPass::check_*`
/// function is used.
///
/// If you're emitting the lint at the span of a different node than the one provided by the
/// `LintPass::check_*` function, consider using [`span_lint_hir_and_then`] instead.
/// This is needed for `#[allow]` and `#[expect]` attributes to work on the node
/// highlighted in the displayed warning.
///
/// If you're unsure which function you should use, you can test if the `#[allow]` attribute works
/// where you would expect it to.
/// If it doesn't, you likely need to use [`span_lint_hir_and_then`] instead.
///
/// # Example
///
/// ```text
Expand Down Expand Up @@ -139,6 +181,20 @@ pub fn span_lint_and_note<T: LintContext>(
///
/// 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: Lint emissions are always bound to a node in the HIR, which is used to determine
/// the lint level.
/// For the `span_lint_and_then` function, the node that was passed into the `LintPass::check_*`
/// function is used.
///
/// If you're emitting the lint at the span of a different node than the one provided by the
/// `LintPass::check_*` function, consider using [`span_lint_hir_and_then`] instead.
/// This is needed for `#[allow]` and `#[expect]` attributes to work on the node
/// highlighted in the displayed warning.
///
/// If you're unsure which function you should use, you can test if the `#[allow]` attribute works
/// where you would expect it to.
/// If it doesn't, you likely need to use [`span_lint_hir_and_then`] instead.
pub fn span_lint_and_then<C, S, F>(cx: &C, lint: &'static Lint, sp: S, msg: &str, f: F)
where
C: LintContext,
Expand All @@ -152,13 +208,61 @@ where
});
}

/// Like [`span_lint`], but emits the lint at the node identified by the given `HirId`.
///
/// This is in contrast to [`span_lint`], which always emits the lint at the node that was last
/// passed to the `LintPass::check_*` function.
///
/// The `HirId` is used for checking lint level attributes and to fulfill lint expectations defined
/// via the `#[expect]` attribute.
///
/// For example:
/// ```ignore
/// fn f() { /* <node_1> */
///
/// #[allow(clippy::some_lint)]
/// let _x = /* <expr_1> */;
/// }
/// ```
/// If `some_lint` does its analysis in `LintPass::check_fn` (at `<node_1>`) and emits a lint at
/// `<expr_1>` using [`span_lint`], then allowing the lint at `<expr_1>` 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 `<expr_1>`, 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| {
docs_link(diag, lint);
});
}

/// Like [`span_lint_and_then`], but emits the lint at the node identified by the given `HirId`.
///
/// This is in contrast to [`span_lint_and_then`], which always emits the lint at the node that was
/// last passed to the `LintPass::check_*` function.
///
/// The `HirId` is used for checking lint level attributes and to fulfill lint expectations defined
/// via the `#[expect]` attribute.
///
/// For example:
/// ```ignore
/// fn f() { /* <node_1> */
///
/// #[allow(clippy::some_lint)]
/// let _x = /* <expr_1> */;
/// }
/// ```
/// If `some_lint` does its analysis in `LintPass::check_fn` (at `<node_1>`) and emits a lint at
/// `<expr_1>` using [`span_lint`], then allowing the lint at `<expr_1>` 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 `<expr_1>`, 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,
Expand All @@ -182,6 +286,20 @@ pub fn span_lint_hir_and_then(
///
/// If you change the signature, remember to update the internal lint `CollapsibleCalls`
///
/// NOTE: Lint emissions are always bound to a node in the HIR, which is used to determine
/// the lint level.
/// For the `span_lint_and_sugg` function, the node that was passed into the `LintPass::check_*`
/// function is used.
///
/// If you're emitting the lint at the span of a different node than the one provided by the
/// `LintPass::check_*` function, consider using [`span_lint_hir_and_then`] instead.
/// This is needed for `#[allow]` and `#[expect]` attributes to work on the node
/// highlighted in the displayed warning.
///
/// If you're unsure which function you should use, you can test if the `#[allow]` attribute works
/// where you would expect it to.
/// If it doesn't, you likely need to use [`span_lint_hir_and_then`] instead.
///
/// # Example
///
/// ```text
Expand Down
3 changes: 3 additions & 0 deletions tests/ui-internal/disallow_span_lint.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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)]`

Expand All @@ -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

0 comments on commit 86717f2

Please sign in to comment.