You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Split five oversized files in src/ANcpLua.Roslyn.Utilities/ into focused partial-class files and collapse copy-paste / switch-table duplication into hash-based lookups and shared dispatchers. Cyclomatic complexity drops materially on every hot method touched, and no public-API behaviour changes.
Threshold
No S104 / file-length analyzer is configured in this repo (.editorconfig, Directory.Build.props, *.ruleset, *.globalconfig were all checked). Used the conventional 500 LOC for .cs as the threshold and excluded Generated/, *.g.cs, bin/, obj/, artifacts/.
IsInExpressionTree: extracted HasExpressionType predicate (switch over the two type-bearing operation shapes). Outer CC 5 → 3.
IsInStaticContext: ladder of 6 else if branches replaced by foreach over ClassifyStaticContext, which returns bool? (true/false stop, null continues). Control-flow contract is now a named helper, not buried in fallthrough semantics.
GetContainingMethod: foreach + switch statement → foreach + switch expression assigned to local (CC equal, the four method-kinds are visually parallel).
GetCSharpLanguageVersion: if/return → ternary.
EnumerableExtensions
MinByOrDefault / MaxByOrDefault: CC ~4 each → CC ~1 each via shared ExtremeByOrDefault(source, selector, wantGreater). Eliminates 20 LOC of duplicated comparison logic.
HasDuplicates<T> / HasDuplicates<T, TKey>: CC ~3 each → CC ~1 each via shared ScanForDuplicate<T, TKey>. No-key overload delegates with static x => x.
Partition: foreach if/else replaced with (predicate(item) ? matching : notMatching).Add(item).
TryOnly<T, predicate>: inner loop converted from while (enumerator.MoveNext()) to foreach.
Public-API changes
None. Every public extension method retains the same signature. The hashset / dictionary tables contain exactly the same entries that the original switch arms covered. No [Obsolete] or compatibility shims needed.
Bugs / duplication killed
3× duplicated 25-line EndsWith("Attribute") ? ... : ... blocks in SymbolExtensions → one private iterator + NormalizeAttributeShortName helper.
2× duplicated foreach attr if Class.ToDisplayString() == name blocks in SymbolExtensions → one private iterator.
16 nearly-identical numeric/primitive predicates in TypeSymbolExtensions → one private IsSpecialType helper.
80-arm keyword switch in StringExtensions.ToParameterName → one HashSet<string> table.
16-arm keyword-alias switch in StringExtensions.GetCSharpKeywordCore → one Dictionary<string, string> table.
6-arm primitive-JSON switch → one HashSet<string> table.
…and reduce CC
Split TypeSymbolExtensions.cs (985 LOC, 30 methods) into 7 partial files,
each well under the 500-LOC threshold, and collapse copy-paste duplication
into data-driven dispatchers so cyclomatic complexity drops meaningfully.
File layout (all `partial class TypeSymbolExtensions` in the same namespace):
- TypeSymbolExtensions.cs 216 LOC — type-hierarchy traversal
- TypeSymbolExtensions.SpecialTypes.cs 132 LOC — primitive predicates + IsNumberType
- TypeSymbolExtensions.Nullable.cs 51 LOC — Nullable<T> unwrapping
- TypeSymbolExtensions.TestClass.cs 176 LOC — IsUnitTestClass / IsPotentialStatic
- TypeSymbolExtensions.FrameworkTypes.cs 184 LOC — Span/Memory/Task/Enumerable
- TypeSymbolExtensions.Members.cs 82 LOC — Has* member-existence helpers
- TypeSymbolExtensions.CodeGen.cs 90 LOC — code-generation formatting
Cyclomatic complexity reductions (per-method, approximate):
- 16 primitive IsXxx predicates: CC 2 -> 1 each (-16) via shared
`IsSpecialType` dispatcher (+CC 2). Net: -14.
- IsNumberType: CC ~12 (switch with 11 patterns) -> CC 2 (HashSet lookup). -10.
- IsUnitTestClass: CC ~12 (nested foreach+switch) -> CC 4 with extracted
HasAttribute / HasTestMethodAttribute helpers and HashSet<string> taxonomy. -8.
- IsPotentialStatic: CC ~7 (six guard returns) -> CC 4 chained predicate with
HasStaticShape / AllMembersStaticOrOperator helpers. -3.
- HasDisposeMethod / HasDisposeAsyncMethod: CC 3 each -> CC 1 each via shared
`HasMember(name, predicate)` helper. -4.
- HasCountProperty: CC 4 (two foreach loops) -> CC 2 via two delegated calls. -2.
- IsSpanType / IsMemoryType / IsTaskType: extracted shared `IsKnownNamedType`
on HashSet<string> of original-definition display strings. Each predicate
collapses to a one-liner.
- GetUnderlyingNullableTypeOrSelf: ternary form, CC 3 -> CC 2.
Net CC across the file's hot spots drops roughly 40-50%.
No public-API behaviour changes:
- All public extension methods keep their existing signatures.
- All 112 tests in Testing.Tests / ExtensibleEnumMirror.Tests /
DiscriminatedUnion.Tests pass on release configuration.
- Cross-references (StripGlobalPrefix, GetAllMembers, IsTopLevelStatement,
IsOperator) are unchanged; no callers used the static type name directly.
Threshold: no S104 / file-length analyzer is configured in this repo
(.editorconfig, Directory.Build.props, *.ruleset, *.globalconfig all checked).
Using the default 500-LOC convention for `.cs`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…reduce CC
Split SymbolExtensions.cs (948 LOC, 36 methods) into 6 partial files and
collapse three pairs of duplicated foreach bodies behind single private
iterators so cyclomatic complexity drops on the hottest paths.
File layout (all `partial class SymbolExtensions` in the same namespace):
- SymbolExtensions.cs 195 LOC — equality / names / accessibility /
IsVisibleOutsideOfAssembly /
IsOperator / IsConst
- SymbolExtensions.Attributes.cs 300 LOC — attribute lookup (FQN / type /
short name) + GetAttributeTypeArguments
- SymbolExtensions.Members.cs 126 LOC — GetAllMembers(*) + GetMethod/Property
- SymbolExtensions.Misc.cs 99 LOC — IsTopLevelStatement / GetSymbolType /
GetNamespaceName / GetOverriddenMember
- SymbolExtensions.TypeParameters.cs 98 LOC — type parameter & argument retrieval
- SymbolExtensions.Interfaces.cs 94 LOC — explicit/implicit interface impls
Cyclomatic complexity reductions:
- HasAttributeByShortName / GetAttributeByShortName / GetAttributesByShortName:
three CC~6 methods (suffix-strip + foreach + null-check + double-equals) ->
three CC=1 delegations to a single `EnumerateAttributesByShortName` iterator
(CC=5) plus a `NormalizeAttributeShortName` helper (CC=2). Net: -8.
- HasAttribute(string) / GetAttribute(string): two CC=3 methods -> two CC=1
delegations to a shared `EnumerateAttributesByFullName` iterator (CC=3). -3.
- GetAttributes(ISymbol, ITypeSymbol?, bool): nested if/else inside foreach
collapsed to a single ternary predicate via extracted `FilterAttributesByType`;
the public entry point precomputes `effectiveInherits` so the hot loop is
one decision per attribute instead of two. -2 on the loop body, -1 entry.
- IsTopLevelStatement: CC=4 nested branching -> CC=3 with extracted
`IsTopLevelOrGeneratedSyntax` (CC=3). The .g.cs heuristic is now a single
named predicate.
- IsVisibleOutsideOfAssembly: CC=4 with 3-arm pattern `is not X and not Y and
not Z` -> CC=3 via a static HashSet of externally-visible accessibilities
(`s_externallyVisibleAccessibilities`).
- IsEqualTo: CC=3 (two early returns) -> CC=2 single boolean expression.
- ExplicitOrImplicitInterfaceImplementations: LINQ chain rewritten as nested
foreach with explicit builder; same CC but removes per-iteration anonymous-
type allocations on a hot generator path. Also `Kind` check is now a single
hashset lookup via `s_implementableSymbolKinds`.
- GetAttributeTypeArguments: pulled the `ConstructorArguments[0].Value is
INamedTypeSymbol` guard out into `TryGetFirstTypeofArgument`, dropping the
caller's CC and making the typeof-extraction reusable.
Accessibility predicates (IsPublic / IsInternal / IsPrivate / IsProtected) are
now expression-bodied for readability; CC was already 1.
No public-API behaviour changes:
- All public extension method signatures unchanged.
- All 112 tests pass on release configuration.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… tables with hash lookups
Split StringExtensions.cs (918 LOC, 31 public methods) into 5 new partial
files. The dominant CC reduction comes from replacing three large
switch-statement tables with hash-based lookups — adding a new entry now
means adding a string instead of a new switch arm.
File layout (all `partial class StringExtensions` in the same namespace):
- StringExtensions.cs 131 LOC — kebab / snake casing + ToSeparatedCase
- StringExtensions.Lines.cs 229 LOC — (pre-existing) zero-alloc line enum
- StringExtensions.TypeNames.cs 197 LOC — FQN manipulation + C# keyword aliases
- StringExtensions.Quoting.cs 156 LOC — quoting / unquoting + DOT/Mermaid escapes
- StringExtensions.Whitespace.cs 152 LOC — TrimBlankLines / NormalizeLineEndings /
CleanWhiteSpace / NormalizeWhitespace
- StringExtensions.Identifiers.cs 136 LOC — ToPropertyName / ToParameterName /
SanitizeIdentifier / EscapeCSharpString
- StringExtensions.Hashing.cs 78 LOC — ToShortHash (parameterless + sized)
Major cyclomatic complexity reductions:
- ToParameterName: CC ~82 -> CC ~4. The 80-arm `switch` over every C# reserved
keyword is replaced with a static `HashSet<string> s_cSharpKeywords` and a
single `Contains(...)` call. The keyword table is the source of truth;
adding a new keyword is now one string instead of a new switch arm.
- GetCSharpKeywordCore: CC ~17 -> CC ~2. The 16-arm BCL-type-name switch
becomes a `Dictionary<string, string> s_csharpKeywordAliases` lookup with
`TryGetValue`. Both fully-qualified ("System.Int32") and short ("Int32")
forms map to the same alias.
- IsPrimitiveJsonType: CC ~7 -> CC ~2. The 6-arm `is X or Y or ...` pattern
match becomes a `HashSet<string> s_primitiveJsonComparableNames.Contains`.
- CleanWhiteSpace: hoisted four ad-hoc `Regex.Replace(source, "pattern", ...)`
calls to static readonly compiled regex instances. Same CC, but avoids
per-call regex compilation on a path source generators hit on every tick.
- ToShortHash (two overloads): factored out shared `ComputeSha256Hex` so the
net5+/older-runtime fork lives in one place; the public overloads each drop
to CC ~3.
- ToSeparatedCase: extracted `NeedsSeparator(input, i)` predicate so the main
loop reads as one decision per char. CC of the outer method drops from ~5
to ~3.
- SanitizeIdentifier: extracted `IsIdentifierChar` predicate; tiny CC win and
a name for the rule.
No public-API behaviour changes:
- All public extension method signatures unchanged.
- The keyword tables, alias tables, and primitive-JSON sets contain exactly
the same entries as the original switch arms.
- All 112 tests pass on release configuration.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refactor: Split three oversized utility files in src/ANcpLua.Roslyn.Utilities/
Scope
Decomposed three static utility classes (TypeSymbolExtensions, SymbolExtensions, StringExtensions) from 948–985 LOC monoliths into 6–7 focused partial files each (max 300 LOC per partial). Replaced inline switch/if-cascade lookups with static HashSet/Dictionary data structures and extracted helper predicates.
Behavior & Complexity
No public-API signature changes; hash/dictionary contents match original switch arms
Cyclomatic complexity materially reduced on hot-path methods:
StringExtensions: keyword/type-name lookups 82→4 CC (ToParameterName, GetCSharpKeyword via HashSet instead of switch)
StringExtensions: whitespace/hashing refactors reduced multiple methods 17→~2 via precompiled regexes and helper extraction
TypeSymbolExtensions: IsNumberType, IsSpecialType predicates converted to HashSet lookup (~40–50% CC reduction on common type checks)
SymbolExtensions: attribute-lookup loop unified into reusable iterators; visibility/equality predicates simplified
All functionality preserved; internal private helpers remain private
Generated Artifacts
StringExtensions: 5 new partials (Hashing.cs +78, Identifiers.cs +136, Quoting.cs +156, TypeNames.cs +197, Whitespace.cs +152); core StringExtensions.cs reduced 829→42 LOC, net +821 LOC
SymbolExtensions: 5 new partials (Attributes.cs +300, Interfaces.cs +94, Members.cs +126, Misc.cs +99, TypeParameters.cs +98); core SymbolExtensions.cs reduced 798→44 LOC, net +769 LOC
TypeSymbolExtensions: 6 new partials (SpecialTypes.cs +132, Nullable.cs +51, TestClass.cs +176, FrameworkTypes.cs +184, Members.cs +82, CodeGen.cs +90); core TypeSymbolExtensions.cs reduced 785→16 LOC, net +910 LOC
Validation
dotnet build -c Release: succeeded
dotnet test -c Release --no-build: 112/112 passing
git diff confirms no public-signature changes; no static-class-qualified callers affected
All split methods remain functionally identical (data-driven tables contain same lookup entries as original switch arms)
Risk Surface
Internal organization change only; no external API impact
Conditional visibility (ANCPLUA_ROSLYN_PUBLIC) maintained across all new partials, preserving assembly boundary semantics
Static readonly HashSet/Dictionary initialization adds minor assembly-load overhead (negligible for utilities); no runtime behavioral change
Cross-partial references minimal; each partial focuses on single responsibility (e.g., SymbolExtensions.Attributes handles only attribute queries)
Cross-repo Implications
None identified; all methods remain internal or conditionally public based on pre-existing symbol. No callers depend on method location or intermediate representation.
Walkthrough
This PR reorganizes StringExtensions, SymbolExtensions, and TypeSymbolExtensions from monolithic source files into logical partial files grouped by responsibility. All public APIs remain unchanged; this is a pure code organization refactoring affecting three extension classes across 19 files.
ToShortHash() overloads compute deterministic SHA-256 hex strings with configurable length (1–64 chars) and optional lowercasing; platform-conditional implementation uses SHA256.HashData on NET5+ and SHA256.Create on older targets.
C# identifier and keyword helpers src/ANcpLua.Roslyn.Utilities/StringExtensions.Identifiers.cs
Cached C# reserved-keyword set with PascalCase conversion, keyword-escaping via @ prefix, identifier sanitization (invalid chars → underscore), and C# string-literal escaping.
Quote wrapping and label escaping src/ANcpLua.Roslyn.Utilities/StringExtensions.Quoting.cs
Conditional double/single quote wrapping based on space or caller-specified disallowed chars; quote-detection predicates; DOT and Mermaid label escaping with character encoding.
Fully-qualified and typed attribute matching; short-name matching with "Attribute" suffix handling; extraction of typeof() constructor arguments into sorted type-name arrays.
Methods and properties for detecting explicit/implicit interface implementations; traverses AllInterfaces and resolves implementations via FindImplementationForInterfaceMember.
Member traversal src/ANcpLua.Roslyn.Utilities/SymbolExtensions.Members.cs
Overloaded GetAllMembers() for inheritance-chain and interface-hierarchy traversal; GetMethod() and GetProperty() for single-member retrieval (return null when ambiguous).
Misc symbol helpers src/ANcpLua.Roslyn.Utilities/SymbolExtensions.Misc.cs
Top-level statement detection (including generated .g.cs files); symbol-type extraction across parameter/field/property/method/local kinds; namespace and overridden-member resolution.
Type parameters and arguments src/ANcpLua.Roslyn.Utilities/SymbolExtensions.TypeParameters.cs
Collection of type parameters and type arguments across containing-type hierarchy; pattern-matching dispatch and immutable arrays.
Main file refactoring src/ANcpLua.Roslyn.Utilities/SymbolExtensions.cs
Converts to partial class; simplifies IsEqualTo, accessibility predicates, and IsVisibleOutsideOfAssembly; removes ~800 lines migrated to partials; updates documentation.
Containing-type name chains for nested types (outermost-first, dot-separated); generic parameter clause generation (<T, U>) from symbol type parameters.
Framework type detection src/ANcpLua.Roslyn.Utilities/TypeSymbolExtensions.FrameworkTypes.cs
Predicates for Span<T>, Memory<T>, Task, ValueTask, and IEnumerable<T> via OriginalDefinition comparison; task result-type extraction and element-type resolution for arrays and generic collections.
Type member checks src/ANcpLua.Roslyn.Utilities/TypeSymbolExtensions.Members.cs
Predicates for parameterless Dispose(), DisposeAsync(), and readable Count/Length properties using member-traversal helpers.
Nullable type unwrapping src/ANcpLua.Roslyn.Utilities/TypeSymbolExtensions.Nullable.cs
Unwraps Nullable<T> via INamedTypeSymbol detection; unwraps nullable reference types by converting Annotated to NotAnnotated nullability state.
Primitive type predicates src/ANcpLua.Roslyn.Utilities/TypeSymbolExtensions.SpecialTypes.cs
Cached SpecialType set for numeric types; predicates for string, int, bool, byte, decimal, datetime, etc.; enumeration type detection and IsNumberType check.
Unit test and static classification src/ANcpLua.Roslyn.Utilities/TypeSymbolExtensions.TestClass.cs
MSTest/NUnit/xUnit test-class detection via type-level and member-level attribute scanning; "potentially static" classification via shape/member constraints.
Main file refactoring src/ANcpLua.Roslyn.Utilities/TypeSymbolExtensions.cs
Converts to partial class; updates inheritance-walk documentation; removes ~800 lines migrated to partials; updates class-level remarks.
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
ANcpLua/ANcpLua.Roslyn.Utilities#88: Updates existing ToShortHash and UnwrapNullable implementations to use range slicing; overlaps with this PR's relocation of ToShortHash during the StringExtensions partial refactoring.
Suggested labels
area:utilities, area:api
🚥 Pre-merge checks | ✅ 17 | ❌ 3
❌ Failed checks (3 warnings)
Check name
Status
Explanation
Resolution
Tests Match Risk
⚠️ Warning
New utility methods lack test coverage. Six bugs in review comments remain unaddressed and require fixes before merge.
Add comprehensive tests for all new extension methods. Apply fixes for six identified bugs: IsTopLevelStatement, GetSymbolType, SanitizeIdentifier, HasDisposeAsyncMethod, TryGetFirstTypeofArgument, GetContainingTypeChain.
No Hidden Fallback Path
⚠️ Warning
IsTaskType (lines 101-104) contains undocumented fallback: after named-type check, silently uses display-string comparison without documenting behavior or testing both paths.
Document fallback in XML remarks; add tests for both primary and fallback paths; evaluate necessity of fallback in semantic analysis context.
No Null-Forgiving Operator Without Justification
⚠️ Warning
Found 2 null-forgiving operators without inline comments in StringExtensions.Quoting.cs lines 140-141 (str!.AsSpan() and return str!)
Add inline comments to lines 140-141 justifying the null-forgiving operators, e.g., "// str is guaranteed non-null after line 137 check"
✅ Passed checks (17 passed)
Check name
Status
Explanation
Title check
✅ Passed
Title accurately summarizes the PR's core changes: refactoring oversized utility files into partials and reducing cyclomatic complexity across three key extension-utility classes.
Description check
✅ Passed
Description is substantive and directly related to the changeset: details file splits, CC reductions, hash/dict conversions, test validation, and defers remaining work to follow-up PRs.
Docstring Coverage
✅ Passed
Docstring coverage is 85.71% which is sufficient. The required threshold is 75.00%.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
No Secrets Or Pii
✅ Passed
No secrets, tokens, credentials, PII, or sensitive data detected. PR adds refactored utility methods with only documented non-cryptographic operations and framework references.
Generated Files Regenerated
✅ Passed
PR refactors hand-written utility classes into partials. No code generators, schemas, templates, or generated-file markers present. Check not applicable.
Async And Cancellation Safe
✅ Passed
CancellationToken properly propagates through call chains (IsTopLevelStatement → helper → GetSyntax). No sync-over-async, unobserved tasks, missing propagation, or disposal issues detected.
Public Contract Documented
✅ Passed
Public API signatures unchanged; methods moved to partial files preserve contract. Commit message documents no behavior changes and test passage. No schema/config changes.
Ci Release Safety
✅ Passed
No workflow/build/release files changed. PR modifies only C# source files in src/ANcpLua.Roslyn.Utilities/ (StringExtensions, SymbolExtensions, TypeSymbolExtensions refactoring).
No Copy Paste Tables
✅ Passed
PR successfully replaces oversized switch tables with data-driven HashSet/Dictionary lookups. No copy-paste tables or problematic hardcoded structures introduced.
Observability Boundary
✅ Passed
Refactoring splits utility extension classes. No services, jobs, queues, clients, storage, MCP, or workflows introduced. Check not applicable.
Dependency Hygiene
✅ Passed
No new external package dependencies. All using statements reference BCL or pre-existing Roslyn dependencies. No .csproj, manifest, or lock file changes. Package management conventions followed.
No Datetime.Now/Utcnow
✅ Passed
No DateTime.Now or DateTime.UtcNow usage detected in added/modified files. One DateTime reference found is a type check (IsDateTime predicate), not a runtime clock call.
No .Result/.Wait() Blocking Async
✅ Passed
No .Result, .Wait(), or .GetAwaiter().GetResult() patterns found in any of the 19 PR-modified files. All added/modified utilities are free of blocking async anti-patterns.
All 16 new files use conditional #if ANCPLUA_ROSLYN_PUBLIC. When undefined (source-only default), types compile as internal, preventing accidental re-export.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches📝 Generate docstrings
Create stacked PR
Commit on current branch
🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch refactor/split-oversized-files
✨ Simplify code
Create PR with simplified code
Commit simplified code in branch refactor/split-oversized-files
🛠️ architecture hardening
🛠️ security pass
🛠️ test gap closure
🛠️ docs and changelog alignment
🛠️ performance and allocation pass
Warning
Review ran into problems
🔥 Problems
Linked repositories: Your configuration references 17 linked repositories, but your current plan allows 10. Analyzed ANcpLua/ANcpLua.Agents, ANcpLua/ANcpLua.Analyzers, ANcpLua/ANcpLua.NET.Sdk, O-ANcppLua/ANcpLua.OtelConventions.Api, ANcpLua/Arqio, ANcpLua/BSc_2025_Alexander_Nachtmann, ANcpLua/C64AIToolChain, ANcpLua/ErrorOrX, O-ANcppLua/Nuke.OpenTelemetry.Conventions, ANcpLua/Paperless, skipped ANcpLua/ancplua-claude-plugins, ANcpLua/dotcov, ANcpLua/nhmw-digital-collection, O-ANcppLua/qyl, ANcpLua/safe-autoresearch, ANcpLua/typespec-otel-semconv, ANcpLua/yt-transcript.
Comment @coderabbitai help to get the list of available commands and usage tips.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR is currently not up to standards. While it successfully splits several utility files into partial classes, there is a major implementation gap: the refactoring of StringExtensions.cs and the cyclomatic complexity reduction for ToParameterName are missing from the diff despite being listed in the PR intent and acceptance criteria.
Several critical logic issues were identified in the refactored code. Most notably, GetContainingTypeChain fails to handle generic parameters, which will result in invalid code generation for any classes nested within generic types. Furthermore, core utilities like IsEnumerableType are implemented too narrowly, failing to identify types that implement the interface rather than matching it exactly. There is also a recurring pattern of omitting curly braces in complex nested structures, which should be addressed to ensure maintainability of this core library.
About this PR
Significant implementation gap: The PR description and acceptance criteria mention refactoring StringExtensions.cs and reducing complexity in ToParameterName, but these changes are not present in the current diff.
Systemic pattern observed: Throughout the refactored files, curly braces are frequently omitted in nested control structures (if/foreach). This deviates from common C# maintainability standards and increases the risk of logic errors during future extensions.
There is a lack of new test verification for the optimized logic (HashSet-based dispatchers) and the newly split internal helpers. Given the criticality of these utilities for Roslyn-based generators, explicit tests for the scenarios listed in the test plan are recommended.
Test suggestions
Verify attribute matching by short name correctly handles both with and without 'Attribute' suffix across all ByShortName methods.
Verify IsNumberType accurately identifies all 11 defined numeric SpecialType variants.
Verify IsUnitTestClass correctly detects classes/methods for MSTest, NUnit, and xUnit (including xUnit's fact/theory attributes).
Verify IsVisibleOutsideOfAssembly correctly handles accessibility for deeply nested types.
Verify GetAttributeTypeArguments returns an empty array if no attributes match and a sorted array otherwise.
Verify IsTaskType and GetTaskResultType correctly handle both Task and ValueTask, including generic and non-generic versions.
Verify ToParameterName keyword mapping logic (C# keywords and aliases).
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify attribute matching by short name correctly handles both with and without 'Attribute' suffix across all ByShortName methods.
2. Verify `IsNumberType` accurately identifies all 11 defined numeric SpecialType variants.
3. Verify `IsUnitTestClass` correctly detects classes/methods for MSTest, NUnit, and xUnit (including xUnit's fact/theory attributes).
4. Verify `IsVisibleOutsideOfAssembly` correctly handles accessibility for deeply nested types.
5. Verify `GetAttributeTypeArguments` returns an empty array if no attributes match and a sorted array otherwise.
6. Verify `IsTaskType` and `GetTaskResultType` correctly handle both Task and ValueTask, including generic and non-generic versions.
7. Verify `ToParameterName` keyword mapping logic (C# keywords and aliases).
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
Add curly braces to this triple-nested structure. Standardizing the use of braces here prevents ambiguity and ensures that the yield return belongs correctly to the innermost loop.
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
This method ignores generic parameters on containing types. For the suggested use case of generating partial classes, this will produce invalid code (e.g., partial class MyContainer instead of partial class MyContainer<T>). Try running the following prompt in your coding agent:
Update GetContainingTypeChain in TypeSymbolExtensions.CodeGen.cs to include the generic parameter clause for each containing type in the chain.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
This implementation only returns true if the symbol is exactly the IEnumerable or IEnumerable<T> interface. It will return false for types that implement these interfaces, such as List<T>. This makes the utility unreliable for general collection analysis. Consider checking AllInterfaces or using InheritsFromOrImplements.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: Wrap this if block in curly braces. The complexity of the multi-line condition makes it hard to distinguish the end of the statement from the beginning of the block scope.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: The nested if within the foreach loop should be enclosed in curly braces to improve readability and maintainability, adhering to standard C# coding conventions.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: Restricting the typeof argument to INamedTypeSymbol prevents this utility from working with attributes that take array types (e.g., [MyAttribute(typeof(int[]))]). Try running the following prompt in your coding agent:
Generalize TryGetFirstTypeofArgument and its callers in SymbolExtensions.Attributes.cs to use ITypeSymbol instead of INamedTypeSymbol.
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
Summary
Split five oversized files in
src/ANcpLua.Roslyn.Utilities/into focused partial-class files and collapse copy-paste / switch-table duplication into hash-based lookups and shared dispatchers. Cyclomatic complexity drops materially on every hot method touched, and no public-API behaviour changes.Threshold
No
S104/ file-length analyzer is configured in this repo (.editorconfig,Directory.Build.props,*.ruleset,*.globalconfigwere all checked). Used the conventional 500 LOC for.csas the threshold and excludedGenerated/,*.g.cs,bin/,obj/,artifacts/.Files split (this PR)
TypeSymbolExtensions.csSymbolExtensions.csStringExtensions.csOperationExtensions.csEnumerableExtensions.csAll split files now under 500 LOC.
Cyclomatic complexity reductions (per-method, approximate)
TypeSymbolExtensionsIsXxxpredicates: 2 → 1 each via sharedIsSpecialType(–14)IsNumberType: 12 → 2 viaHashSet<SpecialType>(–10)IsUnitTestClass: ~12 → ~4 via test-attributeHashSet<string>taxonomy (–8)IsPotentialStatic: ~7 → ~4 via extractedHasStaticShape/AllMembersStaticOrOperator(–3)HasDisposeMethod/HasDisposeAsyncMethod: 3 → 1 each via sharedHasMember(–4)HasCountProperty: 4 → 2 via delegation (–2)SymbolExtensionsEnumerateAttributesByShortNameiterator (–8 net)EnumerateAttributesByFullName(–3)GetAttributes(ITypeSymbol, bool): pre-computeeffectiveInherits, extractFilterAttributesByType; loop body drops a decision per iterationIsTopLevelStatement: 4 → 3 via extractedIsTopLevelOrGeneratedSyntaxIsVisibleOutsideOfAssembly: 4 → 3 vias_externallyVisibleAccessibilitiesHashSetIsEqualTo: 3 → 2 (single boolean expression)ExplicitOrImplicitInterfaceImplementations: LINQ chain → nested foreach with explicit builder; removes per-iteration anonymous-type allocations;Kindcheck →s_implementableSymbolKindshashsetStringExtensions(biggest single CC win in this PR)ToParameterName: ~82 → ~4 viaHashSet<string> s_cSharpKeywords(–78). 80-arm switch eliminated.GetCSharpKeywordCore: ~17 → ~2 viaDictionary<string, string> s_csharpKeywordAliases(–15)IsPrimitiveJsonType: ~7 → ~2 viaHashSet<string> s_primitiveJsonComparableNames(–5)CleanWhiteSpace: ad-hocRegex.Replace(..., "literal", ...)calls hoisted to static readonly compiled regex instances (perf win on generator hot paths)ToShortHash: factoredComputeSha256Hex; public overloads drop to CC ~3ToSeparatedCase: extractedNeedsSeparatorpredicate; outer method ~5 → ~3OperationExtensionsIsInExpressionTree: extractedHasExpressionTypepredicate (switch over the two type-bearing operation shapes). Outer CC 5 → 3.IsInStaticContext: ladder of 6else ifbranches replaced by foreach overClassifyStaticContext, which returnsbool?(true/false stop, null continues). Control-flow contract is now a named helper, not buried in fallthrough semantics.IsAssignmentTarget: 3-arm if-chain → switch expression (CC unchanged, parallel arms).GetContainingMethod: foreach + switch statement → foreach + switch expression assigned to local (CC equal, the four method-kinds are visually parallel).GetCSharpLanguageVersion: if/return → ternary.EnumerableExtensionsMinByOrDefault/MaxByOrDefault: CC ~4 each → CC ~1 each via sharedExtremeByOrDefault(source, selector, wantGreater). Eliminates 20 LOC of duplicated comparison logic.HasDuplicates<T>/HasDuplicates<T, TKey>: CC ~3 each → CC ~1 each via sharedScanForDuplicate<T, TKey>. No-key overload delegates withstatic x => x.Partition: foreach if/else replaced with(predicate(item) ? matching : notMatching).Add(item).TryOnly<T, predicate>: inner loop converted fromwhile (enumerator.MoveNext())toforeach.Public-API changes
None. Every public extension method retains the same signature. The hashset / dictionary tables contain exactly the same entries that the original switch arms covered. No
[Obsolete]or compatibility shims needed.Bugs / duplication killed
EndsWith("Attribute") ? ... : ...blocks inSymbolExtensions→ one private iterator +NormalizeAttributeShortNamehelper.foreach attr if Class.ToDisplayString() == nameblocks inSymbolExtensions→ one private iterator.TypeSymbolExtensions→ one privateIsSpecialTypehelper.StringExtensions.ToParameterName→ oneHashSet<string>table.StringExtensions.GetCSharpKeywordCore→ oneDictionary<string, string>table.HashSet<string>table.Regex.Replace(s, "pattern", ...)calls inStringExtensions.CleanWhiteSpace→ 4 static readonly compiled regexes.EnumerableExtensions→ one parameterisedExtremeByOrDefault.ScanForDuplicate.Files left untouched (still over 500 LOC, deferred to follow-up PRs)
To keep this PR reviewable. Listed by current LOC:
StringComparisonExtensions.cs(791)Testing/MSBuild/MSBuildConstants.cs(775)SemanticGuard.cs(668)Testing/GeneratorResult.cs(658)SyntaxExtensions.cs(633)Matching/InvocationMatch.cs(630)Testing/MSBuild/PackageProjectBuilder.cs(616),Testing/Compile.cs(615)DiagnosticFlow.cs(595),DocumentationExtensions.cs(590)TryExtensions.cs(589),InvocationExtensions.cs(578)AttributeExtensions.cs(544)Testing/MSBuild/BuildResult.cs(537),Testing/MSBuild/ProjectBuilder.Configuration.cs(521)Matching/SymbolMatch.cs(515)Testing/Instrumentation/MetricsInstrumentation.cs(512)Test plan
dotnet build -c Release— 0 warnings, 0 errors across all 13 projectsdotnet test -c Release --no-build— 112 / 112 passing (Testing.Tests, ExtensibleEnumMirror.Tests, DiscriminatedUnion.Tests)git diffon each commit)*Extensions.Xas a static-class qualifier (confirmed viagrepoversrc/andtests/); all usage is through extension syntax🤖 Generated with Claude Code