Strengeres Clippy

Es gibt da ein Zitat, dessen Urheber ich nicht mehr finde, deswegen gebe ich es hier sinngemäß wieder (das Original war meine ich auf Englisch):

cargo clippy ist für diejenigen, die eine gewisse Leere fühlen, wenn der Rust-Compiler endlich ihren Code akzeptiert.

Der Rust-Compiler ist notorisch dafür, pingelig zu sein (was ich allerdings in diesem Fall für eine gute Eigenschaft halte). Cargo clippy (benannt nach der berüchtigten MS-Office-Büroklammer aus den 90ern) ist ein Linter für Rust, der unschöne unsaubere oder einfach uneinheitliche Codestellen findet, auflistet, und einige davon auch (nach Aufforderung) selbst korrigieren kann.

Clippy hat schon per default einen Haufen Regeln, und ich benutze es schon seit vielen Jahren. Bisher aber immer nur mit den Standardeinstellungen. Bis ich dann letzte Woche durch „This Week in Rust“ auf zwei Blogposts aufmerksam geworden bin: Your Clippy Config Should Be Stricter und als Antwort darauf, von einer anderen Person Your Clippy Config Should Be Stricter-er.

Im ersten Post geht es hauptsächlich darum, welche Lints man aktivieren sollte, wenn man nicht aus Versehen Stellen, die ein panic!() machen können, im Code haben will, plus ein paar anderer Lints für potentielle fallen. Der zweite Post ist eine Antwort darauf, und der Autor vertritt die Ansicht, man solle einfach ganze Listen von Lints freischalten und die, die man nicht haben will, wieder deaktivieren. So hat man eine allow-list und kann keine potentiell nützlichen Lints übersehen.

Ich wollte das einfach einmal ausprobieren und habe dazu direkt mein größtes aktives Rust-Projekt genommen: Den Generator für dieses Blog.

Pedantisch

Als erstes habe ich die „pedantischen“ lints aktiviert, indem ich diese Zeilen in meine Cargo.toml eingefügt habe:

[lints.clippy]
pedantic = { level = "warn", priority = -1 }

Alle weiteren Änderungen, die ich hier an der Cargo.toml vornehme, werden auch in dem lints.clippy-Block stehen.

Die pedantischen Regeln haben sofort zu einem Haufen Fehler geführt. Für einige davon gab es automatische Fixes, für andere nicht. Ich gehe jetzt nicht genauer auf alle Regeln ein, aber ich habe zum Beispiel ein paar hilfreiche Funktionen entdeckt. So kann man map().unwrap_or_else() an einer Option durch ein map_or_else() ersetzen.

Es waren auch ein paar Lints dabei, die theoretisch ein Problem sein könnten, aber praktisch nicht. Wie zum Beispiel ein type casting mit as von einem u64 in ein usize. Hier ging es um Dateigrößen. Praktisch gesehen werde ich keine Dateien haben, die mehr als 4 GiB groß sind (falls usize nur 32 bit groß ist), aber ich habe den Lint trotzdem drin gelassen, grundsätzlich ist er nützlich. Auch ein casting von u64 auf f64 wurde wegen möglichen Präzisionsverlustes bemäkelt. In diesem Fall habe ich die Regel ignoriert, weil ich die Zahl sowieso gerundet habe. Nicht global ignoriert, sondern nur für diese Stelle, damit ich auch an anderen Stellen gewarnt werde. Dazu habe ich am Anfang der Funktion, in der die Warnung auftrat, diese Zeile hinterlassen:

#[expect(clippy::cast_precision_loss, reason = "precision loss not a problem here")]

Ich habe expect genutzt anstatt allow, damit ich eine Warnung kriege, falls diese Ausnahme jemals überflüssig wird (z.B. weil ich den Code geändert habe). Ich finde ich es praktisch, dass man direkt einen Grund angeben kann. Auf diese beiden Details bin ich aber erst durch weitere Lints aus einem anderen Set aufmerksam geworden, aber dazu weiter unten mehr.

Die einzige pedantische Regel, die ich komplett deaktiviert habe, war die Längenbeschränkung für Funktionen. Meiner Erfahrung nach bringen willkürliche Längenbeschränkungen für Funktionen mehr Ärger als Nutzen. Eine lange, aber übersichtliche Funktion möchte ich nicht aufteilen, nur um dem Linter gerecht zu werden. In meinem Code war zwar auch nur eine Funktion davon betroffen, aber ich habe die Regel lieber global deaktiviert, mit folgender Zeile in der Cargo.toml:

too_many_lines = "allow"

Abgesehen davon waren die Regeln eigentlich allesamt recht brauchbar.

Restriktionen

Als nächstes habe ich die restrictions lints aktiviert:

restriction = { level = "warn", priority = -2 }

Und meine Güte. Die sind wirklich nicht dafür geeignet. Die erste Warnung, die man bekommt, ist, dass man diese Regeln nicht en bloc aktivieren sollte. Ich habe das mal ignoriert. Insgesamt sind bei mir seitenweise Warnungen aufgetaucht, von zusammengenommen 40 Regeln (darunter auch die Regel, dass man diese Regeln nicht en bloc aktivieren soll). Nur 11 von den Regeln habe ich behalten, den Rest habe ich deaktiviert. Ich habe die Regeln, die ich behalten habe, auch explizit aktiviert, falls mir die restrictions-Liste irgendwann mal zu nervig werden sollte, dann behalte ich wenigstens die Regeln, die ich für brauchbar befunden habe.

Brauchbare Regeln

Diese Regeln habe ich behalten, teilweise weil ich sie wirklich gut fand, teilweise weil ich sie nicht zu nervig fand:

allow_attributes verhindert, dass Regelausnahmen mit [allow()] mache, ich muss stattdessen except nehmen
allow_attributes_without_reason erzwingt, dass ich bei Regelausnahmen einen Grund angeben muss
ref_patterns erzwingt, statt etwas wie if let Some(ref a) = b etwas wie if let Some(a) = &b zu schreiben
default_numeric_fallback verhindert, dass bei Integerliteralen der Fallbacktyp verwendet wird, man muss immer einen Typ angeben
str_to_string statt str::to_string() soll lieber str::to_owned() verwendet werden
doc_paragraphs_missing_punctuation sorgt für einheitliche Strukturen in Doc-Kommentaren (in diesem Projekt eher unwichtig, schadet aber nicht)
create_dir statt create_dir lieber create_dir_all verwenden, das äquivalent zu mkdir -p. Würde ich nicht immer empfehlen, aber für dieses Projekt schon
arithmetic_side_effects markiert Stellen mit potentiellen Overflow-Effekten, Divisionen durch 0 bei arithmetischen Operatoren (+, -, *, /). Ich habe es mal aktiviert, obwohl meine Zahlen klein genug sind, dass eigentlich keine Überläufe passieren können. Ich habe stattdessen strict_add und vergleichbare Funktionen verwendet, das panicked bei Overflows. Für anderen Code würde ich vielleicht eher checked_add oder overflowing_add verwenden, je nachdem welches Verhalten gewünscht ist. Ein Nachteil: Arithmetische Ausdrücke sind danach viel schwieriger zu lesen.
wildcard_enum_match_arm: Wenn man kein catch-all beim Enum-Matching haben will. Hilft, damit neu hinzugefügte Enum-Varianten nicht übergangen werden. Für die Fälle, wo ich wirklich catch-alls haben wollte, habe ich Ausnahmen gesetzt.
as_conversions verhindert Typkonversionen mit as, die potentiell zu Fehlern führen. Auch hier habe ich mich entschieden, dass mein Programm im Fehlerfall lieber panicken soll. Eine Ausnahme habe ich hinzugefügt, weil ich keine Ahnung hatte, die ich sonst verlustbehaftet von u64 zu f64 casten sollte.
unwrap_used verhindert, dass man unwrap verwendet. Definitiv eine gute Regel. Wenn überhaupt, sollte man expect verwenden, dann kriegt man noch eine Information dazu, warum man meinte, dass ein Wert Some() oder Ok() sein sollte. Ich habe tatsächlich ein paar expects im Code, weil ich z.B. einen Formatter verwende, der in einen String-Buffer schreibt, was nicht schiefgehen kann.

Ein unwrap bin ich bei der Gelegenheit auch eine schöne Art und Weise losgeworden. Ich hatte zunächst eine match-expression die ungefähr so aussah:

let description = match blogposts.len() {
    0 => "Keine Blogposts".to_string(),
    1 => format!(
        "Blogposts vom {}",
        blogposts[0].0.date.format(DATE_FORMAT_GERMAN)
    ),
    _ => format!(
        blogposts[0].0.date.format(DATE_FORMAT_GERMAN),
        blogposts.last().unwrap().0.date.format(DATE_FORMAT_GERMAN)
    )
};

last() kann unten nie None zurückgeben, weil die Länge mindestens 2 sein muss. Aber unschön ist es trotzdem. Da ist mir eingefallen, dass es ja mittlerweile slice patterns gibt. Dasselbe Problem lässt sich also viel schöner so lösen:

let description = match blogposts {
    [] => "Keine Blogposts".to_owned(),
    [only] => format!("Blogposts vom {}", only.0.date.format(DATE_FORMAT_GERMAN)),
    [first, .., last] => format!(
        "Blogposts vom {} bis zum {}",
        first.0.date.format(DATE_FORMAT_GERMAN),
        last.0.date.format(DATE_FORMAT_GERMAN)
    ),
};

gegebenenfalls brauchbare Regeln

Das sind Regeln, die ich in manchen Situationen für brauchbar halte, aber nicht in diesem Projekt hier:

single_char_lifetime_names verbietet Lifetime-Namen wie 'a. Für ein neues Projekt vielleicht eine gute Idee, hier nicht.
unneeded_field_pattern verhindert, dass man beim destructuring eines structs unbenutzte Felder ausschreibt anstatt .. zu verwenden. Ich habe die mal deaktiviert, weil ich im Zweifelsfall sicher sein will, dass ich kein Feld übersehen habe. .. kann ich ja trotzdem nutzen, wenn ich will
missing_docs_in_private_items macht Doc-Kommentare für private Elemente verpflichtend. Halte ich in dieser Codebasis nicht für Sinnvoll
module_name_repetitions verhindert, dass man Worte in der Module-Hierarchie doppelt verwendet, z.B. file::FileData. Grundsätzlich keine schlechte Idee, aber Data als Datentyp war mir dann doch zu generisch
min_ident_chars verhindert Bezeichner, die nur aus einem Zeichen bestehen. Ich verwende solche aber gerne in sehr kurzen Ausdrücken, wo die Bedeutung des Namens nicht verloren gehen kann. Ich muss dann halt manuell aufpassen, dass sich die Variable i nicht doch über die ganze Funktion zieht.
if_then_some_else_none verlangt, dass man etwas wie if v.is_empty() { Some(42) } else { None }; stattdessen let a = v.is_empty().then(|| {42}); ausschreibt. Ich finde die erste Variante doch ein bisschen einfacher zu lesen (zumindest, wenn sie umgebrochen ist)
absolute_paths: verbietet absolute Pfade für Bezeichner im Code, man solle die lieber in die use-statements packen. Könnte ich drüber nachdenken, aber gerade mit den ganzen verschiedenen Result-Typen gelangt man da schnell in Namenskonflikte.
indexing_slicing verbietet, direkte slice-Operationen à la a[i]. Gut, um sich vor panics abzusichern. Mein Code ist aber nicht kritisch, in diesem Fall habe ich die Regel deaktiviert
print_stdout verbietet print! und println!. Hilfreich, wenn man keine Ausgabe mit ´println!haben möchte. In diesem Fall ist es aber eine Kommandozeilen-App mit wenig Ausgabe, und daher will ich meinprintln!` schon haben
expect_used analog zu unwrap_used: verbietet expect() bei Result und Option. Gut, wenn man keine panics haben möchte. Für dieses Projekt habe ich aber, wie oben beschrieben, eine Handvoll expects, mit denen ich klarkommen muss.
Regeln für Nischenfälle

Und zum Schluss noch ein paar Regeln, die ich überhaupt nicht haben möchte bzw. die für sehr enge Nischenfälle gedacht sind:

missing_trait_methods verhindert, dass man ein trait implementiert ohne alle Funktionen des traits zu implementieren. Eine Hölle, wenn man Iteratoren implementiert. Die Doku sagt, man solle es am besten nur für Einzelfälle aktivieren (also für einzelne Trait-Implementierungen)
implicit_return verbietet impliztes return am Ende der Funktion. Da implizites return aber in den Standard-Clippy-Regeln erzwungen wird, bleibe ich lieber bei den Standardregeln.
mod_module_files verbietet mod.rs-module files. Ich komme mit dieser Struktur aber gut klar
std_instead_of_alloc verpflichtet, für bestimmte use statements aus alloc und nicht aus std zu importieren. Gut für std-only-crates
std_instead_of_core s.o. nur, für core anstatt alloc
unused_trait_names möchte, dass man traits nur nicht-anonym importiert, wenn man sie beim Namen verwendet, nicht, weil man sie nur für die Nutzung des traits importiert werden. Ich sehe den Nutzen hier nicht wirklich.
arbitrary_source_item_ordering erzwingt, dass man im Quellcode eine bestimmte Reihenfolge einhält, z.B. alle structs for alle Funktionen. Im Zweifelsfall verschlechtert das aber imho eher die Übersichtlichkeit.
shadow_reuse Ich mag das shadowing-Verhalten von rust. Wem das zu gefährlich ist, kann gerne diese Regel aktivieren
single_call_fn warnt vor Funktionen, die in der Codebasis nur ein Mal genutzt werden. Aber für mich ist es strukturell schon manchmal sinnvoll, eine solche Funktion zu schreiben.
non_ascii_literal erlaubt nur Ascii-Zeichen in Stringliteralen. Falls es Leute gibt, deren Editor kein UTF-8 kann. Mal im Ernst: wir habe 2026. Wenn euer Editor nach über 30 Jahren UTF-8 immer noch kein UTF-8 unterstützt, ist das nicht mein Problem. Wäre aber vielleicht für sicherheitsrelevanten Code interessant, um Homoglraph-Attacken zu verhindern.
pattern_type_mismatch irgendwas mit borrows und pattern matching. Hier vertraue ich aber einfach dem Compiler, dass der sich beschwert, wenn es ein Problem gibt
integer_division ich weiß, dass bei Integer-Division der Rest verloren geht, danke.
integer_division_remainder_used ist als Schutz gegen timing-Seitenkanalangriffe gedacht. Für Kryptographiecode. Das ist eine wichtige Nischenanwendung, aber halt eine Nischenanwendung und in meinem Code hier irrelevant.
impl_trait_in_params verhindert die impl Trait-Syntax in Funktionsparametern. Ich finde impl Trait eigentlich ganz praktisch, deswegen: raus mit der Regel
unseparated_literal_suffix dazu gibt es eine Gegenregel separated_literal_suffix. Man kann sich maximal eine aussuchen.
iter_over_hash_type verbietet iteratoren über ungeordnete Datenstrukturen (z.B. HashSet). Ich weiß aber, wie HashSets funktionieren und will über die Werte iterieren, danke.
float_arithmetic verbietet Fließkommazahlrechnungen. Praktisch für Mikroprozessoren, die keine FP-Hardware haben und das alles Simulieren müssen (mit extremen Performanceeinbußen). Also wieder nur eine Nischenanwendung.

Fazit

Ich habe an einigen Stellen meinen Code stabiler und sauberer gemacht. ob ich die restriction-Regeln weiterhin en bloc drin lasse oder nicht, muss ich mir aber noch überlegen. Ein paar von den Regeln sind jedoch ganz in Ordnung.