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 1dd053a

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

rustbot commented Jun 13, 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.

@rustbot

This comment has been minimized.

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.
@sylvestre sylvestre force-pushed the manual-max-min-lint branch from 38ea87c to 1dd053a Compare June 13, 2026 08:12
@rustbot

rustbot commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@sylvestre

Copy link
Copy Markdown
Contributor Author

@samueltardieu done, merci!

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

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

Development

Successfully merging this pull request may close these issues.

5 participants