Skip to content

Fix duration_suboptimal_units for small literals#16922

Merged
ada4a merged 1 commit into
rust-lang:masterfrom
tanndlin:duration_suboptimal_units
May 15, 2026
Merged

Fix duration_suboptimal_units for small literals#16922
ada4a merged 1 commit into
rust-lang:masterfrom
tanndlin:duration_suboptimal_units

Conversation

@tanndlin

@tanndlin tanndlin commented Apr 27, 2026 •
edited by ada4a
Loading

Copy link
Copy Markdown
Contributor

View all comments

fixes #16532

changelog: [duration_suboptimal_units]: fix false positive for small integer literals whose promoted value would be at most 10.

Combined #16565 and #16596

Copilot AI review requested due to automatic review settings April 27, 2026 22:19
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 27, 2026
@rustbot

rustbot commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

r? @llogiq

rustbot has assigned @llogiq.
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: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, dswij, llogiq

Copilot AI reviewed Apr 27, 2026
View reviewed changes

Copilot AI left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates duration_suboptimal_units to avoid linting on small integer literal values where the promoted (larger-unit) value would be <= 10, addressing the false positive described in #16532.

Changes:

  • Adjusts lint logic to suppress suggestions for small integer literals while still linting expressions.
  • Updates lint documentation/examples to avoid the new suppression edge cases.
  • Expands UI tests (including duration_constructors coverage) to validate the new behavior and expected suggestions.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
clippy_lints/src/duration_suboptimal_units.rs Implements the <= 10 suppression rule for integer literals and updates lint docs/examples.
tests/ui/duration_suboptimal_units.rs Adds UI coverage for small literals vs expressions and updates expected lint sites.
tests/ui/duration_suboptimal_units.fixed Updates fixed-output expectations to match the new lint behavior.
tests/ui/duration_suboptimal_units.stderr Updates stderr expectations for the new/shifted diagnostics.
tests/ui/duration_suboptimal_units_days_weeks.rs Adds non-linting literal + linting expression cases under duration_constructors.
tests/ui/duration_suboptimal_units_days_weeks.fixed Updates fixed-output expectations for the days/weeks test.
tests/ui/duration_suboptimal_units_days_weeks.stderr Updates stderr expectations for the days/weeks test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

View changes since this review

Comment thread tests/ui/duration_suboptimal_units.rs Outdated
Comment thread clippy_lints/src/duration_suboptimal_units.rs Outdated
Comment thread tests/ui/duration_suboptimal_units.rs Outdated
Comment thread tests/ui/duration_suboptimal_units.fixed Outdated
Comment thread tests/ui/duration_suboptimal_units.rs
@tanndlin tanndlin mentioned this pull request Apr 28, 2026
Closed
ada4a
ada4a requested changes May 4, 2026
View reviewed changes

@ada4a ada4a 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.

Thank you for picking this up:)

View changes since this review

Comment thread clippy_lints/src/duration_suboptimal_units.rs
Comment thread clippy_lints/src/duration_suboptimal_units.rs Outdated
Comment thread tests/ui/duration_suboptimal_units.rs Outdated
Comment thread tests/ui/duration_suboptimal_units.rs Outdated
Comment thread tests/ui/duration_suboptimal_units.rs Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 4, 2026
@rustbot

rustbot commented May 4, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@tanndlin

tanndlin commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 4, 2026
@ada4a

ada4a commented May 6, 2026

Copy link
Copy Markdown
Contributor

Could you please squash down the commits? I think it would also be nice to add @Hectonight and @veeceey as co-authors to the commit

@tanndlin tanndlin force-pushed the duration_suboptimal_units branch from 3c536bd to a1ffdf3 Compare May 6, 2026 21:33
@rustbot

rustbot commented May 6, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@tanndlin

tanndlin commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Could you please squash down the commits? I think it would also be nice to add @Hectonight and @veeceey as co-authors to the commit

Absolutely! It's squashed with all 3 of us on the commit now

ada4a
ada4a requested changes May 6, 2026
View reviewed changes

@ada4a ada4a 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.

Sorry, one last thing^^ Feel free to force-push

View changes since this review

Comment thread tests/ui/duration_suboptimal_units_days_weeks.rs
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 6, 2026
@tanndlin tanndlin force-pushed the duration_suboptimal_units branch 3 times, most recently from 7112278 to 0f1bb4e Compare May 11, 2026 21:37
ada4a
ada4a requested changes May 12, 2026
View reviewed changes
Comment thread tests/ui/duration_suboptimal_units.fixed Outdated
@@ -1,13 +1,14 @@
//@aux-build:proc_macros.rs
#![warn(clippy::duration_suboptimal_units)]
#![feature(duration_constructors)]

@ada4a ada4a May 12, 2026 •
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.

Ideally this file wouldn't contain any tests that require this feature. Could you please move all of those into the duration_suboptimal_units_days_weeks.rs instead?

View changes since the review

@ada4a ada4a mentioned this pull request May 12, 2026
Open
@tanndlin tanndlin force-pushed the duration_suboptimal_units branch from 0f1bb4e to 1668b6c Compare May 12, 2026 21:29
@ada4a

ada4a commented May 13, 2026

Copy link
Copy Markdown
Contributor

r? @ada4a

Changed the changelog entry to fit the format a bit better. For posterity, here's how it used to look:

changelog: Fix [duration_suboptimal_units] false positive for small integer literals whose promoted value would be at most 10.

@rustbot rustbot assigned ada4a and unassigned llogiq May 13, 2026
ada4a
ada4a requested changes May 13, 2026
View reviewed changes
Comment thread tests/ui/duration_suboptimal_units.rs Outdated
@tanndlin tanndlin force-pushed the duration_suboptimal_units branch from 1668b6c to 2a7ac1f Compare May 13, 2026 23:19
@ada4a

ada4a commented May 15, 2026

Copy link
Copy Markdown
Contributor

Could you please bless the tests? I'd merge this right after^^

@tanndlin @veeceey
No longer promotes for integer literals <= 10
3872c05 
Co-authored-by: Hectonight <joshuaoring@yahoo.com>

Co-authored-by: veeceey <varun_6april@hotmail.com>
@tanndlin tanndlin force-pushed the duration_suboptimal_units branch from 2a7ac1f to 3872c05 Compare May 15, 2026 13:37
@tanndlin

tanndlin commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

Could you please bless the tests? I'd merge this right after^^

Sorry! I thought I did. Its, updated

ada4a
ada4a approved these changes May 15, 2026
View reviewed changes

@ada4a ada4a 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.

Thank you:)

View changes since this review

@ada4a ada4a added this pull request to the merge queue May 15, 2026
Merged via the queue into rust-lang:master with commit b734854 May 15, 2026
11 checks passed
@tanndlin tanndlin deleted the duration_suboptimal_units branch May 15, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@ada4a ada4a ada4a approved these changes

Assignees

@ada4a ada4a

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

duration_suboptimal_units should not trigger on small values

5 participants

@tanndlin @rustbot @ada4a @llogiq