Skip to content

feat: add manual_max and manual_min lints - #17206

Open
sylvestre wants to merge 1 commit into
rust-lang:masterfrom
sylvestre:manual-max-min-lint
Open

feat: add manual_max and manual_min lints#17206
sylvestre wants to merge 1 commit into
rust-lang:masterfrom
sylvestre:manual-max-min-lint

Conversation

@sylvestre

@sylvestre sylvestre commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Add two complexity lints that detect a single-sided clamp written as an if/else (or a guarding if) and suggest Ord::max / Ord::min:

let _ = if a < b { b } else { a };  // -> a.max(b)
if cores < b { cores = b; }         // -> cores = cores.max(b);

This is the sound, generalizable form of the manual clamp simplified by hand in uutils/coreutils#12753 (nproc). Unlike manual_clamp, which only fires when both a lower and an upper bound are applied, these catch the common single-bound floor/ceiling case.

Restricted to Ord types so float NaN semantics are not changed, and emitted as MaybeIncorrect since the branching form re-evaluates the selected operand.

changelog: add manual_max and manual_min lints

@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 Jun 10, 2026
@rustbot

rustbot commented Jun 10, 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: 8 candidates
  • 8 candidates expanded to 8 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@rustbot

This comment has been minimized.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown

Lintcheck changes for 4992d2c

Lint Added Removed Changed
clippy::manual_max 17 0 0
clippy::manual_min 8 0 0

This comment will be updated if you push new changes

@sylvestre
sylvestre force-pushed the manual-max-min-lint branch from 72b7356 to a39a158 Compare June 10, 2026 20:08
@rustbot

This comment has been minimized.

@oech3

oech3 commented Jun 11, 2026

Copy link
Copy Markdown

nproc did not compare cores - limit with 1. So this is slightly different with this lint.

@rustbot

This comment has been minimized.

@samueltardieu

Copy link
Copy Markdown
Member

You'll have to guard this with a MSRV check (1.21).

@sylvestre
sylvestre force-pushed the manual-max-min-lint branch from a39a158 to 38ea87c Compare June 13, 2026 07:38
@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@sylvestre
sylvestre force-pushed the manual-max-min-lint branch from 38ea87c to 1dd053a Compare June 13, 2026 08:12
@rustbot

This comment has been minimized.

@sylvestre

Copy link
Copy Markdown
Contributor Author

@samueltardieu done, merci!

@sylvestre

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@sylvestre

sylvestre commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Lintcheck - 4 hits on default set (27 crates), 51 hits (22 min + 29 max) on extended set (499 crates), no ICEs.

@sylvestre
sylvestre force-pushed the manual-max-min-lint branch from 1dd053a to 33cdb19 Compare July 6, 2026 22:57
@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@sylvestre
sylvestre force-pushed the manual-max-min-lint branch from 33cdb19 to ee53007 Compare July 31, 2026 13:12
Add two `complexity` lints that detect a single-sided clamp written as an
`if`/`else` (or a guarding `if`) and suggest `Ord::max` / `Ord::min`:

    let _ = if a < b { b } else { a };  // -> a.max(b)
    if cores < b { cores = b; }         // -> cores = cores.max(b);

This is the sound, generalizable form of a manual clamp that is often
simplified by hand in real code (for example in the `nproc` utility of
uutils/coreutils). Unlike `manual_clamp`, which only fires when both a
lower and an upper bound are applied, these catch the common
single-bound floor/ceiling case.

Restricted to `Ord` types so float `NaN` semantics are not changed, and
emitted as `MaybeIncorrect` since the branching form re-evaluates the
selected operand.
@sylvestre
sylvestre force-pushed the manual-max-min-lint branch from ee53007 to 4992d2c Compare August 3, 2026 21:14
@rustbot

rustbot commented Aug 3, 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.

@samueltardieu samueltardieu added the lint-nominated Create an FCP-thread on Zulip for this PR label Aug 4, 2026
@rustbot

rustbot commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

This lint has been nominated for inclusion.

A FCP topic has been created on Zulip.

@rustbot

rustbot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

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

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

Labels

lint-nominated Create an FCP-thread on Zulip for this PR 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

Development

Successfully merging this pull request may close these issues.

5 participants