Skip to content
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

chore(lint): use dylint as lint driver for tfhe-lint #1933

Merged
merged 1 commit into from
Jan 14, 2025
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
4 changes: 4 additions & 0 deletions .linelint.yml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 6 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ members = [

exclude = [
"tests/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 }
Expand Down Expand Up @@ -47,3 +46,8 @@ inherits = "dev"
opt-level = 3
lto = "off"
debug-assertions = false

[workspace.metadata.dylint]
libraries = [
{ path = "utils/tfhe-lints" }
]
31 changes: 19 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -416,10 +415,15 @@ 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: install_cargo_dylint # the toolchain is selected with toolchain.toml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toolchain auto installs itself ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then perfect

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 \
clippy_versionable
clippy_versionable clippy_tfhe_lints

.PHONY: clippy_fast # Run main clippy targets
clippy_fast: clippy_rustdoc clippy clippy_all_targets clippy_c_api clippy_js_wasm_api clippy_tasks \
Expand All @@ -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,strings,zk-pok

.PHONY: build_core # Build core_crypto without experimental features
build_core: install_rs_build_toolchain install_rs_check_toolchain
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion tasks/src/check_tfhe_docs_are_tested.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const DIR_TO_IGNORE: [&str; 3] = [
"tests/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
Expand All @@ -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> {
Expand Down
2 changes: 1 addition & 1 deletion tfhe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,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)',
] }
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ pub enum DynamicDistributionVersions<T: UnsignedInteger> {
}

#[derive(Serialize)]
#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))]
pub enum CompressionSeedVersioned<'vers> {
V0(&'vers CompressionSeed),
}
Expand All @@ -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),
}
Expand Down
2 changes: 0 additions & 2 deletions tfhe/src/core_crypto/backward_compatibility/fft_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]>),
}
Expand All @@ -31,7 +30,6 @@ impl<'vers, C: Container<Element = c64>> From<&'vers FourierPolynomialList<C>>

// 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<ABox<[c64]>>),
}
Expand Down
2 changes: 2 additions & 0 deletions tfhe/src/core_crypto/backward_compatibility/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tfhe/src/core_crypto/fft_impl/fft64/math/fft/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,12 +623,12 @@ impl<C: Container<Element = c64>> serde::Serialize for FourierPolynomialList<C>
) -> Result<S::Ok, S::Error> {
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<S: serde::Serializer>(
&self,
Expand Down
1 change: 0 additions & 1 deletion tfhe/src/high_level_api/backward_compatibility/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
2 changes: 0 additions & 2 deletions tfhe/src/high_level_api/backward_compatibility/integers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
2 changes: 2 additions & 0 deletions tfhe/src/high_level_api/backward_compatibility/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tfhe/src/high_level_api/booleans/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<crate::integer::BooleanBlock as VersionizeOwned>::VersionedOwned,
);
Expand Down
2 changes: 1 addition & 1 deletion tfhe/src/high_level_api/integers/signed/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<crate::integer::SignedRadixCiphertext as VersionizeOwned>::VersionedOwned,
);
Expand Down
2 changes: 1 addition & 1 deletion tfhe/src/high_level_api/integers/unsigned/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<crate::integer::RadixCiphertext as VersionizeOwned>::VersionedOwned,
);
Expand Down
2 changes: 1 addition & 1 deletion tfhe/src/high_level_api/keys/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl AsRef<crate::integer::ServerKey> 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,
Expand Down
4 changes: 2 additions & 2 deletions tfhe/src/high_level_api/strings/ascii/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(<FheString as VersionizeOwned>::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),
}
Expand Down
8 changes: 4 additions & 4 deletions tfhe/src/safe_serialization.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tfhe/src/shortint/wopbs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 0 additions & 10 deletions utils/cargo-tfhe-lints-inner/Cargo.toml

This file was deleted.

16 changes: 0 additions & 16 deletions utils/cargo-tfhe-lints-inner/README.md

This file was deleted.

4 changes: 0 additions & 4 deletions utils/cargo-tfhe-lints-inner/rust-toolchain.toml

This file was deleted.

59 changes: 0 additions & 59 deletions utils/cargo-tfhe-lints-inner/src/main.rs

This file was deleted.

Loading
Loading