Skip to content

Best Practices

What separates good custom rules from bad ones.

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

Extra words after the rule ID are ignored by the matcher, so the reason travels with the suppression. 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.