Zum Inhalt springen

Best practices for writing good CodeCharter rules

Guidelines for writing CodeCharter rules that stay productive in a team, file scope, severity rollout, and spec tests.

Writing rules that are productive in a team is a discipline of its own. Here are the most important guidelines.

1. One Rule per File

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

The .ccr format already enforces this technically: each file holds exactly one set of directives and one query. The discipline lies in the scope — resist writing one broad query that checks several conventions at once.

One @name, one severity, one responsibility per file. This keeps diffs readable and suppressions targeted.

2. Meaningful File Names

The file name (without extension) is the rule ID: it appears in the JSON and SARIF reports and is what you write in suppression comments. rule-42.ccr helps no one. controller-must-end-controller.ccr immediately tells you what's wrong.

Convention: kebab-case, verb phrase where possible.

3. Start Small with Severity info

Setting a new rule straight to error creates friction. Instead:

  1. Set the rule to info.
  2. Let it run for a week and see which locations it catches.
  3. Fix or suppress existing locations.
  4. Promote to warn.
  5. Once the team has established it as a convention: error.

Watch out for typos: an unrecognized @severity value silently falls back to warn, so a misspelled eror will not fail the build.

4. Write @recommendation Honestly

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

Not:

@recommendation "Fix this"

The recommendation is what the developer reads in the build log, in CI annotations, and in the JSON report. If it's useless, the whole rule is useless — devs will disable it rather than follow it.

5. Don't Duplicate Roslyn

If Microsoft.CodeAnalysis.NetAnalyzers already covers a rule (CA1822, CA2007, ...), checking it again with CodeCharter is just noise. Instead:

  • Roslyn analyzers for language-level concerns
  • CodeCharter for team-specific conventions and architecture

6. Keep Architecture Rules Specific

// Good, specific
@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
// Bad, too generic
@name "Layers must not be violated"

from t in Types
where t.UsedTypes.Any(u => true)   // what?
select t

One rule per concrete architectural relationship. If you have five layers, write five rules — don't collapse them into one.

7. Keep an Eye on Performance

Sub-queries on large collections are expensive:

// Expensive when there are 5000 types
from t in Types
where t.UsedTypes.Any(u => u.Methods.Any(m => m.IsAsync))
select t

If you need something like this, check whether you can start from the outer collection directly:

// Direct
from m in Methods
where m.IsAsync
where m.DeclaringType.UsedByTypes.Any(...)
select m

8. Prefer the Typed Statement-Level Facts When Available

Common code smells such as direct DateTime.Now/DateTime.UtcNow usage or fire-and-forget async calls are already exposed as typed facts on every method/constructor/property-accessor body (m.MemberAccesses, m.Invocations, m.Catches, and so on), reachable from the flattened AllBodies collection. Don't try to rebuild these checks with hand-written m.Syntax navigation — read the typed fact directly:

AllBodies.SelectMany(m => m.MemberAccesses.Where(a => (a.Name == "Now" || a.Name == "UtcNow")
    && (a.ResolvedType.FullName == "System.DateTime" || a.ResolvedType.FullName == "System.DateTimeOffset")))

To narrow a finding further, filter on the enclosing member first — every item in AllBodies carries SourceFile and 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<"))))

The typed facts point directly to the location — faster, more readable, and more reliable than approximating the same check by hand.

9. No Magic Strings for Namespaces

If you need to hard-code namespace prefixes, at least centralize them in a shared conventions document or keep them in a small set of architecture rules that you review together:

// In every architecture rule:
where t.Namespace.FullName.StartsWith("Acme.Domain")
where u.Namespace.FullName.StartsWith("Acme.Web")

If you rename a namespace you will have to update every affected rule file. Keeping these rules co-located makes the update visible in a single diff.

10. Suppressions with Justification

Suppressions are plain comments in the code, on the violation line or the line above:

// codecharter-disable generic-exception-catch
// codecharter-disable-next-line no-manager-suffix

A bare // codecharter-disable with nothing after it suppresses all rules for the whole file — use it sparingly. Make it a team convention to add a short reason after the rule ID:

// codecharter-disable generic-exception-catch — legacy interop boundary

Put a delimiter (an em dash, --, or a full stop) between the rule ID and the reason, so free text after it is never mistaken for another rule ID to suppress; see Suppressions for the exact rule. Suppressions without justification leak.

11. Test Your Rules with Specs

Put a <rule-name>.spec.md next to each <rule-name>.ccr with code examples that the rule must hit and examples it must not, then run codecharter test rules/ to verify them through the same engine that runs in the build. codecharter test --scaffold path/to/rule.ccr generates a starter spec next to the rule. A rule without a spec is a rule whose exact behavior nobody has pinned down.

Anti-Patterns

  • Rules that only have select t without any where. This produces one Finding per element in the collection, which is almost never useful.
  • Rules that match almost every file. Either the severity is wrong, or the rule doesn't fit the team.
  • Hundreds of suppressions on one rule. If 80% of findings are suppressed, the rule is wrong.

Reviews

Treat rule changes like code changes — put them through PR review. A new rule changes the behavior of the build system for everyone; that deserves a discussion.