Regeln zu schreiben, die in einem Team produktiv sind, ist eine Disziplin für sich. Hier die wichtigsten Faustregeln.
1. Eine Regel pro Datei
rules/
├── no-manager-suffix.ccr
├── controller-must-end-controller.ccr
└── domain-must-not-reference-web.ccr
Das .ccr-Format erzwingt das bereits technisch: jede Datei enthält genau
einen Satz Direktiven und genau eine Query. Die Disziplin liegt im Zuschnitt:
Widersteht der Versuchung, eine breite Query zu schreiben, die mehrere
Konventionen auf einmal prüft.
Pro Datei eine @name, eine Severity, eine Verantwortung. Das macht
Diffs lesbar und Suppressions zielgerichtet.
2. Sprechende Dateinamen
Der Dateiname (ohne Endung) ist die Rule-ID: Sie taucht in den JSON- und
SARIF-Reports auf und ist das, was ihr in Suppression-Kommentare schreibt.
rule-42.ccr nutzt niemandem. controller-must-end-controller.ccr sagt
direkt, was los ist.
Konvention: kebab-case, Verb-Phrase wenn möglich.
3. Klein anfangen mit Severity info
Eine neue Regel direkt auf error zu stellen produziert Reibung. Stattdessen:
- Regel auf
infosetzen. - Eine Woche laufen lassen, schauen, welche Stellen erfasst werden.
- Fixen oder Suppressions setzen für die existierenden Stellen.
- Auf
warnhochstufen. - Wenn das Team sie als Konvention etabliert hat:
error.
Vorsicht bei Tippfehlern: Ein unbekannter @severity-Wert fällt stillschweigend
auf warn zurück; ein vertipptes eror lässt den Build also nicht fehlschlagen.
4. @recommendation ehrlich schreiben
@recommendation "Add 'CancellationToken cancellationToken = default' as the last parameter"
Nicht:
@recommendation "Fix this"
Die Empfehlung ist das, was Entwickler im Build-Log, in CI-Annotations und im JSON-Report lesen. Wenn sie nutzlos ist, ist die ganze Regel nutzlos; Entwickler deaktivieren sie, statt sie zu befolgen.
5. Keine Dopplung zu Roslyn
Wenn Microsoft.CodeAnalysis.NetAnalyzers eine Regel schon abdeckt
(CA1822, CA2007, ...), nochmal mit CodeCharter zu prüfen ist Lärm.
Lieber:
- Roslyn-Analyzer für sprachen-nahe Themen
- CodeCharter für team-spezifische Konventionen und Architektur
6. Architektur-Regeln spezifisch halten
// Gut, spezifisch
@name "Domain layer must not reference Web layer"
from t in Types
where t.Namespace.FullName.StartsWith("Acme.Domain")
where t.UsedTypes.Any(u => u.Namespace.FullName.StartsWith("Acme.Web"))
select t
// Schlecht, zu generisch
@name "Layers must not be violated"
from t in Types
where t.UsedTypes.Any(u => true) // what?
select t
Eine Regel pro konkrete Architektur-Beziehung. Wenn ihr fünf Layer habt, dann fünf Regeln, nicht eine, die alle zusammenwirft.
7. Performance im Auge behalten
Sub-Queries auf großen Collections sind teuer:
// Teuer wenn 5000 Typen
from t in Types
where t.UsedTypes.Any(u => u.Methods.Any(m => m.IsAsync))
select t
Wenn ihr so etwas braucht, prüft vorher, ob ihr gleich auf der äußeren Collection landen können:
// Direkt
from m in Methods
where m.IsAsync
where m.DeclaringType.UsedByTypes.Any(...)
select m
8. Die typisierten Statement-Level-Facts bevorzugen wenn vorhanden
Verbreitete Code-Smells wie direkte DateTime.Now/DateTime.UtcNow-Nutzung
oder Fire-and-Forget-Async-Aufrufe stehen bereits als typisierte Facts an
jedem Methoden-/Konstruktor-/Property-Accessor-Body zur Verfügung
(m.MemberAccesses, m.Invocations, m.Catches, usw.), erreichbar über
die abgeflachte AllBodies-Collection. Versucht nicht, diese Prüfungen
mit einer eigenen m.Syntax-Traversierung nachzubauen, sondern lest
den typisierten Fact direkt:
AllBodies.SelectMany(m => m.MemberAccesses.Where(a => (a.Name == "Now" || a.Name == "UtcNow")
&& (a.ResolvedType.FullName == "System.DateTime" || a.ResolvedType.FullName == "System.DateTimeOffset")))
Wenn ihr ein Finding weiter eingrenzen wollt, filtert zuerst auf den
Eigenschaften des umschließenden Members. Jedes Element unter AllBodies
trägt SourceFile und DeclaringType:
AllBodies
.Where(m => !m.SourceFile.Contains("Tests"))
.SelectMany(m => m.Invocations.Where(i => i.IsUnobservedStatement
&& (i.ResolvedType.FullName == "System.Threading.Tasks.Task"
|| i.ResolvedType.FullName.StartsWith("System.Threading.Tasks.Task<"))))
Die typisierten Facts liefern direkt die Stelle. Schneller, lesbarer und zuverlässiger als die gleiche Prüfung von Hand anzunähern.
9. Keine Magic Strings für Namespaces
Wenn ihr Namespace-Prefixes hartcodieren müsst, zentralisiert sie in einem gemeinsamen Konventions-Dokument oder haltet sie in einer kleinen Gruppe von Architektur-Regeln, die ihr zusammen reviewt:
// In jeder Architektur-Regel:
where t.Namespace.FullName.StartsWith("Acme.Domain")
where u.Namespace.FullName.StartsWith("Acme.Web")
Bei einer Umbenennung müsst ihr alle betroffenen Regel-Dateien anpassen. Wenn ihr diese Regeln nebeneinander haltet, wird die Änderung in einem einzigen Diff sichtbar.
10. Suppressions mit Begründung
Suppressions sind einfache Kommentare im Code, auf der Zeile des Findings oder der Zeile darüber:
// codecharter-disable generic-exception-catch
// codecharter-disable-next-line no-manager-suffix
Ein nacktes // codecharter-disable ohne Rule-ID unterdrückt alle Regeln für
die ganze Datei; setzt es sparsam ein. Macht es zur Team-Konvention,
nach der Rule-ID eine kurze Begründung anzuhängen:
// codecharter-disable generic-exception-catch — legacy interop boundary
Setzt ein Trennzeichen (einen Geviertstrich, -- oder einen Punkt)
zwischen Rule-ID und Begründung, damit freier Text danach nie versehentlich
als weitere zu unterdrückende Rule-ID gelesen wird; die genaue Regel steht
unter Suppressions. Suppressions ohne
Begründung reißen ein.
11. Regeln mit Specs testen
Legt neben jede <rule-name>.ccr eine <rule-name>.spec.md mit
Code-Beispielen, die die Regel treffen muss, und Beispielen, die sie nicht
treffen darf. codecharter test rules/ prüft sie dann mit derselben Engine,
die auch im Build läuft. codecharter test --scaffold pfad/zur/regel.ccr
erzeugt eine Start-Spec neben der Regel. Eine Regel ohne Spec ist eine Regel,
deren genaues Verhalten niemand festgehalten hat.
Anti-Patterns
- Regeln, die nur
select tohnewherehaben. Erzeugt ein Finding pro Element der Collection, meist sinnlos. - Regeln, die fast jede Datei treffen. Die Severity ist falsch, oder die Regel passt nicht zum Team.
- Hunderte Suppressions auf einer Regel. Wenn 80% der Findings unterdrückt werden, ist die Regel falsch.
Reviews
Behandelt Regel-Änderungen wie Code-Änderungen, durch PR-Review. Eine neue Regel verändert das Verhalten des Build-Systems für alle; das verdient eine Diskussion.