From 4da677fdc9e255dea19b6823b09113167a526400 Mon Sep 17 00:00:00 2001 From: Nicolas Sarlin Date: Mon, 16 Dec 2024 14:55:17 +0100 Subject: [PATCH] chore(lint): use dylint as lint driver for tfhe-lint --- .linelint.yml | 4 + Cargo.toml | 8 +- Makefile | 29 ++++--- tasks/src/check_tfhe_docs_are_tested.rs | 3 +- tfhe/Cargo.toml | 2 +- .../commons/math/random/mod.rs | 2 - .../backward_compatibility/fft_impl/mod.rs | 2 - .../core_crypto/backward_compatibility/mod.rs | 2 + .../fft_impl/fft64/math/fft/mod.rs | 2 +- .../backward_compatibility/booleans.rs | 1 - .../backward_compatibility/integers.rs | 2 - .../backward_compatibility/mod.rs | 2 + tfhe/src/high_level_api/booleans/inner.rs | 2 +- .../high_level_api/integers/signed/inner.rs | 2 +- .../high_level_api/integers/unsigned/inner.rs | 2 +- tfhe/src/high_level_api/keys/server.rs | 2 +- tfhe/src/high_level_api/strings/ascii/mod.rs | 4 +- tfhe/src/safe_serialization.rs | 8 +- tfhe/src/shortint/wopbs/mod.rs | 2 +- utils/cargo-tfhe-lints-inner/Cargo.toml | 10 --- utils/cargo-tfhe-lints-inner/README.md | 16 ---- .../rust-toolchain.toml | 4 - utils/cargo-tfhe-lints-inner/src/main.rs | 59 -------------- utils/cargo-tfhe-lints/Cargo.toml | 8 -- utils/cargo-tfhe-lints/README.md | 25 ------ utils/cargo-tfhe-lints/src/main.rs | 50 ------------ utils/tfhe-lints/.cargo/config.toml | 5 ++ utils/tfhe-lints/.gitignore | 1 + utils/tfhe-lints/Cargo.toml | 25 ++++++ utils/tfhe-lints/README.md | 29 +++++++ utils/tfhe-lints/rust-toolchain | 3 + utils/tfhe-lints/src/lib.rs | 12 +++ .../src/serialize_without_versionize.rs | 81 +++++++++++-------- .../src/utils.rs | 11 +-- utils/tfhe-lints/ui/main.rs | 11 +++ utils/tfhe-lints/ui/main.stderr | 17 ++++ 36 files changed, 199 insertions(+), 249 deletions(-) delete mode 100644 utils/cargo-tfhe-lints-inner/Cargo.toml delete mode 100644 utils/cargo-tfhe-lints-inner/README.md delete mode 100644 utils/cargo-tfhe-lints-inner/rust-toolchain.toml delete mode 100644 utils/cargo-tfhe-lints-inner/src/main.rs delete mode 100644 utils/cargo-tfhe-lints/Cargo.toml delete mode 100644 utils/cargo-tfhe-lints/README.md delete mode 100644 utils/cargo-tfhe-lints/src/main.rs create mode 100644 utils/tfhe-lints/.cargo/config.toml create mode 100644 utils/tfhe-lints/.gitignore create mode 100644 utils/tfhe-lints/Cargo.toml create mode 100644 utils/tfhe-lints/README.md create mode 100644 utils/tfhe-lints/rust-toolchain create mode 100644 utils/tfhe-lints/src/lib.rs rename utils/{cargo-tfhe-lints-inner => tfhe-lints}/src/serialize_without_versionize.rs (59%) rename utils/{cargo-tfhe-lints-inner => tfhe-lints}/src/utils.rs (80%) create mode 100644 utils/tfhe-lints/ui/main.rs create mode 100644 utils/tfhe-lints/ui/main.stderr diff --git a/.linelint.yml b/.linelint.yml index 0cac828924..fbbba0e349 100644 --- a/.linelint.yml +++ b/.linelint.yml @@ -1,11 +1,15 @@ ignore: - .git - target + - tfhe/build + - venv + - web-test-runner - tfhe/benchmarks_parameters - tfhe/web_wasm_parallel_tests/node_modules - tfhe/web_wasm_parallel_tests/dist - keys - coverage + - utils/tfhe-lints/ui/main.stderr rules: # checks if file ends in a newline character diff --git a/Cargo.toml b/Cargo.toml index dd28562866..d905f167aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,8 +15,7 @@ members = [ exclude = [ "tfhe/backward_compatibility_tests", - "utils/cargo-tfhe-lints-inner", - "utils/cargo-tfhe-lints" + "utils/tfhe-lints", ] [workspace.dependencies] aligned-vec = { version = "0.6", default-features = false } @@ -46,3 +45,8 @@ inherits = "dev" opt-level = 3 lto = "off" debug-assertions = false + +[workspace.metadata.dylint] +libraries = [ + { path = "utils/tfhe-lints" } +] diff --git a/Makefile b/Makefile index 7d6a3abb05..7d2c5e54b5 100644 --- a/Makefile +++ b/Makefile @@ -151,10 +151,9 @@ install_tarpaulin: install_rs_build_toolchain cargo $(CARGO_RS_BUILD_TOOLCHAIN) install cargo-tarpaulin --locked || \ ( echo "Unable to install cargo tarpaulin, unknown error." && exit 1 ) -.PHONY: install_tfhe_lints # Install custom tfhe-rs lints -install_tfhe_lints: - (cd utils/cargo-tfhe-lints-inner && cargo install --path .) && \ - cd utils/cargo-tfhe-lints && cargo install --path . +.PHONY: install_cargo_dylint # Install custom tfhe-rs lints +install_cargo_dylint: + cargo install cargo-dylint dylint-link .PHONY: install_typos_checker # Install typos checker install_typos_checker: install_rs_build_toolchain @@ -416,6 +415,11 @@ clippy_versionable: install_rs_check_toolchain RUSTFLAGS="$(RUSTFLAGS)" cargo "$(CARGO_RS_CHECK_TOOLCHAIN)" clippy --all-targets \ -p tfhe-versionable -- --no-deps -D warnings +.PHONY: clippy_tfhe_lints # Run clippy lints on tfhe-lints +clippy_tfhe_lints: # the toolchain is selected with toolchain.toml + cd utils/tfhe-lints && \ + cargo clippy --all-targets -- --no-deps -D warnings + .PHONY: clippy_all # Run all clippy targets clippy_all: clippy_rustdoc clippy clippy_boolean clippy_shortint clippy_integer clippy_all_targets \ clippy_c_api clippy_js_wasm_api clippy_tasks clippy_core clippy_tfhe_csprng clippy_zk_pok clippy_trivium \ @@ -439,9 +443,9 @@ check_rust_bindings_did_not_change: .PHONY: tfhe_lints # Run custom tfhe-rs lints -tfhe_lints: install_tfhe_lints - cd tfhe && RUSTFLAGS="$(RUSTFLAGS)" cargo tfhe-lints \ - --features=boolean,shortint,integer,zk-pok -- -D warnings +tfhe_lints: install_cargo_dylint + RUSTFLAGS="$(RUSTFLAGS)" cargo dylint --all -p tfhe --no-deps -- \ + --features=boolean,shortint,integer,zk-pok .PHONY: build_core # Build core_crypto without experimental features build_core: install_rs_build_toolchain install_rs_check_toolchain @@ -887,6 +891,11 @@ test_versionable: install_rs_build_toolchain RUSTFLAGS="$(RUSTFLAGS)" cargo $(CARGO_RS_BUILD_TOOLCHAIN) test --profile $(CARGO_PROFILE) \ --all-targets -p tfhe-versionable +.PHONY: test_tfhe_lints # Run test on tfhe-lints +test_tfhe_lints: install_cargo_dylint + cd utils/tfhe-lints && \ + cargo test + # The backward compat data repo holds historical binary data but also rust code to generate and load them. # Here we use the "patch" functionality of Cargo to make sure the repo used for the data is the same as the one used for the code. .PHONY: test_backward_compatibility_ci @@ -1303,9 +1312,7 @@ sha256_bool: install_rs_check_toolchain .PHONY: pcc # pcc stands for pre commit checks (except GPU) pcc: no_tfhe_typo no_dbg_log check_fmt check_typos lint_doc check_md_docs_are_tested check_intra_md_links \ -clippy_all check_compile_tests -# TFHE lints deactivated as it's incompatible with 1.83 - temporary -# tfhe_lints +clippy_all check_compile_tests test_tfhe_lints tfhe_lints .PHONY: pcc_gpu # pcc stands for pre commit checks for GPU compilation pcc_gpu: clippy_gpu clippy_cuda_backend check_compile_tests_benches_gpu check_rust_bindings_did_not_change @@ -1315,7 +1322,7 @@ fpcc: no_tfhe_typo no_dbg_log check_fmt check_typos lint_doc check_md_docs_are_t check_compile_tests .PHONY: conformance # Automatically fix problems that can be fixed -conformance: fmt fmt_js +conformance: fix_newline fmt fmt_js #=============================== FFT Section ================================== .PHONY: doc_fft # Build rust doc for tfhe-fft diff --git a/tasks/src/check_tfhe_docs_are_tested.rs b/tasks/src/check_tfhe_docs_are_tested.rs index 142add6cb0..93a2d5d5d6 100644 --- a/tasks/src/check_tfhe_docs_are_tested.rs +++ b/tasks/src/check_tfhe_docs_are_tested.rs @@ -10,7 +10,7 @@ const DIR_TO_IGNORE: [&str; 3] = [ "tfhe/tfhe-backward-compat-data", ]; -const FILES_TO_IGNORE: [&str; 5] = [ +const FILES_TO_IGNORE: [&str; 6] = [ // This contains fragments of code that are unrelated to TFHE-rs "tfhe/docs/tutorials/sha256_bool.md", // TODO: This contains code that could be executed as a trivium docstring @@ -21,6 +21,7 @@ const FILES_TO_IGNORE: [&str; 5] = [ "tfhe-fft/README.md", // TODO: find a way to test the tfhe-ntt readme "tfhe-ntt/README.md", + "utils/tfhe-lints/README.md", ]; pub fn check_tfhe_docs_are_tested() -> Result<(), Error> { diff --git a/tfhe/Cargo.toml b/tfhe/Cargo.toml index 08ee93b587..ce5d48127d 100644 --- a/tfhe/Cargo.toml +++ b/tfhe/Cargo.toml @@ -321,7 +321,7 @@ crate-type = ["lib", "staticlib", "cdylib"] [lints.rust] unexpected_cfgs = { level = "warn", check-cfg = [ 'cfg(tarpaulin)', - 'cfg(tfhe_lints)', + 'cfg(dylint_lib, values(any()))', # This is a bug/unwanted behavior from wasm_bindgen macro, for now warn instead of erroring 'cfg(wasm_bindgen_unstable_test_coverage)', ] } diff --git a/tfhe/src/core_crypto/backward_compatibility/commons/math/random/mod.rs b/tfhe/src/core_crypto/backward_compatibility/commons/math/random/mod.rs index 162ff5fc8e..37a13ebd59 100644 --- a/tfhe/src/core_crypto/backward_compatibility/commons/math/random/mod.rs +++ b/tfhe/src/core_crypto/backward_compatibility/commons/math/random/mod.rs @@ -19,7 +19,6 @@ pub enum DynamicDistributionVersions { } #[derive(Serialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] pub enum CompressionSeedVersioned<'vers> { V0(&'vers CompressionSeed), } @@ -31,7 +30,6 @@ impl<'vers> From<&'vers CompressionSeed> for CompressionSeedVersioned<'vers> { } #[derive(Serialize, Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] pub enum CompressionSeedVersionedOwned { V0(CompressionSeed), } diff --git a/tfhe/src/core_crypto/backward_compatibility/fft_impl/mod.rs b/tfhe/src/core_crypto/backward_compatibility/fft_impl/mod.rs index 455d1add3a..9789e9d430 100644 --- a/tfhe/src/core_crypto/backward_compatibility/fft_impl/mod.rs +++ b/tfhe/src/core_crypto/backward_compatibility/fft_impl/mod.rs @@ -12,7 +12,6 @@ use crate::core_crypto::prelude::{ }; #[derive(Serialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] pub enum FourierPolynomialListVersioned<'vers> { V0(FourierPolynomialList<&'vers [c64]>), } @@ -31,7 +30,6 @@ impl<'vers, C: Container> From<&'vers FourierPolynomialList> // Here we do not derive "VersionsDispatch" so that we can implement a non recursive Versionize #[derive(Serialize, Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] pub enum FourierPolynomialListVersionedOwned { V0(FourierPolynomialList>), } diff --git a/tfhe/src/core_crypto/backward_compatibility/mod.rs b/tfhe/src/core_crypto/backward_compatibility/mod.rs index 660e9023a3..662e44f2ee 100644 --- a/tfhe/src/core_crypto/backward_compatibility/mod.rs +++ b/tfhe/src/core_crypto/backward_compatibility/mod.rs @@ -1,4 +1,6 @@ #![allow(clippy::large_enum_variant)] +// Backward compatibility types should not be themselves versioned +#![cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] pub mod commons; pub mod entities; diff --git a/tfhe/src/core_crypto/fft_impl/fft64/math/fft/mod.rs b/tfhe/src/core_crypto/fft_impl/fft64/math/fft/mod.rs index 666fef7142..79d1446ca1 100644 --- a/tfhe/src/core_crypto/fft_impl/fft64/math/fft/mod.rs +++ b/tfhe/src/core_crypto/fft_impl/fft64/math/fft/mod.rs @@ -623,12 +623,12 @@ impl> serde::Serialize for FourierPolynomialList ) -> Result { use crate::core_crypto::commons::traits::Split; - #[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] pub struct SingleFourierPolynomial<'a> { fft: FftView<'a>, buf: &'a [c64], } + #[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] impl serde::Serialize for SingleFourierPolynomial<'_> { fn serialize( &self, diff --git a/tfhe/src/high_level_api/backward_compatibility/booleans.rs b/tfhe/src/high_level_api/backward_compatibility/booleans.rs index f4a8370082..5b3ae4bb6d 100644 --- a/tfhe/src/high_level_api/backward_compatibility/booleans.rs +++ b/tfhe/src/high_level_api/backward_compatibility/booleans.rs @@ -9,7 +9,6 @@ use std::convert::Infallible; // Manual impl #[derive(Serialize, Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] pub(crate) enum InnerBooleanVersionedOwned { V0(InnerBooleanVersionOwned), } diff --git a/tfhe/src/high_level_api/backward_compatibility/integers.rs b/tfhe/src/high_level_api/backward_compatibility/integers.rs index 33302968a5..90a11927f2 100644 --- a/tfhe/src/high_level_api/backward_compatibility/integers.rs +++ b/tfhe/src/high_level_api/backward_compatibility/integers.rs @@ -24,13 +24,11 @@ use self::unsigned::RadixCiphertext as UnsignedRadixCiphertext; // Manual impl #[derive(Serialize, Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] pub(crate) enum SignedRadixCiphertextVersionedOwned { V0(SignedRadixCiphertextVersionOwned), } #[derive(Serialize, Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] pub(crate) enum UnsignedRadixCiphertextVersionedOwned { V0(UnsignedRadixCiphertextVersionOwned), } diff --git a/tfhe/src/high_level_api/backward_compatibility/mod.rs b/tfhe/src/high_level_api/backward_compatibility/mod.rs index e12411d526..020ad94bf1 100644 --- a/tfhe/src/high_level_api/backward_compatibility/mod.rs +++ b/tfhe/src/high_level_api/backward_compatibility/mod.rs @@ -1,4 +1,6 @@ #![allow(clippy::large_enum_variant)] +// Backward compatibility types should not be themselves versioned +#![cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] pub mod booleans; pub mod compact_list; diff --git a/tfhe/src/high_level_api/booleans/inner.rs b/tfhe/src/high_level_api/booleans/inner.rs index 669675edaf..13011091c1 100644 --- a/tfhe/src/high_level_api/booleans/inner.rs +++ b/tfhe/src/high_level_api/booleans/inner.rs @@ -52,7 +52,7 @@ impl<'de> serde::Deserialize<'de> for InnerBoolean { // Only CPU data are serialized so we only versionize the CPU type. #[derive(serde::Serialize, serde::Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] +#[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] pub(crate) struct InnerBooleanVersionOwned( ::VersionedOwned, ); diff --git a/tfhe/src/high_level_api/integers/signed/inner.rs b/tfhe/src/high_level_api/integers/signed/inner.rs index dfcc9e1564..cd72210980 100644 --- a/tfhe/src/high_level_api/integers/signed/inner.rs +++ b/tfhe/src/high_level_api/integers/signed/inner.rs @@ -67,7 +67,7 @@ impl<'de> serde::Deserialize<'de> for RadixCiphertext { // Only CPU data are serialized so we only versionize the CPU type. #[derive(serde::Serialize, serde::Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] +#[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] pub(crate) struct RadixCiphertextVersionOwned( ::VersionedOwned, ); diff --git a/tfhe/src/high_level_api/integers/unsigned/inner.rs b/tfhe/src/high_level_api/integers/unsigned/inner.rs index 8ef5e28880..07573530b9 100644 --- a/tfhe/src/high_level_api/integers/unsigned/inner.rs +++ b/tfhe/src/high_level_api/integers/unsigned/inner.rs @@ -63,7 +63,7 @@ impl<'de> serde::Deserialize<'de> for RadixCiphertext { // Only CPU data are serialized so we only version the CPU type. #[derive(serde::Serialize, serde::Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] +#[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] pub(crate) struct RadixCiphertextVersionOwned( ::VersionedOwned, ); diff --git a/tfhe/src/high_level_api/keys/server.rs b/tfhe/src/high_level_api/keys/server.rs index e360482ced..5fb69da913 100644 --- a/tfhe/src/high_level_api/keys/server.rs +++ b/tfhe/src/high_level_api/keys/server.rs @@ -157,7 +157,7 @@ impl AsRef for ServerKey { // in multi-threading scenarios. #[derive(serde::Serialize)] // We directly versionize the `ServerKey` without having to use this intermediate type. -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] +#[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] struct SerializableServerKey<'a> { pub(crate) integer_key: &'a IntegerServerKey, pub(crate) tag: &'a Tag, diff --git a/tfhe/src/high_level_api/strings/ascii/mod.rs b/tfhe/src/high_level_api/strings/ascii/mod.rs index 51bdc13201..a0fa4c74e4 100644 --- a/tfhe/src/high_level_api/strings/ascii/mod.rs +++ b/tfhe/src/high_level_api/strings/ascii/mod.rs @@ -77,11 +77,11 @@ impl<'de> serde::Deserialize<'de> for AsciiDevice { // Only CPU data are serialized so we only versionize the CPU type. #[derive(serde::Serialize, serde::Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] +#[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] pub(crate) struct AsciiDeviceVersionOwned(::VersionedOwned); #[derive(Serialize, Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] +#[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] pub(crate) enum AsciiDeviceVersionedOwned { V0(AsciiDeviceVersionOwned), } diff --git a/tfhe/src/safe_serialization.rs b/tfhe/src/safe_serialization.rs index e29ce2f696..fac2fa6ed1 100644 --- a/tfhe/src/safe_serialization.rs +++ b/tfhe/src/safe_serialization.rs @@ -1,5 +1,9 @@ //! Serialization utilities with some safety checks +// Types in this file should never be versioned because they are a wrapper around the versioning +// process +#![cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] + use std::borrow::Cow; use std::fmt::Display; @@ -29,8 +33,6 @@ const CRATE_VERSION: &str = concat!( /// Tells if this serialized object is versioned or not #[derive(Serialize, Deserialize, Clone, PartialEq, Eq)] -// This type should not be versioned because it is part of a wrapper of versioned messages. -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] enum SerializationVersioningMode { /// Serialize with type versioning for backward compatibility Versioned { @@ -70,8 +72,6 @@ impl SerializationVersioningMode { /// Header with global metadata about the serialized object. This help checking that we are not /// deserializing data that we can't handle. #[derive(Serialize, Deserialize)] -// This type should not be versioned because it is part of a wrapper of versioned messages. -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] struct SerializationHeader { header_version: Cow<'static, str>, versioning_mode: SerializationVersioningMode, diff --git a/tfhe/src/shortint/wopbs/mod.rs b/tfhe/src/shortint/wopbs/mod.rs index a2cb04105a..27b331f046 100644 --- a/tfhe/src/shortint/wopbs/mod.rs +++ b/tfhe/src/shortint/wopbs/mod.rs @@ -17,7 +17,7 @@ mod test; // Struct for WoPBS based on the private functional packing keyswitch. #[derive(Clone, Debug, Serialize, Deserialize)] -#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))] +#[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))] pub struct WopbsKey { //Key for the private functional keyswitch pub wopbs_server_key: ServerKey, diff --git a/utils/cargo-tfhe-lints-inner/Cargo.toml b/utils/cargo-tfhe-lints-inner/Cargo.toml deleted file mode 100644 index 80447dea8f..0000000000 --- a/utils/cargo-tfhe-lints-inner/Cargo.toml +++ /dev/null @@ -1,10 +0,0 @@ -[package] -name = "cargo-tfhe-lints-inner" -version = "0.1.0" -edition = "2021" - -[dependencies] -rustc-tools = "0.80" - -[package.metadata.rust-analyzer] -rustc_private=true diff --git a/utils/cargo-tfhe-lints-inner/README.md b/utils/cargo-tfhe-lints-inner/README.md deleted file mode 100644 index cde2b7603d..0000000000 --- a/utils/cargo-tfhe-lints-inner/README.md +++ /dev/null @@ -1,16 +0,0 @@ -# TFHE-lints-inner - -Implementation of the lints in the custom [tfhe-lints](../cargo-tfhe-lints/README.md) tool. - -## Installation -``` -cargo install --path . -``` - -## Usage -This can be run directly by specifying the toolchain: -``` -cargo +nightly-2024-05-02 tfhe-lints-inner -``` - -For a more ergonomic usage, see [tfhe-lints](../cargo-tfhe-lints/README.md) diff --git a/utils/cargo-tfhe-lints-inner/rust-toolchain.toml b/utils/cargo-tfhe-lints-inner/rust-toolchain.toml deleted file mode 100644 index 9e0f505df3..0000000000 --- a/utils/cargo-tfhe-lints-inner/rust-toolchain.toml +++ /dev/null @@ -1,4 +0,0 @@ -[toolchain] -channel = "nightly-2024-07-25" -components = ["rustc-dev", "rust-src", "llvm-tools-preview"] -profile = "minimal" diff --git a/utils/cargo-tfhe-lints-inner/src/main.rs b/utils/cargo-tfhe-lints-inner/src/main.rs deleted file mode 100644 index c166ed1e19..0000000000 --- a/utils/cargo-tfhe-lints-inner/src/main.rs +++ /dev/null @@ -1,59 +0,0 @@ -#![feature(rustc_private)] -#![warn(clippy::pedantic)] -#![allow(clippy::module_name_repetitions)] -#![allow(clippy::must_use_candidate)] - -mod serialize_without_versionize; -pub mod utils; - -// We need to import them like this otherwise it doesn't work. -extern crate rustc_ast; -extern crate rustc_hir; -extern crate rustc_lint; -extern crate rustc_middle; -extern crate rustc_session; -extern crate rustc_span; - -use rustc_lint::LintStore; -use rustc_tools::with_lints; -use serialize_without_versionize::SerializeWithoutVersionize; - -fn main() { - let tool_args = std::env::args().skip(2).collect::>(); - - let (cargo_args, rustc_args) = if let Some(idx) = tool_args.iter().position(|arg| arg == "--") { - tool_args.split_at(idx) - } else { - (tool_args.as_slice(), &[] as &[String]) - }; - - // The linter calls rustc without cargo, so these variables won't be set. Since we use them in - // our code, we need to set them to any value to avoid a compilation error. - std::env::set_var("CARGO_PKG_VERSION_MAJOR", "X"); - std::env::set_var("CARGO_PKG_VERSION_MINOR", "Y"); - - rustc_tools::cargo_integration(&cargo_args, |args| { - let mut args = args.to_vec(); - args.extend(rustc_args.iter().skip(1).cloned()); - args.extend( - [ - "--emit=metadata", - // These params allows to use the syntax - // `#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))]` - "-Zcrate-attr=feature(register_tool)", - "-Zcrate-attr=register_tool(tfhe_lints)", - "--cfg=tfhe_lints", - ] - .iter() - .map(ToString::to_string), - ); - let serialize_without_versionize = SerializeWithoutVersionize::new(); - - with_lints(&args, vec![], move |store: &mut LintStore| { - let lint = serialize_without_versionize.clone(); - store.register_late_pass(move |_| Box::new(lint.clone())); - }) - .expect("with_lints failed"); - }) - .expect("cargo_integration failed"); -} diff --git a/utils/cargo-tfhe-lints/Cargo.toml b/utils/cargo-tfhe-lints/Cargo.toml deleted file mode 100644 index c2c779838f..0000000000 --- a/utils/cargo-tfhe-lints/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "cargo-tfhe-lints" -version = "0.1.0" -edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dependencies] diff --git a/utils/cargo-tfhe-lints/README.md b/utils/cargo-tfhe-lints/README.md deleted file mode 100644 index 91d98df960..0000000000 --- a/utils/cargo-tfhe-lints/README.md +++ /dev/null @@ -1,25 +0,0 @@ -# TFHE-lints - -A collection of rust lints specific to the **TFHE-rs** project. This tool is built using [rustc-tools](https://github.com/GuillaumeGomez/rustc-tools). - -## List of lints -- `serilaize_without_versionize`: warns if `Serialize` is implemented without `Versionize` - -## Installation - -### Install the inner tool -``` -cargo install --path ../cargo-tfhe-lints-inner -``` - -### Install this wrapper -``` -cargo install --path . -``` - -## Usage -Use it as any other cargo tool: -``` -cargo tfhe-lints -``` -You can specify features like you would do with clippy. diff --git a/utils/cargo-tfhe-lints/src/main.rs b/utils/cargo-tfhe-lints/src/main.rs deleted file mode 100644 index 90fa9ac472..0000000000 --- a/utils/cargo-tfhe-lints/src/main.rs +++ /dev/null @@ -1,50 +0,0 @@ -use std::{ - io::{Error, ErrorKind}, - process::{exit, Command}, -}; - -fn get_supported_rustc_version() -> &'static str { - const TOOLCHAIN_FILE: &str = include_str!("../../cargo-tfhe-lints-inner/rust-toolchain.toml"); - - TOOLCHAIN_FILE - .lines() - .find(|line| line.starts_with("channel")) - .and_then(|line| { - line.rsplit('=') - .next() - .unwrap() - .trim() - .strip_prefix('"') - .unwrap() - .strip_suffix('"') - }) - .unwrap() -} - -fn main() { - let cargo_args = std::env::args().skip(2).collect::>(); - let toolchain = format!("+{}", get_supported_rustc_version()); - - if let Err(err) = Command::new("cargo") - .arg(toolchain.as_str()) - .arg("tfhe-lints-inner") - .args(&cargo_args) - .status() - .and_then(|res| { - if !res.success() { - Err(Error::new( - ErrorKind::Other, - format!("Inner process failed with {res}"), - )) - } else { - Ok(()) - } - }) - { - eprintln!( - "Command `cargo {toolchain} tfhe-lints-inner {}` failed: {err:?}", - cargo_args.join(" "), - ); - exit(1); - } -} diff --git a/utils/tfhe-lints/.cargo/config.toml b/utils/tfhe-lints/.cargo/config.toml new file mode 100644 index 0000000000..e6f34ff7ee --- /dev/null +++ b/utils/tfhe-lints/.cargo/config.toml @@ -0,0 +1,5 @@ +[build] +target-dir = "../../target/tfhe-lints" + +[target.'cfg(all())'] +linker = "dylint-link" diff --git a/utils/tfhe-lints/.gitignore b/utils/tfhe-lints/.gitignore new file mode 100644 index 0000000000..ea8c4bf7f3 --- /dev/null +++ b/utils/tfhe-lints/.gitignore @@ -0,0 +1 @@ +/target diff --git a/utils/tfhe-lints/Cargo.toml b/utils/tfhe-lints/Cargo.toml new file mode 100644 index 0000000000..fbf2d17602 --- /dev/null +++ b/utils/tfhe-lints/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "tfhe-lints" +version = "0.1.0" +description = "Project specific lints for TFHE-rs" +edition = "2021" +publish = false + +[lib] +crate-type = ["cdylib"] + +[dependencies] +clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "ff4a26d442bead94a4c96fb1de967374bc4fbd8e" } +dylint_linting = "3.2.1" + +[dev-dependencies] +dylint_testing = "3.2.1" +serde = { version = "1.0", features = ["derive"] } +tfhe-versionable = "0.4.0" + +[package.metadata.rust-analyzer] +rustc_private = true + +[[example]] +name = "ui" +path = "ui/main.rs" diff --git a/utils/tfhe-lints/README.md b/utils/tfhe-lints/README.md new file mode 100644 index 0000000000..73364f22fd --- /dev/null +++ b/utils/tfhe-lints/README.md @@ -0,0 +1,29 @@ +# Project specific lints for TFHE-rs +This tool is based on [dylint](https://github.com/trailofbits/dylint). + +## Usage +From TFHE-rs root folder: +``` +make tfhe_lints +``` + +## `serialize_without_versionize` + +### What it does +For every type that implements `Serialize`, checks that it also implement `Versionize` + +### Why is this bad? +If a type is serializable but does not implement Versionize, it is likely that the +implementation has been forgotten. + +### Example +```rust +#[derive(Serialize)] +pub struct MyStruct {} +``` +Use instead: +```rust +#[derive(Serialize, Versionize)] +#[versionize(MyStructVersions)] +pub struct MyStruct {} +``` diff --git a/utils/tfhe-lints/rust-toolchain b/utils/tfhe-lints/rust-toolchain new file mode 100644 index 0000000000..e5dee9dafd --- /dev/null +++ b/utils/tfhe-lints/rust-toolchain @@ -0,0 +1,3 @@ +[toolchain] +channel = "nightly-2024-11-28" +components = ["llvm-tools-preview", "rustc-dev"] diff --git a/utils/tfhe-lints/src/lib.rs b/utils/tfhe-lints/src/lib.rs new file mode 100644 index 0000000000..c07ebd3cc4 --- /dev/null +++ b/utils/tfhe-lints/src/lib.rs @@ -0,0 +1,12 @@ +#![feature(rustc_private)] +#![feature(let_chains)] +#![warn(unused_extern_crates)] + +extern crate rustc_ast; +extern crate rustc_hir; +extern crate rustc_lint; +extern crate rustc_middle; +extern crate rustc_span; + +mod serialize_without_versionize; +mod utils; diff --git a/utils/cargo-tfhe-lints-inner/src/serialize_without_versionize.rs b/utils/tfhe-lints/src/serialize_without_versionize.rs similarity index 59% rename from utils/cargo-tfhe-lints-inner/src/serialize_without_versionize.rs rename to utils/tfhe-lints/src/serialize_without_versionize.rs index ac60db53fb..e90069c05e 100644 --- a/utils/cargo-tfhe-lints-inner/src/serialize_without_versionize.rs +++ b/utils/tfhe-lints/src/serialize_without_versionize.rs @@ -3,17 +3,10 @@ use std::sync::{Arc, OnceLock}; use rustc_hir::def_id::DefId; use rustc_hir::{Impl, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::sym; use crate::utils::{get_def_id_from_ty, is_allowed_lint, symbols_list_from_str}; -declare_tool_lint! { - pub tfhe_lints::SERIALIZE_WITHOUT_VERSIONIZE, - Warn, - "warns if `Serialize` is implemented without `Versionize`" -} - #[derive(Default)] pub struct SerializeWithoutVersionizeInner { pub versionize_trait: OnceLock>, @@ -42,14 +35,31 @@ impl SerializeWithoutVersionizeInner { #[derive(Default, Clone)] pub struct SerializeWithoutVersionize(pub Arc); -impl SerializeWithoutVersionize { - pub fn new() -> Self { - Self::default() - } +dylint_linting::impl_late_lint! { + /// ### What it does + /// For every type that implements `Serialize`, checks that it also implement `Versionize` + /// + /// ### Why is this bad? + /// If a type is serializable but does not implement Versionize, it is likely that the + /// implementation has been forgotten. + /// + /// ### Example + /// ```rust + /// #[derive(Serialize)] + /// pub struct MyStruct {} + /// ``` + /// Use instead: + /// ```rust + /// #[derive(Serialize, Versionize)] + /// #[versionize(MyStructVersions)] + /// pub struct MyStruct {} + /// ``` + pub SERIALIZE_WITHOUT_VERSIONIZE, + Warn, + "Detects types that implement Serialize without implementing Versionize", + SerializeWithoutVersionize::default() } -impl_lint_pass!(SerializeWithoutVersionize => [SERIALIZE_WITHOUT_VERSIONIZE]); - impl<'tcx> LateLintPass<'tcx> for SerializeWithoutVersionize { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { // If the currently checked item is a trait impl @@ -74,14 +84,12 @@ impl<'tcx> LateLintPass<'tcx> for SerializeWithoutVersionize { } // Check if the implemented trait is `Serialize` - if let Some(versionize_trait) = self.0.versionize_trait(cx) { - if let Some(def_id) = trait_ref.trait_def_id() { - if cx.match_def_path( - def_id, - symbols_list_from_str(&SERIALIZE_TRAIT).as_slice(), - ) { - // Try to find an implementation of versionize for this type - let mut found_impl = false; + if let Some(def_id) = trait_ref.trait_def_id() { + if cx.match_def_path(def_id, symbols_list_from_str(&SERIALIZE_TRAIT).as_slice()) + { + // Try to find an implementation of versionize for this type + let mut found_impl = false; + if let Some(versionize_trait) = self.0.versionize_trait(cx) { cx.tcx .for_each_relevant_impl(versionize_trait, ty, |impl_id| { if !found_impl { @@ -95,20 +103,20 @@ impl<'tcx> LateLintPass<'tcx> for SerializeWithoutVersionize { } } }); + } - if !found_impl { - // Emit a warning - cx.span_lint( - SERIALIZE_WITHOUT_VERSIONIZE, - cx.tcx.def_span(type_def_id), - |diag| { - diag.primary_message("Type {ty} implements `Serialize` but does not implement `Versionize`"); - diag.note("Add `#[derive(Versionize)] for this type or silence this warning using \ -`#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))]``"); - diag.span_note(item.span, "`Serialize` derived here"); - }, - ); - } + if !found_impl { + // Emit a warning + cx.span_lint( + SERIALIZE_WITHOUT_VERSIONIZE, + cx.tcx.def_span(type_def_id), + |diag| { + diag.primary_message(format!("Type {ty} implements `Serialize` but does not implement `Versionize`")); + diag.note("Add `#[derive(Versionize)]` for this type or silence this warning using \ + `#[cfg_attr(dylint_lib = \"tfhe_lints\", allow(serialize_without_versionize))]`"); + diag.span_note(item.span, "`Serialize` derived here"); + }, + ); } } } @@ -116,3 +124,8 @@ impl<'tcx> LateLintPass<'tcx> for SerializeWithoutVersionize { } } } + +#[test] +fn ui() { + dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "ui"); +} diff --git a/utils/cargo-tfhe-lints-inner/src/utils.rs b/utils/tfhe-lints/src/utils.rs similarity index 80% rename from utils/cargo-tfhe-lints-inner/src/utils.rs rename to utils/tfhe-lints/src/utils.rs index f16b0763a1..644d916c4d 100644 --- a/utils/cargo-tfhe-lints-inner/src/utils.rs +++ b/utils/tfhe-lints/src/utils.rs @@ -4,8 +4,6 @@ use rustc_lint::LateContext; use rustc_middle::ty::{Ty, TyKind}; use rustc_span::Symbol; -const TOOL_NAME: &str = "tfhe_lints"; - /// Converts an array of str into a Vec of [`Symbol`] pub fn symbols_list_from_str(list: &[&str]) -> Vec { list.iter().map(|s| Symbol::intern(s)).collect() @@ -21,13 +19,8 @@ pub fn is_allowed_lint(cx: &LateContext<'_>, target: DefId, lint_name: &str) -> let mut trees = tokens.trees(); if let Some(TokenTree::Token(tool_token, _)) = trees.next() { - if tool_token.is_ident_named(Symbol::intern(TOOL_NAME)) { - trees.next(); - if let Some(TokenTree::Token(tool_token, _)) = trees.next() { - if tool_token.is_ident_named(Symbol::intern(lint_name)) { - return true; - } - } + if tool_token.is_ident_named(Symbol::intern(lint_name)) { + return true; } } } diff --git a/utils/tfhe-lints/ui/main.rs b/utils/tfhe-lints/ui/main.rs new file mode 100644 index 0000000000..9191026306 --- /dev/null +++ b/utils/tfhe-lints/ui/main.rs @@ -0,0 +1,11 @@ +use serde::Serialize; + +#[derive(Serialize)] +struct MyStruct { + value: u64, +} + +fn main() { + let st = MyStruct { value: 42 }; + println!("{}", st.value); +} diff --git a/utils/tfhe-lints/ui/main.stderr b/utils/tfhe-lints/ui/main.stderr new file mode 100644 index 0000000000..ca60122286 --- /dev/null +++ b/utils/tfhe-lints/ui/main.stderr @@ -0,0 +1,17 @@ +warning: Type MyStruct implements `Serialize` but does not implement `Versionize` + --> $DIR/main.rs:4:1 + | +LL | struct MyStruct { + | ^^^^^^^^^^^^^^^ + | + = note: Add `#[derive(Versionize)]` for this type or silence this warning using `#[cfg_attr(dylint_lib = "tfhe_lints", allow(serialize_without_versionize))]` +note: `Serialize` derived here + --> $DIR/main.rs:3:10 + | +LL | #[derive(Serialize)] + | ^^^^^^^^^ + = note: `#[warn(serialize_without_versionize)]` on by default + = note: this warning originates in the derive macro `Serialize` (in Nightly builds, run with -Z macro-backtrace for more info) + +warning: 1 warning emitted +