Best Practices

Regeln zu schreiben die in einem Team produktiv sind ist eine Disziplin für sich. Hier die wichtigsten Faustregeln.

1. Eine Regel pro Datei

.codeguard/rules/
├── no-manager-suffix.cgr
├── controller-must-end-controller.cgr
└── domain-must-not-reference-web.cgr

Nicht:

.codeguard/rules/
└── all-naming-rules.cgr   # 200 Zeilen, 12 Regeln drin

Pro Datei eine @name, eine Severity, eine Verantwortung. Das macht Diffs lesbar und Suppressions zielgerichtet.

2. Sprechende Dateinamen

Der Dateiname taucht im Build-Log auf wenn die Regel anschlägt. rule-42.cgr nutzt niemandem. controller-must-end-controller.cgr 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.

4. @recommendation ehrlich schreiben

@recommendation "Add 'CancellationToken cancellationToken = default' as the last parameter"

Nicht:

@recommendation "Fix this"

Die Empfehlung ist das was die Entwicklerin im VS-Code-Hover sieht und im Build-Log liest. Wenn sie nutzlos ist, ist die ganze Regel nutzlos - Devs deaktivieren sie statt sie zu befolgen.

5. Nicht doppelt zu Roslyn

Wenn Microsoft.CodeAnalysis.NetAnalyzers eine Regel schon abdeckt (CA1822, CA2007, ...), nochmal mit CodeGuard zu prüfen ist Lärm. Lieber:

• Roslyn-Analyzer für sprachen-nahe Sachen
• CodeGuard 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.StartsWith("Acme.Domain")
where t.UsedTypes.Any(u => u.Namespace.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 du sowas brauchst, prüf vorher ob du gleich auf der äußeren Collection landen kannst:

# Direkt
from m in Methods
where m.IsAsync
where m.DeclaringType.UsedByTypes.Any(...)
select m

8. SyntaxIssues bevorzugen wenn vorhanden

Statt:

from m in Methods
where m.CalledMethods.Any(c => c.FullName == "System.DateTime.UtcNow.get")
select m

Lieber:

SyntaxIssues.Where(s => s.Kind == "DateTimeDirectUsage")

SyntaxIssues sind vor-analysiert und liefern direkt die Stelle. Schneller, lesbarer, weniger fragile.

9. Keine Magic Strings für Namespaces

Wenn ihr Namespace-Prefixes hartcodieren müsst, zentralisiert sie:

# In jeder Regel:
where t.Namespace.StartsWith("Acme.Domain")
where u.Namespace.StartsWith("Acme.Web")

Bei einer Umbenennung müsst ihr durch alle Files. Eine konventionelle Lösung gibt es heute nicht, aber wir denken über DSL-Konstanten für v2 nach.

10. Suppressions mit Begründung

Wenn ihr require-justification = true setzt, müssen alle Suppressions einen Kommentar haben. Empfehlung: einstellen. Suppressions ohne Begründung lecken aus.

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.
• Regeln die nur auf einer einzigen Stelle anschlagen. Vielleicht ist das ein Suppression-Pattern verkehrt herum, ihr wolltet die eine Stelle markieren, dafür braucht ihr keine Regel.
• 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.