Skip to content

docs: clarify MethodDataSourceAttribute.Factory is source-generator-managed#5835

Merged
thomhurst merged 2 commits intomainfrom
copilot/remove-methoddatasourceattribute-factory
May 5, 2026
Merged

docs: clarify MethodDataSourceAttribute.Factory is source-generator-managed#5835
thomhurst merged 2 commits intomainfrom
copilot/remove-methoddatasourceattribute-factory

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 5, 2026

MethodDataSourceAttribute.Factory confused users into thinking it could be set in attribute syntax — it can't, since C# attributes only accept literal values and Func<T> is not one. The property exists solely for the TUnit source generator to populate at compile time with a generated delegate that calls the named method directly (enabling AOT/trimming compatibility without reflection).

Changes

  • MethodDataSourceAttribute.Factory XML doc — replaced the terse description with an expanded <remarks> block that explains:
    • The property is automatically set by the source generator; manual assignment in attribute syntax is not possible
    • What the source generator actually emits: a delegate that directly invokes the data-source member, bypassing reflection

Copilot AI linked an issue May 5, 2026 that may be closed by this pull request
1 task
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 5, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

…ttribute syntax

Agent-Logs-Url: https://github.com/thomhurst/TUnit/sessions/31a2fe52-f921-4815-a7b3-4f0eaae13b49

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove MethodDataSourceAttribute.Factory due to incorrect usage docs: clarify MethodDataSourceAttribute.Factory is source-generator-managed May 5, 2026
Copilot AI requested a review from thomhurst May 5, 2026 15:15
@thomhurst thomhurst marked this pull request as ready for review May 5, 2026 15:15
@thomhurst thomhurst merged commit 07d0823 into main May 5, 2026
5 of 6 checks passed
@thomhurst thomhurst deleted the copilot/remove-methoddatasourceattribute-factory branch May 5, 2026 15:16
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This is a documentation-only PR that clarifies the purpose of MethodDataSourceAttribute.Factory in the XML doc comments.

Summary of changes

The PR replaces a one-line summary (Gets or sets an AOT-safe factory function for providing test data programmatically. When set, this factory is used instead of reflection-based member lookup.) with an expanded <remarks> block that explains:

  1. The property is populated automatically by the TUnit source generator — it cannot be set in attribute syntax because Func<T> is not a C# attribute-legal value.
  2. The source generator emits a delegate that calls the named method directly, enabling AOT/trimming compatibility.

Assessment

The change is correct and the new documentation is accurate. Looking at the implementation in GetDataRowsAsync, Factory is indeed checked first (if (Factory != null)) and the reflection-based path only runs when it is null, which is consistent with the description.

A few minor observations:

Accuracy — the summary line after the change reads "Gets or sets an AOT-safe factory function that provides test data without reflection." The "Gets or sets" phrasing is technically accurate (it is a settable property) but slightly misleading alongside the remark that manual assignment is not possible in attribute syntax. It is still settable via post-construction object initializer syntax, so the phrasing is not wrong. If the intent is to further discourage external setting, adding [EditorBrowsable(EditorBrowsableState.Never)] to the property would hide it from IntelliSense completion without changing runtime behavior — but that is a separate concern outside the scope of this PR.

Missing <example> block — the sibling properties (Arguments, SkipIfEmpty) and the class-level doc include <example> elements. The Factory property does not have one here, but given that the source generator manages this property exclusively, there is no useful user-facing example to add, so omitting it is fine.

Formatting — the use of <strong> in an XML doc <remarks> block renders correctly in generated API docs (DocFX, Sandcastle) and in VS tooltip tooltips (via the bold rendering in the IDE); no concern there.

Overall, this is a clear, accurate, and well-scoped documentation improvement. No issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: MethodDataSourceAttribute.Factory is incorrect

2 participants