Zum Inhalt springen

Best Practices für gute eigene CodeCharter-Regeln

Leitlinien für CodeCharter-Regeln, die im Team produktiv bleiben, Datei-Zuschnitt, Severity-Rollout und Spec-Tests.

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:

  1. Regel auf info setzen.
  2. Eine Woche laufen lassen, schauen, welche Stellen erfasst werden.
  3. Fixen oder Suppressions setzen für die existierenden Stellen.
  4. Auf warn hochstufen.
  5. 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 t ohne where haben. 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.