[Quattordici]

This PR holds the production fixes split out of #7129. It is stacked on #7129, so until that PR merges this one also shows its commits; the change itself is the last commit, fix: reapply the analyzer, Aspire and [AssertionFrom] fixes split out of #7129. Once #7129 merges I'll merge main in so only that commit remains.

Before: TUnit0014 flagged public methods that implement an interface member. Aspire printed out-of-memory exit codes with the current culture, so sv-SE showed a U+2212 minus sign. [AssertionFrom] custom ExpectationMessage values rendered without "to" (Expected be null or empty), {param} placeholders were not substituted, attributes that shared a method lost all but the first message, and overloads that mapped to the same generated class name all used the first overload's class.

After: TUnit0014 skips interface implementations. Exit codes are formatted with the invariant culture. Custom expectation messages read "to …" / "to not …" with NegateLogic, {param} placeholders (including non-ASCII names) are substituted, {{ / }} print literal braces, each attribute keeps its own message, and each overload gets its own condition class. [AssertionFrom] diagnostics are reported at the attribute.

How: AssertionMethodGenerator renders message text in one tokenizer pass, decides the "to"/"not" wording in one place, and resolves generated class names at emit time through per-overload references, so colliding names get numbered classes instead of being dropped. The attribute location is only computed when a diagnostic is reported, which keeps incremental caching intact.

Release note: [AssertionFrom] ExpectationMessage is now written without the leading "to"; the generator adds "to", and "not" with NegateLogic. Messages that already start with "to " or "to not " render as before.

Related Issue

Split out of #7129.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Performance improvement
• Refactoring (no functional changes)

Checklist

Required

• I have read the Contributing Guidelines
• If this is a new feature, I started a discussion first and received agreement
• My code follows the project's code style (modern C# syntax, proper naming conventions)
• I have written tests that prove my fix is effective or my feature works

TUnit-Specific Requirements

• Dual-Mode Implementation: not applicable, no discovery or execution changes
• Snapshot Tests: If I changed source generator output or public APIs:
  • I ran TUnit.Assertions.SourceGenerator.Tests and TUnit.PublicAPI tests
  • I reviewed the .received.txt files and accepted them as .verified.txt
  • I committed the updated .verified.txt files
• Performance: generator work only; the attribute location is computed lazily
• AOT Compatibility: not applicable, no reflection changes

Testing

• All existing tests pass
• I have added tests that cover my changes
• I have tested both source-generated and reflection modes (if applicable)

Additional Notes

The review points from the github-actions reviews on #7129 about the generator are addressed here: tests for the message forms (ExpectationMessageRenderingTests, BuiltInExpectationMessageTests), a guard that no class-name marker reaches the output, numbered classes for colliding overloads (TU0002 is gone), and the lazy attribute location.

Summary by CodeRabbit

• Enhancements

  • Improved assertion expectation messages with consistent “to”/“not” wording.
  • Added support for parameter placeholders, escaped braces, literal text, and clearer negation handling in custom expectation messages.
  • Improved generated assertion naming and diagnostic locations for clearer build feedback.

• Bug Fixes

  • Interface implementations are no longer incorrectly flagged as missing test attributes.
  • Exit-code and diagnostic formatting is now consistent across cultures.
  • Removed generated span emptiness assertions; compatibility types now direct users to the supported memory assertion APIs.

• Documentation

  • Expanded guidance for custom expectation message formatting and placeholders.

[Quattordici]

Review of #7135

Overall this is a well-scoped set of fixes with good documentation. I read the production changes in the analyzer, Aspire and [AssertionFrom] generator. I did not build the PR or run any tests.

What looks good

• TUnit0014: ImplementsInterfaceMember uses FindImplementationForInterfaceMember against the containing type's AllInterfaces. That is the right API, and it correctly covers implicit implementations.
• Aspire: every exit-code format site now goes through FormatExitCodeNumber with the invariant culture. The OutOfMemory branch also handles a null exit code.
• Generator: the single-pass tokenizer (RenderMessageText) is much safer than the old string interpolation, which did not escape quotes or braces. SymbolDisplay.FormatLiteral handles quotes and backslashes. Keeping non-ASCII parameter names working through a broad [^{}]* match is a nice call.
• Diagnostics: only the SyntaxReference flows through the pipeline, and the location is computed only when a diagnostic is reported. DiagnosticInfo is now shared instead of nested, which keeps incremental caching intact.

Concerns and suggestions

1. In-band string markers (\u0001/\u0002) for class-name resolution. The approach works, and the comment says an unresolved marker fails the build loudly. But pre-rendering strings and then patching them with string.Replace is fragile: any new place that emits a class name must remember to use ClassNameReference. A more robust design is to keep the rendered snippets as small structured parts (a literal / class-ref sequence) and resolve names at emit time. Alternatively, derive a unique class name deterministically at transform time, for example by appending a short hash of the overload signature when a method has more than one overload mapping to the same name. That would remove the global emit-time dedup state (GeneratedConditionClasses) and make the generated names stable across edits. With the current approach, adding or removing an unrelated attribute can change which attribute gets the 2 suffix, and so change public generated type names.
2. Numbered-name collisions. Claim adds suffixes with _names.Add in a loop. That is fine for ordering within one run, but the suffix depends on attribute iteration order. Please make sure that order is deterministic (sorted by something stable), or snapshot output can flap.
3. "not " prefix handling in RenderCustomExpectation. A message starting with "not " is emitted as to not … and ignores _negated. If such an attribute is used without NegateLogic, or the negation is toggled at runtime, the wording stays fixed. This is an edge case, but a test for it would pin down the intended behaviour. Also, the heuristic strips a leading "to " from any message, so a legitimate phrase like "to-do…" is unaffected but "to " followed by prose is silently altered. That is acceptable given the release note.
4. Scope. The PR is stacked on Make the test suite pass in every environment and run fully in CI #7129 and also contains .slnx additions, test-helper changes and new test projects (e.g. TUnit.SourceGenerator.IncrementalTests). Once Make the test suite pass in every environment and run fully in CI #7129 merges, please verify that the diff really is the single intended commit. The generator snapshot tests and tests/TUnit.PublicAPI should be run, since the attribute docs and generated output changed.
5. Behaviour change. The release note on ExpectationMessage is good. Consider also calling it out in docs/.../source-generator-assertions.md (already touched) with a before/after example, so existing users who wrote "to be …" know both forms still work.

No blocking issues found. The main suggestion is #1, moving toward structured or deterministic naming instead of marker substitution.

🤖 Generated with Claude Code

Agreed, the order dependence was the real problem. 8d7b295 removes the markers, string.Replace and the order-based numbering:

• Overloads sharing a name are named when the attribute is rendered: the first by parameter count, then signature, keeps the plain name, and the others get a stable hash of their signature. That depends only on the method's own overloads.
• Attributes on one method with different messages (possibly in different classes) are resolved at emit time as a pure function of the set: the source that sorts first keeps the plain name, the others get a hash of their source, renamed token by token.

TUnit's own output and the PublicAPI snapshots are unchanged. ConditionClassNamingTests pins that names don't depend on declaration order or unrelated attributes.

[github-actions]

Review of #7135

I read the production changes. The most important part is AssertionMethodGenerator. I did not build the project or run the tests, and I only skimmed the analyzer and Aspire diffs. The /code-review skill failed to run, so this is a manual review.

What looks good

• Class names no longer depend on declaration or emit order. They come from a sort by source plus an FNV-1a hash, and string.GetHashCode is not used. Line endings are normalized before hashing, so names do not vary by host.
• RenderMessageText handles placeholders, {{ }}, stray braces and non-ASCII parameter names in one regex pass. This replaces the earlier ad hoc string replacement.
• The "to" / "not" wording is decided in one place, RenderNegatableExpectation.
• The SyntaxReference is turned into a Location only when a diagnostic is actually reported. The DiagnosticInfo model and the comment about never caching AttributeSyntax keep incremental caching intact.
• Diagnostics now carry the attribute location instead of Location.None, so #pragma and .editorconfig severities apply.

Suggestions / risks

1. Backward-compatible prefix stripping. RenderCustomExpectation strips a leading "to " and special-cases "not ". That is a sensible compatibility shim, but a message like "to" or "not equal" written in the new style is treated as already negated. Under NegateLogic that renders to not equal, which is probably correct, but without NegateLogic it never gets the negation toggle. I suggest a unit test for a positive message that starts with "not " (for example, "not be empty" as the non-negated form), or a note in the docs.
2. RenameIdentifier reparses source with SyntaxFactory.ParseTokens. It runs only on collisions, so the cost is fine. It does work on already-rendered strings inside the emit step, which is fragile. Longer term, carrying the class name as a placeholder token in the rendered source and substituting once at emit time would remove the reparse and the second naming pass (GetConditionClassName and AssignConditionClassNames).
3. Two-stage naming is hard to reason about. GetConditionClassName hashes overloads per method, and AssignConditionClassNames then hashes colliding variants globally. The salt loop terminates, but a short comment or a test showing a case that hits both layers would help future maintainers.
4. Release note. The ExpectationMessage semantics change (the generator now adds "to"). The compatibility path covers messages that already start with "to ". Other custom messages will read differently, so please make sure the release notes call this out.

No blocking issues found. Overall this is a solid fix, with good test coverage listed in the diff (naming, message rendering, incremental, and snapshot tests).

[github-actions]

Review of PR 7135

I reviewed the diff directly, because the code-review skill failed to run. I didn't build the project or run any tests.

Overall: The fixes are well-reasoned, and the tests and snapshots cover each one. The changes are TUnit0014 skipping interface implementations, Aspire formatting exit codes with the invariant culture, [AssertionFrom] messages rendered in a single tokenizer pass, per-overload condition classes, and attribute-located diagnostics computed lazily so incremental caching still works.

Concerns

1. Hash-suffixed class names depend on the generated source text. AssignConditionClassNames derives the hash for non-first variants from the generated source. Editing an ExpectationMessage therefore renames the generated class, so names are stable across builds but not across edits. The classes look like internal helpers, so this is probably fine. If anything public or user-visible references them (stack traces, downstream snapshot tests), the churn will show. Please confirm nothing public exposes them. If something does, derive the hash from the attribute's identity (target method signature plus an attribute index) instead of the message text.

2. Naming is split across two passes. GetConditionClassName handles overloads and AssignConditionClassNames handles attributes, each with its own collision logic and WithHash call. The doc comments explain the split well. Still, a single resolver at emit time, keyed by (method symbol, attribute), would be easier to reason about than two passes that both have to stay order-independent.

3. Leading 'to ' and 'not ' are handled by prefix sniffing. RenderCustomExpectation strips a leading to and treats a leading not as already negated. A message such as 'not found' without NegateLogic therefore renders as 'to not found' and ignores negation. Apply the 'not ' special case only when NegateLogic is set. If the current behaviour must stay, add a test that pins it.

4. Release note. Dropping the leading 'to' is a user-visible behaviour change. The PR description calls it out. Please also make sure it lands in the docs page and the release notes.

5. Minor. MessageTokenRegex is a static field declared mid-class, so move it up with the other fields. Consider RegexOptions.CultureInvariant, in line with the rest of this PR's culture handling.

Positives

• The analyzer fix uses FindImplementationForInterfaceMember, the correct way to detect implicit implementations.
• CultureInfo.InvariantCulture for the Aspire exit codes fixes the sv-SE minus sign.
• Computing the diagnostic location lazily keeps the incremental pipeline's cache hits intact.

No blocking issues from my read. Item 3 is the one I'd resolve or pin with a test before merge.

[github-actions]

Review: PR 7135 (analyzer, Aspire and [AssertionFrom] fixes)

I read the source changes and found nothing blocking. I did not build the PR or run its tests. I could not run the automated code-review skill, so this review is a manual read of the diff.

What looks good

• TUnit0014: ImplementsInterfaceMember uses FindImplementationForInterfaceMember over AllInterfaces, so it handles implicit implementations correctly, including inherited interfaces.
• Aspire: Formatting exit codes with the invariant culture is correct. Every place that prints one now goes through FormatExitCodeNumber. The ExitCode is null branch in DescribeState is a small extra fix.
• Message rendering: RenderMessageText handles {param}, {{/}}, unknown placeholders, stray braces, and quotes and backslashes (via FormatLiteral) in one tokenizer pass. RenderNegatableExpectation is now the only place that decides the "to"/"to not" wording.
• Compatibility: Accepting messages that already start with to or to not keeps existing usages rendering as before, and the docs describe this.
• Class naming: Names no longer depend on declaration or emit order. Overload names use a signature hash, member collisions get a namespace prefix, and Claim re-hashes with a salt if a hash collides. That is sturdier than the earlier placeholder substitution. The hash is FNV-1a, not string.GetHashCode, so names stay stable across builds. The line endings are normalised before hashing.
• TU0001 diagnostics: DiagnosticInfo holds only primitive location data. The location is computed only when a diagnostic is reported, and the diagnostic is reattached to the current compilation's syntax tree. That preserves incremental caching and lets #pragma and editorconfig severities apply.
• Tests: The new tests cover the message forms, class-name collisions, a guard against leaked name markers, and incremental behaviour.

Non-blocking suggestions

1. Repeated overload scan: GetConditionClassName scans every overload of the method each time it is called, and it is called from the condition-class path and the extension-method path. That is O(n²) per member for heavily overloaded APIs. If a type with many overloads ever shows up in generator timings, compute the names once per (containingType, methodName) and reuse them.
2. Token-based rename: RenameIdentifier renames by token and arity. That is sound, but it re-parses the whole generated source for every renamed class. It only runs for collisions, so the cost is fine. A short comment pointing at the test that covers a same-named class of a different arity would help future maintainers.
3. Breaking change: The release note covers the ExpectationMessage change. Consider also calling it out in the migration or changelog notes. A message such as "not null" was previously rendered as not not null when negated, and is now treated as an already-negated phrase, so NegateLogic is not applied to it.

Overall this is a clean, well-tested fix, and I would approve it.

[github-actions]

Review of #7135

I read the diff for the analyzer, Aspire and [AssertionFrom] changes. I did not build the PR or run its tests, and the code-review skill failed to start, so this was a manual read. I found no blocking issues.

What looks good

• TUnit0014: ImplementsInterfaceMember uses FindImplementationForInterfaceMember over AllInterfaces, which is the correct way to detect implicit implementations. Explicit implementations are not public methods, so they are not affected. The analyzer tests cover the case.
• Aspire exit codes: Every place that formats an exit code now goes through FormatExitCodeNumber with the invariant culture. This includes the OOM branch, which also gets a sensible fallback when ExitCode is null.
• Incremental caching:
  • Holding a SyntaxReference in the model and building the Location only when a diagnostic is reported avoids a per-edit cache miss.
  • DiagnosticInfo stores only primitives and re-attaches to the tree by path, falling back to a checksum when paths collide. That keeps #pragma and .editorconfig severity working without keeping old compilations alive.
• Condition class naming: Resolving names at emit time per overload, and numbering collisions instead of dropping them, fixes the "all overloads share the first class" bug properly. The new naming tests cover it.
• Message rendering: The single tokenizer pass handles {{/}}, placeholders and non-ASCII parameter names. It replaces the earlier ad-hoc string scanning.

Suggestions (non-blocking)

1. SyntaxReference invariant: The remark on the model says the record must never cross a cache boundary. AssertionClassModel, which holds the SyntaxReference, is the transform output. That makes the invariant rely on the SyntaxReference being excluded from the model's equality, or on the incremental tests catching a regression. Please confirm the equality exclusion is explicit (for example [IgnoreDataMember]-style custom Equals, or storing the reference outside the compared members), and keep an incremental test that edits an unrelated file and asserts every step is Cached.
2. Diagnostics re-run: diagnostics.Combine(CompilationProvider) re-runs on every compilation change whenever any TU0001 exists. Your doc comment already says so, and it is acceptable because it only applies in the error case. DiagnosticInfo.ToDiagnostic also hashes full tree text on ambiguous paths, which is rare but potentially costly. If it ever shows up, cache the checksum per tree.
3. Regex in the generator: MessageTokenRegex is fine, but give it RegexOptions.CultureInvariant. A static tokenizer loop would also avoid the Regex dependency in the netstandard2.0 analyzer assembly.
4. Release note: Dropping the leading "to" from ExpectationMessage is a behavior change. Existing messages that start with "to " or "to not " still render as before, which softens it. Please make sure the release note and docs example make the new convention explicit (the docs diff already touches this).

Overall this is a clean split-out with solid test coverage. Approving in spirit.

[github-actions]

Review of #7135

I read the generator changes (AssertionMethodGenerator.cs) and skimmed the other source changes. I didn't build the PR or run any tests.

What looks good

• Message rendering is now one tokenizer pass (RenderMessageText). It handles {{/}}, non-ASCII parameter names, stray braces and quote/backslash escaping, and unknown placeholders stay literal. This is better than the ad-hoc replacements it replaces.
• The "to" / "to not" wording is decided in one place (RenderNegatableExpectation). Messages that already start with "to " still render as before, so the breaking change is softened.
• Class names are stable across builds and don't depend on declaration order. They use FNV-1a rather than GetHashCode, and line endings are normalised in HostIndependent.
• Computing the attribute location only when a diagnostic is reported keeps incremental caching intact.

Concerns and suggestions (non-blocking)

1. Complexity of the naming scheme. Naming is split across GetConditionClassName (per overload) and AssignConditionClassNames (per attribute). RenameIdentifier re-parses the generated source with SyntaxFactory.ParseTokens to rename classes after the fact. A cleaner design would give each condition class a name key (member identity, arity, source hash) when the model is built, and emit with that name directly. Then no post-hoc token rewrite is needed. If that's too large a change here, please at least document the invariant that the rename only touches identifiers with matching arity.
2. Public names derived from message text. A non-first variant gets a hash of its source, which includes the message. Editing an ExpectationMessage can therefore rename a generated public class when two attributes share a member. This only affects the colliding case, but a short note in the docs would help.
3. "to " prefix heuristic. Stripping a leading "to " and treating a leading "not " specially is a silent behavioural rule. A message that legitimately starts with "to " (for example "to-do items…") is unaffected, but one like "to not …" can't be told apart from a caller who wants a literal "not". This is acceptable. Please make sure the release note and AssertionFromAttribute XML docs state the rule explicitly.
4. Test coverage. The new tests (ExpectationMessageRenderingTests, ConditionClassNamingTests) are a good start. Please also cover generic overloads with different arity, and nested generic type arguments in TypeArgumentCount, since >> handling is subtle.
5. The PR is stacked on Make the test suite pass in every environment and run fully in CI #7129, so the diff is large until that merges. Please re-check the snapshots after the main merge, so no .received.txt files slip in.

Overall this is a solid fix with good tests. My concerns are mostly about long-term maintainability of the naming machinery.

🤖 Generated with Claude Code

[thomhurst]

Thanks!