Skip to content

Rewrite EndianBytes lint pass#17363

Open
Jarcho wants to merge 2 commits into
rust-lang:masterfrom
Jarcho:endian_bytes
Open

Rewrite EndianBytes lint pass#17363
Jarcho wants to merge 2 commits into
rust-lang:masterfrom
Jarcho:endian_bytes

Conversation

@Jarcho

@Jarcho Jarcho commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Changes:

  • Check the method name before type checking.
  • Don't use get_def_path for format! when linting.
  • Check that the from_*_bytes method used is defined on one of the primitives.
  • Add suggestions for the other endian conversions if applicable.
  • Lint in the context of the method name.
  • Lint all uses of the methods, not just calls.
  • Split the tests into one file per lint.

The old test file broke ui_test so something had to be done. It was also hard to actually check if it was correct with all the repeated macro expansions.

changelog: [big_endian_bytes], [host_endian_bytes], [little_endian_bytes]: Lint any time the methods are named instead of just when called.
changelog: [big_endian_bytes], [host_endian_bytes], [little_endian_bytes]: Lint when to_*_bytes is called via UFCS.
changelog: [big_endian_bytes], [host_endian_bytes], [little_endian_bytes]: Always lint when the method name comes from the current crate.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 6, 2026
@rustbot

rustbot commented Jul 6, 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 llogiq, samueltardieu

@Jarcho Jarcho added the G-performance-project Goal: For issues and PRs related to the Clippy Performance Project label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Lintcheck changes for 445d8f6

Lint Added Removed Changed
clippy::big_endian_bytes 60 19 113
clippy::host_endian_bytes 23 15 149
clippy::little_endian_bytes 43 19 73

This comment will be updated if you push new changes

* Check the method name before type checking.
* Don't use `get_def_path` for `format!` when linting.
* Check that the `from_*_bytes` method used is defined on one of the primitives.
* Add suggestions for the other endian conversions if applicable.
* Lint in the context of the method name.
* Lint all uses of the methods, not just calls.
@Jarcho

Jarcho commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

The lintcheck results are a pain to read. The added emissions can be split in three cases:

  • The method being used as an argument (e.g. foo(u32::from_ne_bytes)).
  • UFCS form of ty::to_*_bytes which used to not lint.
  • The method name being used as it's own macro argument. This used to lint in the macro, but we now lint at the call site.

The removals all come from the macro location change.

@bushrat011899 bushrat011899 left a comment

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.

Implementation looks good, really clean and makes a lot of sense to me. Just a note around diagnostic items, and a suggestion to keep a handful of cross-lint tests that this PR removed.

View changes since this review

Comment on lines +102 to +103
if let Some(ty) = cx.ty_based_def(e.hir_id).opt_parent(cx).opt_impl_ty(cx)
&& let ty::Uint(_) | ty::Int(_) | ty::Float(_) = *ty.instantiate_identity().skip_normalization().kind()

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.

Was going to suggest using a diagnostic item to be certain these are applicable methods, but there's probably too many methods to justify the added certainty (6 methods, 6 unsigned integers, 6 signed integers, 2+ floats, so at least 84 diagnostic items...)

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.

Any type dependent name lookup prefers resolving to inherent items over trait items. Since we know the implemented type has methods with the names we're matching on we don't have to actually confirm anything here.

if !only_one {
help_str.push_str("either of ");
span_lint_and_then(cx, lint, sp, msg, |diag| {
if !ptr::addr_eq(lint, HOST_ENDIAN_BYTES) && is_lint_allowed(cx, HOST_ENDIAN_BYTES, e.hir_id) {

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.

I'm assuming the is_lint_allowed(...) check prevents suggesting an alternative that's also being linted away? If so, might be worth keeping the tests that confirmed that behaviour?

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.

Resolved below.

Comment thread tests/ui/endian_bytes.rs
Comment on lines -180 to -214
#[rustfmt::skip]
#[warn(clippy::host_endian_bytes)]
#[warn(clippy::big_endian_bytes)]
fn host_encourage_little() { fn_body_smol!(); }

#[rustfmt::skip]
#[warn(clippy::host_endian_bytes)]
#[warn(clippy::little_endian_bytes)]
fn host_encourage_big() { fn_body_smol!(); }

#[rustfmt::skip]
#[warn(clippy::host_endian_bytes)]
#[warn(clippy::little_endian_bytes)]
#[warn(clippy::big_endian_bytes)]
fn no_help() { fn_body_smol!(); }

#[rustfmt::skip]
#[warn(clippy::little_endian_bytes)]
#[warn(clippy::big_endian_bytes)]
fn little_encourage_host() { fn_body_smol!(); }

#[rustfmt::skip]
#[warn(clippy::host_endian_bytes)]
#[warn(clippy::little_endian_bytes)]
fn little_encourage_big() { fn_body_smol!(); }

#[rustfmt::skip]
#[warn(clippy::big_endian_bytes)]
#[warn(clippy::little_endian_bytes)]
fn big_encourage_host() { fn_body_smol!(); }

#[rustfmt::skip]
#[warn(clippy::host_endian_bytes)]
#[warn(clippy::big_endian_bytes)]
fn big_encourage_little() { fn_body_smol!(); }

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.

These are the tests I think are worth keeping as mentioned above.

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.

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.

Whoops, that's my bad then!

@rustbot

rustbot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly #17389) 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

G-performance-project Goal: For issues and PRs related to the Clippy Performance Project 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.

4 participants