[code-simplifier] Simplify BuildEvaluators with Array.ConvertAll method group

[Evangelink]

Code Simplification — 2026-06-13

This PR simplifies code introduced in #9071 (MemberConditionAttribute) to improve clarity and conciseness while preserving all functionality.

File Simplified

• src/TestFramework/TestFramework/Attributes/TestMethod/MemberConditionAttribute.cs — Replace manual for-loop in BuildEvaluators with Array.ConvertAll + method group

Improvement Made

Replaced manual array-fill loop with Array.ConvertAll

The BuildEvaluators method allocated a new Func<bool>[], then filled it with a for-loop. Array.ConvertAll expresses the same intent in one line using a method group:

// Before (9 lines)
private Func<bool>[] BuildEvaluators()
{
    var evaluators = new Func<bool>[_conditionMemberNames.Length];
    for (int i = 0; i < _conditionMemberNames.Length; i++)
    {
        evaluators[i] = BuildEvaluator(_conditionMemberNames[i]);
    }
    return evaluators;
}

// After (2 lines)
private Func<bool>[] BuildEvaluators()
    => Array.ConvertAll(_conditionMemberNames, BuildEvaluator);

Array.ConvertAll is available on netstandard2.0, uses a method group instead of a lambda (avoids a delegate wrapper allocation), and makes the transform intent immediately clear.

Changes Based On

Recent changes from:

• Add [MemberCondition] attribute for static-member-based test conditions (#9070) #9071 — Add [MemberCondition] attribute for static-member-based test conditions

Testing

• ✅ Build succeeds (dotnet build src/TestFramework/TestFramework/TestFramework.csproj)
• ✅ No IDE0046 or other style-rule violations
• ✅ No functional changes — behavior is identical (same array output, same exception paths)

Review Focus

Please verify:

• Array.ConvertAll output is equivalent to the for-loop
• Method group BuildEvaluator correctly binds to Func<bool> BuildEvaluator(string)
• No unintended side effects

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Code Simplifier workflow. · 1.7K AIC · ⌖ 34 AIC · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/code-simplifier.md@main

• expires on Jun 14, 2026, 2:05 PM UTC

[Copilot]

Pull request overview

This PR performs a small refactor in the MSTest MemberConditionAttribute implementation (TestFramework) by simplifying BuildEvaluators() to use Array.ConvertAll, preserving the existing evaluator-building behavior while making the intent more direct.

Changes:

• Replaced a manual array allocation + for loop in BuildEvaluators() with Array.ConvertAll(_conditionMemberNames, BuildEvaluator).
• Preserved the same exception paths and per-member evaluator construction via the existing BuildEvaluator(string) method.

Show a summary per file

File	Description
src/TestFramework/TestFramework/Attributes/TestMethod/MemberConditionAttribute.cs	Simplifies BuildEvaluators() by converting member-name strings to evaluator delegates via Array.ConvertAll using a method group.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 0

[Evangelink]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Code Review: BuildEvaluators → Array.ConvertAll refactoring

Verdict Table

#	Dimension	Severity	Status	Finding
1	Algorithmic Correctness	MAJOR	✅ Clean	Array.ConvertAll applies the converter to each element in order, producing an identical result array to the original for-loop
2	Threading & Concurrency	BLOCKING	✅ Clean	No changes to threading behaviour; _evaluators ??= caching is pre-existing and unchanged
3	Public API Surface & Compatibility	BLOCKING	✅ Clean	BuildEvaluators is private; no public API surface touched
4	Nullability & Defensive Coding	MAJOR	✅ Clean	InvalidOperationException from BuildEvaluator propagates through Array.ConvertAll identically to the for-loop path
5	Performance & Allocation	MAJOR	⚠️ Note	See note below — PR description slightly mischaracterises the allocation story, but the runtime impact is negligible
6	Error Handling	MAJOR	✅ Clean	Exception paths are fully preserved
7	Localization	MAJOR	✅ Clean	No string changes
8	Code Style & Conventions	MINOR	✅ Clean	Expression-bodied => form matches the style of FormatMemberList, ConditionMemberNames, GroupName in the same file
9	Test Coverage	MAJOR	✅ Clean	Pure semantics-preserving refactoring; existing tests provide the same coverage
10	Documentation & Comments	MINOR	✅ Clean	Private method — no XML docs needed
11	Security	BLOCKING	✅ Clean	No security implications
12	Cross-TFM Compatibility	MAJOR	✅ Clean	Array.ConvertAll<TInput,TOutput> has been available since .NET 2.0 / netstandard2.0
13	Init Accessor Rule	BLOCKING	✅ Clean	No init accessors introduced
14	Analyzer & Diagnostic Compliance	MINOR	✅ Clean	Method-group usage triggers no SA/CA/MSTest analyzer violations
15	Scope Discipline	MINOR	✅ Clean	Single method, single file, single concern
16	Backward Compatibility	MAJOR	✅ Clean	Identical observable behaviour
17	Build Infrastructure	MINOR	✅ Clean	No .csproj / .targets changes
18	Dependency Management	MINOR	✅ Clean	No new package references
19	File & Namespace Organization	MINOR	✅ Clean	No structural changes
20	Simplicity	MINOR	✅ Clean	9 lines → 2 lines; intent is immediately legible
21	Pattern Matching / Language Features	MINOR	✅ Clean	Array.ConvertAll + method group is idiomatic C# for this projection pattern
22	PublicAPI.Unshipped.txt	BLOCKING	✅ Clean	Private method change; no public API declaration needed

---

⚠️ Note — Performance characterisation in PR description (informational only, not a blocker)

The PR description states that using a method group "avoids a delegate wrapper allocation." This is slightly misleading. In the original code the body called BuildEvaluator(...) as a direct instance-method invocation — no delegate was ever created for the dispatch itself. In the new code, passing BuildEvaluator as a method group to Array.ConvertAll requires allocating one Converter<string, Func<bool>> delegate that captures this. That is one extra allocation relative to the original.

In practice the impact is completely negligible because BuildEvaluators is only ever called once per attribute instance (guarded by _evaluators ??= ...). This note is purely for accuracy — it is not a request to change the code.

---

Overall Assessment

This is a clean, correct, and focused simplification. All 22 review dimensions pass. The refactoring is semantically equivalent, follows the file's established expression-bodied style, is compatible with every targeted framework, and introduces no public API changes. No changes required.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 175.2 AIC · ⌖ 12.5 AIC · ◷

[github-actions]

This pull request was automatically closed because it expired on 2026-06-14T14:05:42.515Z.

Closed by Workflow