[thomhurst]

The nullable method-row regression fixture still disables TUnit0001 pending #7610, although its analyzer fix has merged. Remove the suppression and obsolete comment so the fixture also guards against analyzer regressions.

Rechecked #7573 against origin/main at 55f4100. #7613 (3dfd584) supplies compatible int/int? analyzer binding and constraint coverage; #7614 supplies generated/reflection discovery parity and constructor precedence. Both native children are closed and both fixes are ancestors of this branch. Their merged PRs record full analyzer/generator and Native AOT validation with passing CI.

Closes #7573

Validation (SDK 10.0.403, net10.0, macOS arm64):

• Built the test project with TUnit0001 enabled in the fixture; all 39 targeted _7573 cases passed in generated mode.
• All 39 targeted cases passed with --reflection.
• git diff --check passed.
• No engine, generator, public API or reflection implementation changed. Existing build warnings remain tracked in Analyzer and code-fixer projects emit release-tracking and nullable build warnings #7410, Mock and core generator test restores report vulnerable System.Security.Cryptography.Pkcs versions #7412 and Generated instance hooks emit IL2075 during Native AOT publishing #7430; engine integration tests remain CI-only.

CI follow-up: updated the nullable row generator snapshot to account for the two removed source lines (30 source-location entries; no other generated differences). The affected generator snapshot test passed on net10.0 after reproducing the failure. Initial multi-framework execution could not run net8.0/net9.0 because those runtimes are absent locally; net10.0 was run explicitly. git diff --check passed.

Summary by CodeRabbit

• Tests
  • Updated generated test snapshots to report source line numbers two lines earlier for nullable class method-row cases.
  • Removed a warning suppression and its accompanying comment from a test fixture. The test declarations and implementation remain unchanged, and no changes to generated test invocation, data sources, or registration behavior are indicated.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The nullable generic class method-row fixture no longer suppresses TUnit0001. Its verified generated test-entry metadata now lists source lines two lines earlier. The generated invocation, data-source, and registration logic are unchanged.

Changes: Nullable method-row regression

Layer / File(s)	Summary
Remove suppression and refresh generated metadata tests/TUnit.TestProject/Bugs/_7573/NullableClassMethodRows.cs, tests/TUnit.Core.SourceGenerator.Tests/NullableClassMethodRowsTests.Generates_Nullable_Class_From_Method_Rows.verified.txt	The fixture removes the TUnit0001 suppression and its related comment. The verified entries record source lines two lines earlier.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other · Severity of issue fixed: Medium

Possibly related issues

• thomhurst/TUnit#7610 — The issue concerns the analyzer behavior tracked by the fixture’s removed TUnit0001 suppression.
• thomhurst/TUnit#7611 — The issue concerns nullable generic method-row discovery in the same regression fixture.

Possibly related PRs

• thomhurst/TUnit#7591 — Added the nullable generic class method-data-source validation and the regression fixture used here.
• thomhurst/TUnit#7614 — Changed discovery for nullable generic class method rows in the same regression area.
• thomhurst/TUnit#7613 — Fixed TUnit0001 validation for rows passed to a nullable generic class parameter.

Merge Risk: Minimal · up to ae746

This change only re-enables analyzer coverage and refreshes matching generated metadata.

Pre-merge checks: 4 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Linked Issues check		Issue #7573 requires TUnit0001 to accept an IEnumerable method data source for a T? parameter on a generic class with where T : struct, and requires analyzer coverage. This PR does not change ana…	Confirm from repository evidence at head that the TUnit0001 analyzer accepts an int method source for a T? parameter on a generic class constrained with where T : struct. Identify the analyzer test file(s) that cover this case, or confirm t…

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "Remove obsolete nullable method-row analyzer suppression" matches the main change. The PR removes the TUnit0001 suppression and its comment from NullableClassMethodRows.cs, and the generate…
Out of Scope Changes check		The whole-PR diff against merge base 55f4100 contains two files. The fixture change removes only the TUnit0001 suppression and its obsolete comment. This change directly supports the issue objective o…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Title check

Explanation

The title "Remove obsolete nullable method-row analyzer suppression" matches the main change. The PR removes the TUnit0001 suppression and its comment from NullableClassMethodRows.cs, and the generated verified snapshot shifts by two lines as a result. The title is specific and concise, and a teammate scanning history would understand the primary change.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against merge base 55f4100 contains two files. The fixture change removes only the TUnit0001 suppression and its obsolete comment. This change directly supports the issue objective of guarding against TUnit0001 regressions for this case. The snapshot change updates only the 30 source-location lineNumber values that shift by two lines after the removal. No generated invocation, data-source, or registration logic changes. No engine, generator, public API, or reflection code changes appear in the diff. No unrelated change was found.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

Full details: Linked Issues check

Explanation

Issue #7573 requires TUnit0001 to accept an IEnumerable<int> method data source for a T? parameter on a generic class with where T : struct, and requires analyzer coverage. This PR does not change analyzer code. Its diff changes only the fixture file tests/TUnit.TestProject/Bugs/_7573/NullableClassMethodRows.cs (removes the #pragma warning disable TUnit0001 line and its comment) and the generator snapshot NullableClassMethodRowsTests.Generates_Nullable_Class_From_Method_Rows.verified.txt (30 lineNumber entries, each shifted by -2). The fixture contains the reproduction case (NullableValue, MethodDataSource(nameof(NullableValues)) with IEnumerable<int?>, and Values/Factory/Tuple variants). Removing the suppression makes TUnit0001 active for these cases. The analyzer fix is reported as merged in #7613 and present in the merge base 55f4100. I could not read the TUnit0001 analyzer source or its tests at head within the inspection limit. Therefore I cannot independently confirm that the analyzer accepts int for T? at the reviewed head, or that analyzer coverage exists at head. The build and test results in the PR description are author-reported and were not verified from repository evidence.

Resolution

Confirm from repository evidence at head that the TUnit0001 analyzer accepts an int method source for a T? parameter on a generic class constrained with where T : struct. Identify the analyzer test file(s) that cover this case, or confirm that the fixture build with TUnit0001 enabled passes in CI. Then the assessment can be re-run to reach a verdict.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the rows at dawn,
The warning’s quiet, the old lines gone.
Two lines shift in the verified page,
The tests still hop from stage to stage.
Soft paws mark the source lines right,
Then tuck the fixture in for night.}

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: bc010613-f6ca-4d9d-ba7b-75676b3aa07f

Commits

Reviewing files that changed from the base of the PR and between 55f4100 and ae7465d.

Files selected for processing: 2

• tests/TUnit.Core.SourceGenerator.Tests/NullableClassMethodRowsTests.Generates_Nullable_Class_From_Method_Rows.verified.txt
• tests/TUnit.TestProject/Bugs/_7573/NullableClassMethodRows.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.

[thomhurst]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T05:44:46.210331Z	ae7465d	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Review: LGTM. The change only removes the obsolete #pragma warning disable TUnit0001 and its comment from the nullable method-row fixture, now that the analyzer fix (#7610/#7613) has merged. This lets the fixture catch analyzer regressions. It changes no engine or generator code, so there is no dual-mode or AOT concern. The PR description records the targeted cases passing in generated and reflection modes. I did not build or run them myself. CI's analyzer build of the TestProject will confirm that no TUnit0001 diagnostic remains. No issues found.

[greptile-apps]

Confidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

Removes the obsolete TUnit0001 suppression and its comment from the nullable method-row fixture.

• The nullable method-row fixture is checked with TUnit0001 enabled.

_{Reviews (2) · Last reviewed commit: "Update nullable row snapshot source loca..." · Reviewed by Greptile}

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 8892e7e2a1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[thomhurst]

The earlier wait on run 41968545548 completed successfully. The subsequent snapshot fix at ae7465d has positive current-head Claude and Codex verdicts and no review threads or blocking findings. Marked ready again with the head unchanged. Promotion started Claude review run https://github.com/thomhurst/TUnit/actions/runs/41975811189 and CI. Read the promotion review after it passes, then arm auto-merge with scripts/Enable-AutoMerge.ps1; the repository gate requires that promotion review before arming.

[github-actions]

Review: LGTM. This removes the stale #pragma warning disable TUnit0001 and its comment now that the analyzer fix for #7610 has merged. The fixture now guards against analyzer regressions. It's a test-only change with no engine, generator or API impact. The PR description reports 39 targeted cases passing in both generated and reflection modes. CI should confirm the build has no TUnit0001 warnings, because warnings are errors in this repo. I have no concerns.

[thomhurst]

@codex review

The previous Claude wait completed successfully. Fixed the CI snapshot failure in ae7465d: removing the obsolete two-line suppression shifts all 30 generated source locations by two. Verified that these are the only snapshot differences; the affected generator snapshot test now passes on net10.0. Returned to draft for fresh review.

[github-actions]

Review: LGTM.

The change removes the stale #pragma warning disable TUnit0001 and its comment, so the fixture now also guards against analyzer regressions now that #7610's fix has merged.

• The snapshot update matches the source change. Two lines were removed from the top of the file, so every lineNumber in the verified file drops by exactly 2. The diff has no other generated differences.
• No engine, generator or public API code changed, so no extra Native AOT or reflection-path concerns arise.
• I didn't build the project or run the tests. The PR description says the 39 targeted _7573 cases pass in both generated and reflection modes, and CI will confirm the analyzer is clean without the suppression.

One minor thing: the snapshot's lineNumber values are brittle to any edit above the test methods. That's pre-existing and not worth changing here.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: ae7465dab3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Review: LGTM.

The change removes the obsolete #pragma warning disable TUnit0001 and its stale comment, now that #7610's analyzer fix has merged. The fixture therefore also guards against analyzer regressions.

• The snapshot diff only shifts lineNumber by -2 across 30 entries. That matches the two removed source lines, and I saw no other generated differences.
• No engine, generator or public API code changed, so the metadata-collection parity and AOT constraints aren't affected.
• The PR description records generated and --reflection runs of the 39 targeted cases.

One optional point: the snapshot's lineNumber values will break again whenever the fixture's header changes. That's an existing characteristic of these snapshots, so I wouldn't change it in this PR.

Approving from my side, pending CI.

[thomhurst]

The earlier blocker is cleared: promotion review run 41975811189 passed for ae7465d, with Claude LGTM and a completed, finding-free Codex verdict for the same head. All current CI checks passed; Assert-PrGreen reports 33 green checks and no unresolved threads.

Disposition of CodeRabbit's inconclusive linked-issue assessment: analyzer coverage is present at this head in MethodDataSourceAnalyzerTests.cs, including Nullable_Generic_Class_Parameter_Accepts_Value_Data with both int and int? rows for T? where T : struct, plus explicit-type and constructor-inferred cases. The Ubuntu pipeline passed its build and test pipeline with the suppression removed. No additional change is needed. Claude's optional note about snapshot line-number sensitivity describes existing behavior and is deferred from this PR as suggested by the reviewer.

Proceeding with the repository merge wrapper.