Skip to content

add documentation to the span_lint_hir functions #12445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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