Skip to content

add new lint: [missing_panic_message] - #17682

Open
qdot3 wants to merge 1 commit into
rust-lang:masterfrom
qdot3:impl_missing_panic_message
Open

add new lint: [missing_panic_message]#17682
qdot3 wants to merge 1 commit into
rust-lang:masterfrom
qdot3:impl_missing_panic_message

Conversation

@qdot3

@qdot3 qdot3 commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Resolves #14843

changelog: new lint: [`missing_panic_message`]
LLM disclosure: I used LLM to refine my English
  • In clippy_lints/src/missing_panic_message.rs: check -> assess
/// ### Known problems
/// This lint cannot assess the quality of the custom messages.
  • In tests/ui/missing_panic_message.rs: originally "The two lints do not conflict but are overlapped. Considering lint coverage, it is better that both of them catch this pattern"
#[expect(
    clippy::unit_arg,
    reason = "These two lints overlap without conflicting. Keeping both active provides better overall coverage."
)]

@rustbot rustbot added the S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. label Sep 4, 2026
@rustbot

rustbot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome!

You should hear from one of our reviewers after this PR gets at least 2 reviews from the community.

Please see the contribution instructions for more information.

@rustbot rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 4, 2026
@qdot3 qdot3 changed the title changelog: new lint: [missing_panic_message] add new lint: [missing_panic_message] Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Lintcheck changes for f0fdf9b

Lint Added Removed Changed
clippy::missing_panic_message 464 0 0

This comment will be updated if you push new changes

@qdot3
qdot3 force-pushed the impl_missing_panic_message branch from 561e190 to b8a6fb7 Compare September 4, 2026 03:35
CommanderStorm
CommanderStorm approved these changes Sep 4, 2026
View reviewed changes

@CommanderStorm CommanderStorm left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Community review: Love the lint. Great work code-quality wise!
I have one question, but that is likely something for the FCP to answer anyhow.

View changes since this review

Comment thread clippy_lints/src/missing_panic_message.rs Outdated
Comment on lines +57 to +72
// `punic_any(_)` passed a unit value as an argument
if !expr.span.from_expansion()
&& let ExprKind::Call(name, [arg]) = expr.kind
&& name.basic_res().is_diag_item(cx, sym::panic_any)
&& cx.typeck_results().expr_ty(arg).is_unit()
{
span_lint_and_help(
cx,
MISSING_PANIC_MESSAGE,
expr.span,
"using `panic_any` with a unit value as an argument",
None,
"consider sending a meaningful message",
);
}
}

@CommanderStorm CommanderStorm Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 should this be a separate lint?

I would find punic_any(()) a lot more suspicious than plain unimplemented!().. and therefore this part of the lint being in suspicious instead of restriction would make a lot of sense to me.
Maybe my intuition is wrong though.

@qdot3 qdot3 Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The four macros have default messages, but panic_any() doesn't. Using panic_any(()) might be wrong, so suspicious category is more suitable as you said.

@CommanderStorm CommanderStorm Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A split into two lints would be very handy in that case 😉

@qdot3 qdot3 Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I create new file?

Gri-ffin
Gri-ffin suggested changes Sep 4, 2026
View reviewed changes

@Gri-ffin Gri-ffin left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add tests for qualified macro calls.

View changes since this review

Comment thread clippy_lints/src/missing_panic_message.rs Outdated
Comment thread clippy_lints/src/missing_panic_message.rs Outdated
Comment thread clippy_lints/src/missing_panic_message.rs
notriddle
notriddle approved these changes Sep 4, 2026
View reviewed changes

@notriddle notriddle left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Community review: LGMT

View changes since this review

@rustbot rustbot removed the S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. label Sep 4, 2026
@rustbot

rustbot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

r? @llogiq

rustbot has assigned @llogiq for the project review.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 9 candidates
  • 9 candidates expanded to 9 candidates
  • Random selection from 6 candidates

Gri-ffin
Gri-ffin approved these changes Sep 4, 2026
View reviewed changes

@Gri-ffin Gri-ffin left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank You.

View changes since this review

@CommanderStorm

CommanderStorm commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

(make sure to squash your changes so that this does not hold up the PR 😉 )

@qdot3
qdot3 force-pushed the impl_missing_panic_message branch from d3e9d7f to f0fdf9b Compare September 4, 2026 15:13
@rustbot

rustbot commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly #13598) made this pull request unmergeable. Please resolve the merge conflicts.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
@CommanderStorm CommanderStorm CommanderStorm approved these changes
@notriddle notriddle notriddle approved these changes
@Gri-ffin Gri-ffin Gri-ffin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@llogiq llogiq

Labels

needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Lint idea: missing_panic_message

6 participants

@qdot3 @rustbot @CommanderStorm @notriddle @Gri-ffin @llogiq