Skip to content

Rewrite EndianBytes lint pass #17363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jarcho wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Rewrite EndianBytes lint pass #17363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
494e1a1
Split `endian_bytes` test into one file per lint.
Jarcho Jul 6, 2026
445d8f6
Rewrite `EndianBytes` lint pass.
Jarcho Jul 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 58 additions & 124 deletions clippy_lints/src/endian_bytes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::Lint;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
use clippy_utils::{is_lint_allowed, sym};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::ty::Ty;
use core::ptr;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::declare_lint_pass;
use rustc_span::Symbol;
use std::fmt::Write;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -69,133 +69,67 @@ declare_lint_pass!(EndianBytes => [
LITTLE_ENDIAN_BYTES,
]);

const HOST_NAMES: [Symbol; 2] = [sym::from_ne_bytes, sym::to_ne_bytes];
const LITTLE_NAMES: [Symbol; 2] = [sym::from_le_bytes, sym::to_le_bytes];
const BIG_NAMES: [Symbol; 2] = [sym::from_be_bytes, sym::to_be_bytes];

#[derive(Clone, Debug)]
enum LintKind {
Host,
Little,
Big,
}

#[derive(Clone, Copy, PartialEq)]
enum Prefix {
#[derive(Clone, Copy)]
enum Direction {
From,
To,
}

impl LintKind {
fn allowed(&self, cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
is_lint_allowed(cx, self.as_lint(), expr.hir_id)
}

fn as_lint(&self) -> &'static Lint {
match self {
LintKind::Host => HOST_ENDIAN_BYTES,
LintKind::Little => LITTLE_ENDIAN_BYTES,
LintKind::Big => BIG_ENDIAN_BYTES,
}
}

fn as_name(&self, prefix: Prefix) -> Symbol {
let index = usize::from(prefix == Prefix::To);

match self {
LintKind::Host => HOST_NAMES[index],
LintKind::Little => LITTLE_NAMES[index],
LintKind::Big => BIG_NAMES[index],
}
}
}

impl LateLintPass<'_> for EndianBytes {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
let (prefix, name, ty_expr) = match expr.kind {
ExprKind::MethodCall(method_name, receiver, [], ..) => (Prefix::To, method_name.ident.name, receiver),
ExprKind::Call(function, ..)
if let ExprKind::Path(qpath) = function.kind
&& let Some(def_id) = cx.qpath_res(&qpath, function.hir_id).opt_def_id()
&& let Some(function_name) = cx.get_def_path(def_id).last() =>
{
(Prefix::From, *function_name, expr)
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
let (sp, direction, lint, msg) = match e.kind {
// rustfmt wants to break each arm into one line per tuple element which
// really hurts readability.
#[rustfmt::skip]
ExprKind::MethodCall(seg, _, [], _) => match seg.ident.name {
sym::to_ne_bytes => (seg.ident.span, Direction::To, HOST_ENDIAN_BYTES, "use of `to_ne_bytes`"),
sym::to_le_bytes => (seg.ident.span, Direction::To, LITTLE_ENDIAN_BYTES, "use of `to_le_bytes`"),
sym::to_be_bytes => (seg.ident.span, Direction::To, BIG_ENDIAN_BYTES, "use of `to_be_bytes`"),
_ => return,
},
#[rustfmt::skip]
ExprKind::Path(QPath::TypeRelative(_, seg)) => match seg.ident.name {
sym::from_ne_bytes => (seg.ident.span, Direction::From, HOST_ENDIAN_BYTES, "use of `from_ne_bytes`"),
sym::from_le_bytes => (seg.ident.span, Direction::From, LITTLE_ENDIAN_BYTES, "use of `from_le_bytes`"),
sym::from_be_bytes => (seg.ident.span, Direction::From, BIG_ENDIAN_BYTES, "use of `from_be_bytes`"),
sym::to_ne_bytes => (seg.ident.span, Direction::To, HOST_ENDIAN_BYTES, "use of `to_ne_bytes`"),
sym::to_le_bytes => (seg.ident.span, Direction::To, LITTLE_ENDIAN_BYTES, "use of `to_le_bytes`"),
sym::to_be_bytes => (seg.ident.span, Direction::To, BIG_ENDIAN_BYTES, "use of `to_be_bytes`"),
_ => return,
},
_ => return,
};
if !expr.span.in_external_macro(cx.sess().source_map())
&& let ty = cx.typeck_results().expr_ty(ty_expr)
&& ty.is_primitive_ty()
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()
Comment on lines +102 to +103

@bushrat011899 bushrat011899 Jul 7, 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.

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...)

@Jarcho Jarcho Jul 8, 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.

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.

// Only check where the name itself comes from. The point of the lints is to
// catch when the wrong byte order is used so we only care if the current crate
// decided on the byte order. Which crate actually assembled the path/call
// isn't relevant for these lints.
&& !sp.in_external_macro(cx.tcx.sess.source_map())
{
maybe_lint_endian_bytes(cx, expr, prefix, name, ty);
}
}
}

fn maybe_lint_endian_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, prefix: Prefix, name: Symbol, ty: Ty<'_>) {
let ne = LintKind::Host.as_name(prefix);
let le = LintKind::Little.as_name(prefix);
let be = LintKind::Big.as_name(prefix);

let (lint, other_lints) = match name {
name if name == ne => ((&LintKind::Host), [(&LintKind::Little), (&LintKind::Big)]),
name if name == le => ((&LintKind::Little), [(&LintKind::Host), (&LintKind::Big)]),
name if name == be => ((&LintKind::Big), [(&LintKind::Host), (&LintKind::Little)]),
_ => return,
};

span_lint_and_then(
cx,
lint.as_lint(),
expr.span,
format!(
"usage of the {}`{ty}::{}`{}",
if prefix == Prefix::From { "function " } else { "" },
lint.as_name(prefix),
if prefix == Prefix::To { " method" } else { "" },
),
move |diag| {
// all lints disallowed, don't give help here
if [&[lint], other_lints.as_slice()]
.concat()
.iter()
.all(|lint| !lint.allowed(cx, expr))
{
return;
}

// ne_bytes and all other lints allowed
if lint.as_name(prefix) == ne && other_lints.iter().all(|lint| lint.allowed(cx, expr)) {
diag.help("specify the desired endianness explicitly");
return;
}

// le_bytes where ne_bytes allowed but be_bytes is not, or le_bytes where ne_bytes allowed but
// le_bytes is not
if (lint.as_name(prefix) == le || lint.as_name(prefix) == be) && LintKind::Host.allowed(cx, expr) {
diag.help("use the native endianness instead");
return;
}

let allowed_lints = other_lints.iter().filter(|lint| lint.allowed(cx, expr));
let len = allowed_lints.clone().count();

let mut help_str = "use ".to_owned();

for (i, lint) in allowed_lints.enumerate() {
let only_one = len == 1;
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) {

@bushrat011899 bushrat011899 Jul 7, 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.

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?

@bushrat011899 bushrat011899 Jul 8, 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.

Resolved below.

let (msg, sugg) = match direction {
Direction::From => ("convert from native endian", "from_ne_bytes"),
Direction::To => ("convert to native endian", "to_ne_bytes"),
};
diag.span_suggestion(sp, msg, sugg, Applicability::MaybeIncorrect);
}

write!(help_str, "`{ty}::{}` ", lint.as_name(prefix)).unwrap();

if i != len && !only_one {
help_str.push_str("or ");
if !ptr::addr_eq(lint, LITTLE_ENDIAN_BYTES) && is_lint_allowed(cx, LITTLE_ENDIAN_BYTES, e.hir_id) {
let (msg, sugg) = match direction {
Direction::From => ("convert from little endian", "from_le_bytes"),
Direction::To => ("convert to little endian", "to_le_bytes"),
};
diag.span_suggestion(sp, msg, sugg, Applicability::MaybeIncorrect);
}
}
help_str.push_str("instead");
diag.help(help_str);
},
);
if !ptr::addr_eq(lint, BIG_ENDIAN_BYTES) && is_lint_allowed(cx, BIG_ENDIAN_BYTES, e.hir_id) {
let (msg, sugg) = match direction {
Direction::From => ("convert from big endian", "from_be_bytes"),
Direction::To => ("convert to big endian", "to_be_bytes"),
};
diag.span_suggestion(sp, msg, sugg, Applicability::MaybeIncorrect);
}
});
}
}
}
1 change: 0 additions & 1 deletion clippy_lints/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,6 @@ mod zombie_processes;
use clippy_config::{Conf, get_configuration_metadata, sanitize_explanation};
use clippy_utils::macros::FormatArgsStorage;
use rustc_data_structures::fx::FxHashSet;
use rustc_lint::Lint;
use rustc_middle::ty::TyCtxt;
use utils::attr_collector::AttrStorage;

Expand Down
56 changes: 56 additions & 0 deletions clippy_utils/src/res.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ impl<'tcx, T: Copy + MaybeQPath<'tcx>> MaybeQPath<'tcx> for &Option<T> {
/// A resolved path and the explicit `Self` type if there is one.
type OptResPath<'tcx> = (Option<&'tcx hir::Ty<'tcx>>, Option<&'tcx Path<'tcx>>);

type OptTyRelPath<'tcx> = Option<(&'tcx hir::Ty<'tcx>, &'tcx PathSegment<'tcx>)>;

/// A HIR node which might be a `QPath::Resolved`.
///
/// The following are resolved paths:
Expand All @@ -354,6 +356,10 @@ pub trait MaybeResPath<'a>: Copy {
/// type associated with it.
fn opt_res_path(self) -> OptResPath<'a>;

/// If this node is a type relative path gets both the type and the final
/// segments of the path.
fn opt_ty_rel_path(self) -> OptTyRelPath<'a>;

/// If this node is a resolved path gets it's resolution. Returns `Res::Err`
/// otherwise.
#[inline]
Expand Down Expand Up @@ -391,6 +397,11 @@ impl<'a> MaybeResPath<'a> for &'a Path<'a> {
(None, Some(self))
}

#[inline]
fn opt_ty_rel_path(self) -> OptTyRelPath<'a> {
None
}

#[inline]
fn basic_res(self) -> &'a Res {
&self.res
Expand All @@ -404,6 +415,14 @@ impl<'a> MaybeResPath<'a> for &QPath<'a> {
QPath::TypeRelative(..) => (None, None),
}
}

#[inline]
fn opt_ty_rel_path(self) -> OptTyRelPath<'a> {
match *self {
QPath::TypeRelative(ty, seg) => Some((ty, seg)),
QPath::Resolved(..) => None,
}
}
}
impl<'a> MaybeResPath<'a> for &Expr<'a> {
#[inline]
Expand All @@ -413,6 +432,14 @@ impl<'a> MaybeResPath<'a> for &Expr<'a> {
_ => (None, None),
}
}

#[inline]
fn opt_ty_rel_path(self) -> OptTyRelPath<'a> {
match &self.kind {
ExprKind::Path(qpath) => qpath.opt_ty_rel_path(),
_ => None,
}
}
}
impl<'a> MaybeResPath<'a> for &PatExpr<'a> {
#[inline]
Expand All @@ -422,6 +449,14 @@ impl<'a> MaybeResPath<'a> for &PatExpr<'a> {
PatExprKind::Lit { .. } => (None, None),
}
}

#[inline]
fn opt_ty_rel_path(self) -> OptTyRelPath<'a> {
match &self.kind {
PatExprKind::Path(qpath) => qpath.opt_ty_rel_path(),
PatExprKind::Lit { .. } => None,
}
}
}
impl<'a, AmbigArg> MaybeResPath<'a> for &hir::Ty<'a, AmbigArg> {
#[inline]
Expand All @@ -431,6 +466,14 @@ impl<'a, AmbigArg> MaybeResPath<'a> for &hir::Ty<'a, AmbigArg> {
_ => (None, None),
}
}

#[inline]
fn opt_ty_rel_path(self) -> OptTyRelPath<'a> {
match &self.kind {
TyKind::Path(qpath) => qpath.opt_ty_rel_path(),
_ => None,
}
}
}
impl<'a> MaybeResPath<'a> for &Pat<'a> {
#[inline]
Expand All @@ -440,6 +483,14 @@ impl<'a> MaybeResPath<'a> for &Pat<'a> {
_ => (None, None),
}
}

#[inline]
fn opt_ty_rel_path(self) -> OptTyRelPath<'a> {
match self.kind {
PatKind::Expr(e) => e.opt_ty_rel_path(),
_ => None,
}
}
}
impl<'a, T: MaybeResPath<'a>> MaybeResPath<'a> for Option<T> {
#[inline]
Expand All @@ -450,6 +501,11 @@ impl<'a, T: MaybeResPath<'a>> MaybeResPath<'a> for Option<T> {
}
}

#[inline]
fn opt_ty_rel_path(self) -> OptTyRelPath<'a> {
self.and_then(T::opt_ty_rel_path)
}

#[inline]
fn basic_res(self) -> &'a Res {
self.map_or(&Res::Err, T::basic_res)
Expand Down
Loading