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

This commit is contained in:
Nicolas Sarlin
2024-12-16 14:55:17 +01:00
committed by Nicolas Sarlin
parent 7103a83ce5
commit 9a64c34989
35 changed files with 199 additions and 250 deletions

View File

@@ -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

View File

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

View File

@@ -1,4 +0,0 @@
[toolchain]
channel = "nightly-2024-07-25"
components = ["rustc-dev", "rust-src", "llvm-tools-preview"]
profile = "minimal"

View File

@@ -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::<Vec<_>>();
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");
}

View File

@@ -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]

View File

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

View File

@@ -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::<Vec<_>>();
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);
}
}

View File

@@ -0,0 +1,5 @@
[build]
target-dir = "../../target/tfhe-lints"
[target.'cfg(all())']
linker = "dylint-link"

View File

@@ -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"

View File

@@ -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 {}
```

View File

@@ -0,0 +1,3 @@
[toolchain]
channel = "nightly-2024-11-28"
components = ["llvm-tools-preview", "rustc-dev"]

View File

@@ -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;

View File

@@ -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<Option<DefId>>,
@@ -42,14 +35,31 @@ impl SerializeWithoutVersionizeInner {
#[derive(Default, Clone)]
pub struct SerializeWithoutVersionize(pub Arc<SerializeWithoutVersionizeInner>);
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");
}

View File

@@ -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<Symbol> {
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;
}
}
}

View File

@@ -0,0 +1,11 @@
use serde::Serialize;
#[derive(Serialize)]
struct MyStruct {
value: u64,
}
fn main() {
let st = MyStruct { value: 42 };
println!("{}", st.value);
}

View File

@@ -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