Skip to content

Add lints for disallowing usage of to_xx_bytes and from_xx_bytes #10826

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.

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

Merged
merged 9 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4641,6 +4641,7 @@ Released 2018-09-13
[`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
[`await_holding_refcell_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_refcell_ref
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
[`big_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#big_endian_bytes
[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
[`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
[`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints
Expand Down Expand Up @@ -4811,6 +4812,7 @@ Released 2018-09-13
[`get_first`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_first
[`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
[`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap
[`host_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#host_endian_bytes
[`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion
[`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op
[`if_let_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_mutex
Expand Down Expand Up @@ -4892,6 +4894,7 @@ Released 2018-09-13
[`let_with_type_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_with_type_underscore
[`lines_filter_map_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok
[`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist
[`little_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#little_endian_bytes
[`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
[`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::empty_drop::EMPTY_DROP_INFO,
crate::empty_enum::EMPTY_ENUM_INFO,
crate::empty_structs_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS_INFO,
crate::endian_bytes::BIG_ENDIAN_BYTES_INFO,
crate::endian_bytes::HOST_ENDIAN_BYTES_INFO,
crate::endian_bytes::LITTLE_ENDIAN_BYTES_INFO,
crate::entry::MAP_ENTRY_INFO,
crate::enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT_INFO,
crate::enum_variants::ENUM_VARIANT_NAMES_INFO,
Expand Down
216 changes: 216 additions & 0 deletions clippy_lints/src/endian_bytes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
use crate::Lint;
use clippy_utils::{diagnostics::span_lint_and_then, is_lint_allowed};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::{lint::in_external_macro, ty::Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Symbol;
use std::borrow::Cow;

declare_clippy_lint! {
/// ### What it does
/// Checks for the usage of the `to_ne_bytes` method and/or the function `from_ne_bytes`.
///
/// ### Why is this bad?
/// It's not, but some may prefer to specify the target endianness explicitly.
///
/// ### Example
/// ```rust,ignore
/// let _x = 2i32.to_ne_bytes();
/// let _y = 2i64.to_ne_bytes();
/// ```
#[clippy::version = "1.71.0"]
pub HOST_ENDIAN_BYTES,
restriction,
"disallows usage of the `to_ne_bytes` method"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for the usage of the `to_le_bytes` method and/or the function `from_le_bytes`.
///
/// ### Why is this bad?
/// It's not, but some may wish to lint usage of this method, either to suggest using the host
/// endianness or big endian.
///
/// ### Example
/// ```rust,ignore
/// let _x = 2i32.to_le_bytes();
/// let _y = 2i64.to_le_bytes();
/// ```
#[clippy::version = "1.71.0"]
pub LITTLE_ENDIAN_BYTES,
restriction,
"disallows usage of the `to_le_bytes` method"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for the usage of the `to_be_bytes` method and/or the function `from_be_bytes`.
///
/// ### Why is this bad?
/// It's not, but some may wish to lint usage of this method, either to suggest using the host
/// endianness or little endian.
///
/// ### Example
/// ```rust,ignore
/// let _x = 2i32.to_be_bytes();
/// let _y = 2i64.to_be_bytes();
/// ```
#[clippy::version = "1.71.0"]
pub BIG_ENDIAN_BYTES,
restriction,
"disallows usage of the `to_be_bytes` method"
}

declare_lint_pass!(EndianBytes => [HOST_ENDIAN_BYTES, LITTLE_ENDIAN_BYTES, BIG_ENDIAN_BYTES]);

const HOST_NAMES: [&str; 2] = ["from_ne_bytes", "to_ne_bytes"];
const LITTLE_NAMES: [&str; 2] = ["from_le_bytes", "to_le_bytes"];
const BIG_NAMES: [&str; 2] = ["from_be_bytes", "to_be_bytes"];

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

#[derive(Clone, Copy, PartialEq)]
enum Prefix {
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) -> &str {
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<'_>) {
if in_external_macro(cx.sess(), expr.span) {
return;
}

if_chain! {
if let ExprKind::MethodCall(method_name, receiver, args, ..) = expr.kind;
if args.is_empty();
let ty = cx.typeck_results().expr_ty(receiver);
if ty.is_primitive_ty();
if maybe_lint_endian_bytes(cx, expr, Prefix::To, method_name.ident.name, ty);
then {
return;
}
}

if_chain! {
if let ExprKind::Call(function, ..) = expr.kind;
if let ExprKind::Path(qpath) = function.kind;
if let Some(def_id) = cx.qpath_res(&qpath, function.hir_id).opt_def_id();
if let Some(function_name) = cx.get_def_path(def_id).last();
let ty = cx.typeck_results().expr_ty(expr);
if ty.is_primitive_ty();
then {
maybe_lint_endian_bytes(cx, expr, Prefix::From, *function_name, ty);
}
}
}
}

fn maybe_lint_endian_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, prefix: Prefix, name: Symbol, ty: Ty<'_>) -> bool {
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.as_str() {
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 false,
};

let mut help = None;

'build_help: {
// all lints disallowed, don't give help here
if [&[lint], other_lints.as_slice()]
.concat()
.iter()
.all(|lint| !lint.allowed(cx, expr))
{
break 'build_help;
}

// ne_bytes and all other lints allowed
if lint.as_name(prefix) == ne && other_lints.iter().all(|lint| lint.allowed(cx, expr)) {
help = Some(Cow::Borrowed("specify the desired endianness explicitly"));
break 'build_help;
}

// 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) {
help = Some(Cow::Borrowed("use the native endianness instead"));
break 'build_help;
}

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 ");
}

help_str.push_str(&format!("`{ty}::{}` ", lint.as_name(prefix)));

if i != len && !only_one {
help_str.push_str("or ");
}
}

help = Some(Cow::Owned(help_str + "instead"));
}

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| {
if let Some(help) = help {
diag.help(help);
}
},
);

true
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ mod else_if_without_else;
mod empty_drop;
mod empty_enum;
mod empty_structs_with_brackets;
mod endian_bytes;
mod entry;
mod enum_clike;
mod enum_variants;
Expand Down Expand Up @@ -996,6 +997,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs));
store.register_early_pass(|| Box::new(needless_else::NeedlessElse));
store.register_late_pass(|_| Box::new(missing_fields_in_debug::MissingFieldsInDebug));
store.register_late_pass(|_| Box::new(endian_bytes::EndianBytes));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
127 changes: 127 additions & 0 deletions tests/ui/endian_bytes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
#![allow(unused)]
#![allow(clippy::diverging_sub_expression)]
#![no_main]

macro_rules! fn_body {
() => {
2u8.to_ne_bytes();
2i8.to_ne_bytes();
2u16.to_ne_bytes();
2i16.to_ne_bytes();
2u32.to_ne_bytes();
2i32.to_ne_bytes();
2u64.to_ne_bytes();
2i64.to_ne_bytes();
2u128.to_ne_bytes();
2i128.to_ne_bytes();
2.0f32.to_ne_bytes();
2.0f64.to_ne_bytes();
2usize.to_ne_bytes();
2isize.to_ne_bytes();
u8::from_ne_bytes(todo!());
i8::from_ne_bytes(todo!());
u16::from_ne_bytes(todo!());
i16::from_ne_bytes(todo!());
u32::from_ne_bytes(todo!());
i32::from_ne_bytes(todo!());
u64::from_ne_bytes(todo!());
i64::from_ne_bytes(todo!());
u128::from_ne_bytes(todo!());
i128::from_ne_bytes(todo!());
usize::from_ne_bytes(todo!());
isize::from_ne_bytes(todo!());
f32::from_ne_bytes(todo!());
f64::from_ne_bytes(todo!());

2u8.to_le_bytes();
2i8.to_le_bytes();
2u16.to_le_bytes();
2i16.to_le_bytes();
2u32.to_le_bytes();
2i32.to_le_bytes();
2u64.to_le_bytes();
2i64.to_le_bytes();
2u128.to_le_bytes();
2i128.to_le_bytes();
2.0f32.to_le_bytes();
2.0f64.to_le_bytes();
2usize.to_le_bytes();
2isize.to_le_bytes();
u8::from_le_bytes(todo!());
i8::from_le_bytes(todo!());
u16::from_le_bytes(todo!());
i16::from_le_bytes(todo!());
u32::from_le_bytes(todo!());
i32::from_le_bytes(todo!());
u64::from_le_bytes(todo!());
i64::from_le_bytes(todo!());
u128::from_le_bytes(todo!());
i128::from_le_bytes(todo!());
usize::from_le_bytes(todo!());
isize::from_le_bytes(todo!());
f32::from_le_bytes(todo!());
f64::from_le_bytes(todo!());
};
}

// bless breaks if I use fn_body too much (oops)
macro_rules! fn_body_smol {
() => {
2u8.to_ne_bytes();
u8::from_ne_bytes(todo!());

2u8.to_le_bytes();
u8::from_le_bytes(todo!());

2u8.to_be_bytes();
u8::from_be_bytes(todo!());
};
}

#[rustfmt::skip]
#[warn(clippy::host_endian_bytes)]
fn host() { fn_body!(); }

#[rustfmt::skip]
#[warn(clippy::little_endian_bytes)]
fn little() { fn_body!(); }

#[rustfmt::skip]
#[warn(clippy::big_endian_bytes)]
fn big() { fn_body!(); }

#[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!(); }
Loading