Skip to content
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

Add register_lints_warn lint #1321

Merged
merged 2 commits into from
Sep 9, 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
1 change: 0 additions & 1 deletion clippy.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
disallowed-methods = [
{ path = "rustc_errors::DiagCtxtHandle::warn", reason = "ensure `DiagCtxtHandle::warn` is not called from `register_lints`" },
{ path = "std::process::Command::output", reason = "use `CommandExt::logged_output`" },
]
27 changes: 14 additions & 13 deletions examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,20 @@ The example libraries are separated into the following three categories:

## Restriction

| Example | Description/check |
| ------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------- |
| [`assert_eq_arg_misordering`](./restriction/assert_eq_arg_misordering) | `assert_eq!(actual, expected)` |
| [`collapsible_unwrap`](./restriction/collapsible_unwrap) | An `unwrap` that could be combined with an `expect` or `unwrap` using `and_then` |
| [`const_path_join`](./restriction/const_path_join) | Joining of constant path components |
| [`env_literal`](./restriction/env_literal) | Environment variables referred to with string literals |
| [`inconsistent_qualification`](./restriction/inconsistent_qualification) | Inconsistent qualification of module items |
| [`misleading_variable_name`](./restriction/misleading_variable_name) | Variables whose names suggest they have types other than the ones they have |
| [`overscoped_allow`](./restriction/overscoped_allow) | `allow` attributes whose scope could be reduced |
| [`question_mark_in_expression`](./restriction/question_mark_in_expression) | The `?` operator in expressions |
| [`ref_aware_redundant_closure_for_method_calls`](./restriction/ref_aware_redundant_closure_for_method_calls) | A ref-aware fork of `redundant_closure_for_method_calls` |
| [`suboptimal_pattern`](./restriction/suboptimal_pattern) | Patterns that could perform additional destructuring |
| [`try_io_result`](./restriction/try_io_result) | The `?` operator applied to `std::io::Result` |
| Example | Description/check |
| ------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------- |
| [`assert_eq_arg_misordering`](./restriction/assert_eq_arg_misordering) | `assert_eq!(actual, expected)` |
| [`collapsible_unwrap`](./restriction/collapsible_unwrap) | An `unwrap` that could be combined with an `expect` or `unwrap` using `and_then` |
| [`const_path_join`](./restriction/const_path_join) | Joining of constant path components |
| [`env_literal`](./restriction/env_literal) | Environment variables referred to with string literals |
| [`inconsistent_qualification`](./restriction/inconsistent_qualification) | Inconsistent qualification of module items |
| [`misleading_variable_name`](./restriction/misleading_variable_name) | Variables whose names suggest they have types other than the ones they have |
| [`overscoped_allow`](./restriction/overscoped_allow) | `allow` attributes whose scope could be reduced |
| [`question_mark_in_expression`](./restriction/question_mark_in_expression) | The `?` operator in expressions |
| [`ref_aware_redundant_closure_for_method_calls`](./restriction/ref_aware_redundant_closure_for_method_calls) | A ref-aware fork of `redundant_closure_for_method_calls` |
| [`register_lints_warn`](./restriction/register_lints_warn) | Calls to `rustc_errors::DiagCtxtHandle::warn` from within a `register_lints` function |
| [`suboptimal_pattern`](./restriction/suboptimal_pattern) | Patterns that could perform additional destructuring |
| [`try_io_result`](./restriction/try_io_result) | The `?` operator applied to `std::io::Result` |

## Experimental

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ impl MissingDocCommentOpenai {
impl<'tcx> LateLintPass<'tcx> for MissingDocCommentOpenai {
fn check_crate(&mut self, cx: &LateContext<'tcx>) {
if std::env::var(OPENAI_API_KEY).is_err() {
#[expect(clippy::disallowed_methods)]
cx.sess().dcx().warn(format!(
"`missing_doc_comment_openai` suggestions are disabled because environment \
variable `{OPENAI_API_KEY}` is not set"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@ where
FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => format!("`{ident}`"),
FnKind::Closure => "closure".to_owned(),
};
#[allow(clippy::disallowed_methods)]
self.cx.sess().dcx().warn(format!(
"reached work limit ({}) while checking {}; set `{}.work_limit` in `dylint.toml` \
to override",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ impl_lint_pass!(NonThreadSafeCallInTest => [NON_THREAD_SAFE_CALL_IN_TEST]);
impl<'tcx> LateLintPass<'tcx> for NonThreadSafeCallInTest {
fn check_crate(&mut self, cx: &LateContext<'tcx>) {
if !cx.sess().opts.test {
#[allow(clippy::disallowed_methods)]
cx.sess().dcx().warn(
"`non_thread_safe_call_in_test` is unlikely to be effective as `--test` was not \
passed to rustc",
Expand Down
9 changes: 9 additions & 0 deletions examples/restriction/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions examples/restriction/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ members = [
"overscoped_allow",
"question_mark_in_expression",
"ref_aware_redundant_closure_for_method_calls",
"register_lints_warn",
"suboptimal_pattern",
"try_io_result",
]
Expand Down
1 change: 0 additions & 1 deletion examples/restriction/overscoped_allow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ impl OverscopedAllow {
fn diagnostics(&self, cx: &LateContext<'_>) -> &Vec<Diagnostic> {
self.diagnostics.get_or_init(|| {
read_diagnostics().unwrap_or_else(|error| {
#[allow(clippy::disallowed_methods)]
cx.sess()
.dcx()
.warn(format!("`overscoped_allow` is disabled: {error:?}"));
Expand Down
24 changes: 24 additions & 0 deletions examples/restriction/register_lints_warn/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[package]
name = "register_lints_warn"
version = "3.1.2"
authors = ["Samuel E. Moelius III <sam@moeli.us>"]
description = "A lint to check for calls to `rustc_errors::DiagCtxtHandle::warn` from within a `register_lints` function"
edition = "2021"
publish = false

[lib]
crate-type = ["cdylib"]

[dependencies]
clippy_utils = { workspace = true }

dylint_linting = { path = "../../../utils/linting" }

[dev-dependencies]
dylint_testing = { path = "../../../utils/testing" }

[lints]
workspace = true

[package.metadata.rust-analyzer]
rustc_private = true
30 changes: 30 additions & 0 deletions examples/restriction/register_lints_warn/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# register_lints_warn

### What it does
Checks for calls to `rustc_errors::DiagCtxtHandle::warn` from within a `register_lints`
function.

### Why is this bad?
Dylint lists a library's lints by calling the library's `register_lints` function and
comparing the lints that are registered before and after the call. If the library's
`register_lints` functions emits warnings, they will be emitted when a user tries to list
the library's lints.

### Example
```rust
pub fn register_lints(sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) {
if condition {
sess.dcx().warn("something bad happened");
}
}
```
Use instead:
```rust
impl<'tcx> rustc_lint::LateLintPass<'tcx> for LintPass {
fn check_crate(&mut self, cx: &rustc_lint::LateContext<'tcx>) {
if condition {
cx.sess().dcx().warn("something bad happened");
}
}
}
```
87 changes: 87 additions & 0 deletions examples/restriction/register_lints_warn/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#![feature(rustc_private)]
#![feature(let_chains)]
#![warn(unused_extern_crates)]

extern crate rustc_hir;

use clippy_utils::{diagnostics::span_lint, match_def_path};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};

dylint_linting::declare_late_lint! {
/// ### What it does
/// Checks for calls to `rustc_errors::DiagCtxtHandle::warn` from within a `register_lints`
/// function.
///
/// ### Why is this bad?
/// Dylint lists a library's lints by calling the library's `register_lints` function and
/// comparing the lints that are registered before and after the call. If the library's
/// `register_lints` functions emits warnings, they will be emitted when a user tries to list
/// the library's lints.
///
/// ### Example
/// ```rust
/// # #![feature(rustc_private)]
/// # extern crate rustc_driver;
/// # extern crate rustc_lint;
/// # extern crate rustc_session;
/// pub fn register_lints(sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) {
/// # let condition = true;
/// if condition {
/// sess.dcx().warn("something bad happened");
/// }
/// }
/// ```
/// Use instead:
/// ```rust
/// # #![feature(rustc_private)]
/// # extern crate rustc_driver;
/// # extern crate rustc_lint;
/// # use rustc_lint::LintContext;
/// # struct LintPass;
/// # impl rustc_lint::LintPass for LintPass {
/// # fn name(&self) -> &'static str {
/// # "lint_pass"
/// # }
/// # }
/// impl<'tcx> rustc_lint::LateLintPass<'tcx> for LintPass {
/// fn check_crate(&mut self, cx: &rustc_lint::LateContext<'tcx>) {
/// # let condition = true;
/// if condition {
/// cx.sess().dcx().warn("something bad happened");
/// }
/// }
/// }
/// ```
pub REGISTER_LINTS_WARN,
Warn,
"calls to `rustc_errors::DiagCtxtHandle::warn` in a `register_lints` function"
}

impl<'tcx> LateLintPass<'tcx> for RegisterLintsWarn {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
if matches!(expr.kind, ExprKind::MethodCall(..))
&& let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
&& match_def_path(cx, def_id, &["rustc_errors", "DiagCtxtHandle", "warn"])
&& let local_def_id = cx.tcx.hir().enclosing_body_owner(expr.hir_id)
&& let hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id)
&& let Some(name) = cx.tcx.hir().opt_name(hir_id)
&& name.as_str() == "register_lints"
{
span_lint(
cx,
REGISTER_LINTS_WARN,
expr.span,
"call to `rustc_errors::DiagCtxtHandle::warn` from within a `register_lints` function",
);
}
}
}

#[test]
fn ui() {
dylint_testing::ui_test(
env!("CARGO_PKG_NAME"),
&std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("ui"),
);
}
26 changes: 26 additions & 0 deletions examples/restriction/register_lints_warn/ui/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#![feature(rustc_private)]

extern crate rustc_lint;
extern crate rustc_session;

pub fn register_lints(sess: &rustc_session::Session, _lint_store: &mut rustc_lint::LintStore) {
sess.dcx().warn("something bad happened");
}

use rustc_lint::LintContext;

struct LintPass;

impl rustc_lint::LintPass for LintPass {
fn name(&self) -> &'static str {
"lint_pass"
}
}

impl<'tcx> rustc_lint::LateLintPass<'tcx> for LintPass {
fn check_crate(&mut self, cx: &rustc_lint::LateContext<'tcx>) {
cx.sess().dcx().warn("something bad happened");
}
}

fn main() {}
10 changes: 10 additions & 0 deletions examples/restriction/register_lints_warn/ui/main.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
warning: call to `rustc_errors::DiagCtxtHandle::warn` from within a `register_lints` function
--> $DIR/main.rs:7:5
|
LL | sess.dcx().warn("something bad happened");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(register_lints_warn)]` on by default

warning: 1 warning emitted