--- description: "CodeWiki C# pitfalls and review checks" globs: ["**/*.cs"] alwaysApply: false --- # C# rules Apply these rules when a matching file is in context. - Do not assume this is safe: generated code often defines `[Cache]`, `[Authorize]`, or `[Validate]` without any middleware, interceptor, generator, or reflection scanner that consumes it. Source: [Attributes](https://codewiki.com/csharp/attributes/) - A serializer scans properties, but generated code writes `[field: Name]`; or a framework scans backing fields while the code decorates only the property. Why: The names look related, but the metadata entities differ. Source: [Attributes](https://codewiki.com/csharp/attributes/) - Do not assume this is safe: a model puts `DateTime.Now`, runtime configuration, or `new Service()` in an attribute constructor, assuming a square-bracket call behaves exactly like ordinary object creation. Source: [Attributes](https://codewiki.com/csharp/attributes/) - After enabling `AllowMultiple = true`, a consumer still calls `GetCustomAttribute()` and encounters `AmbiguousMatchException` only when the data grows to a second record. Source: [Attributes](https://codewiki.com/csharp/attributes/) - Do not assume this is safe: a generated scanner sends classes, overrides, properties, events, and interface implementations through the same `inherit: true` query and assumes equivalent results. Source: [Attributes](https://codewiki.com/csharp/attributes/) - Do not assume this is safe: an attribute constructor reads a file, contacts a network, or depends on mutable global state, making assembly scanning slow, fragile, or unexpectedly effectful inside a tool process. Source: [Attributes](https://codewiki.com/csharp/attributes/) - `double average = total / count;` looks as if it requests a floating-point result, but two `int` operands perform integer division first. Why: Similarly, two `int` values can overflow during multiplication before the damaged result is stored in a `long`. Source: [C# fundamentals](https://codewiki.com/csharp/fundamentals/) - `var` infers one type at compile time. Why: Generated code sometimes starts with `var result = 0;` and later assigns a `decimal` or string, as if the variable's type followed its value. Source: [C# fundamentals](https://codewiki.com/csharp/fundamentals/) - Do not assume this is safe: `isCached || RefreshCache()` can skip the call on the right because `||` short-circuits. Why: If refreshing is required work, putting it in the condition makes behavior depend on the left value and easy to change accidentally during refactoring. Source: [C# fundamentals](https://codewiki.com/csharp/fundamentals/) - The last valid array index is `Length - 1`, so `index <= items.Length` goes out of range on its final iteration. Why: Conversely, mechanically changing a closed business interval to `<` can omit its endpoint. Source: [C# fundamentals](https://codewiki.com/csharp/fundamentals/) - The `!` beside a nullable-reference warning only silences the compiler; it doesn't check or repair `null` at runtime. Why: Generated code often applies it directly to `Console.ReadLine()!`, deserialized results, or database fields. Source: [C# fundamentals](https://codewiki.com/csharp/fundamentals/) - Accepting `object`, returning `null`, or controlling behavior with several Boolean parameters lets invalid combinations pass compilation. Why: The method must then recover information the caller could have expressed through casts and hidden conventions. Source: [C# fundamentals](https://codewiki.com/csharp/fundamentals/) - Generated code often calls `ContainsKey(key)` and then reads `dictionary[key]`. Why: That performs two lookups, and if another thread or callback can mutate state between the steps, the first check doesn't guarantee the second read. Source: [Collections](https://codewiki.com/csharp/collections/) - Do not assume this is safe: if a field used by `Equals` or `GetHashCode` changes after an object enters a `Dictionary` or `HashSet`, later `Contains`, lookup, or removal can fail to locate the object that is still present. Source: [Collections](https://codewiki.com/csharp/collections/) - Structural changes to the same `List`, `Dictionary`, or `HashSet` inside `foreach` usually invalidate the enumerator and cause `InvalidOperationException`. Why: Swallowing the exception doesn't define which elements were already processed. Source: [Collections](https://codewiki.com/csharp/collections/) - Returning `IReadOnlyList` restricts only the static interface. Why: A caller may hold another reference to the source list, and the provider may keep mutating it, so the result is neither necessarily a snapshot nor safe for stable cross-thread reading. Source: [Collections](https://codewiki.com/csharp/collections/) - Current `Dictionary` and `HashSet` implementations may produce an apparently stable order, but their core contract isn't business sorting. Why: Rebuilding the collection, changing runtime, or changing comparer can alter tests and serialized output. Source: [Collections](https://codewiki.com/csharp/collections/) - `ConcurrentDictionary.GetOrAdd` is thread-safe, but its value factory can execute concurrently more than once; only one result enters the dictionary. Why: A `TryGetValue` followed by another call doesn't automatically become one atomic business operation either. Source: [Collections](https://codewiki.com/csharp/collections/) - "Value types are on the stack; reference types are on the heap" mixes implementation with semantics. Why: It can't explain struct fields inside classes, value-type arrays, captured locals, or boxed values. Source: [Data types](https://codewiki.com/csharp/data-types/) - Generated code often adds `!` beside a warning, as in `customer!.Address.City`. Why: If `customer` really is `null` at runtime, the code still throws `NullReferenceException`. Source: [Data types](https://codewiki.com/csharp/data-types/) - `decimal.Parse(text)` uses the current culture. Why: The same `"1,234"` can mean a different value or fail under another configuration, so success on a development machine doesn't prove production behavior. Source: [Data types](https://codewiki.com/csharp/data-types/) - `long total = quantity * unitCents;` can still multiply as `int` and overflow before the damaged result converts to `long`. Why: A wider destination doesn't change the types of intermediate expressions. Source: [Data types](https://codewiki.com/csharp/data-types/) - A boxed `int` can't be unboxed directly with `(long)`. Why: The cast looks plausible, but the boxed type doesn't match and the runtime throws `InvalidCastException`. Source: [Data types](https://codewiki.com/csharp/data-types/) - Reading a struct through a property, collection indexer, or `foreach` variable often produces a copy. Why: Calling a mutating method on that copy might not change the original storage, and some forms fail to compile. Source: [Data types](https://codewiki.com/csharp/data-types/) - Two custom delegates remain different named types even when their parameters and return types match exactly. Why: You can't assign their variables directly. Fix: reuse `Func`, `Action`, or `Predicate` when the API needs no domain-specific meaning. Keep custom types for genuinely different contracts, and create the target type explicitly through a fresh lambda or method group instead of hiding the difference behind a cast. Source: [Delegates and events](https://codewiki.com/csharp/delegates-events/) - `source.Changed -= (_, e) => Handle(e);` usually doesn't remove the handler created by another lambda expression earlier, even when the source text looks identical. Why: Fix: when removal is required, keep the delegate instance in a field or local, or use the same named method. Raise the notification after unsubscription and confirm that the old handler stays silent. Source: [Delegates and events](https://codewiki.com/csharp/delegates-events/) - Generated `for` loops often add lambdas to a list while every lambda captures the same `i`. Why: When called after the loop, they observe the final value of `i`, not each iteration's value. Fix: create `int index = i;` inside the loop body so the lambda captures the iteration's variable, or pass the value to a handler factory. Tests must call every handler after the loop ends. Source: [Delegates and events](https://codewiki.com/csharp/delegates-events/) - A returning multicast delegate gives its caller only the last normally completed return value. Why: If any handler throws, invocation stops and later handlers don't run. Fix: prefer `void` handlers for broadcast notifications. If the contract requires every result or an attempt at every handler, traverse `GetInvocationList()` and define ordering, failure collection, and what happens to partial side effects. Source: [Delegates and events](https://codewiki.com/csharp/delegates-events/) - An async lambda converted to `Action` or `EventHandler` becomes `async void`. Why: The caller can't await completion, and exceptions after an `await` don't return through the original call. Fix: use `Func`, `Func`, or a dedicated `Task`-returning delegate when completion matters. Don't invoke a multicast async delegate once and await only its last returned `Task`; obtain the invocation list and choose sequential awaits or concurrent `Task.WhenAll`. Source: [Delegates and events](https://codewiki.com/csharp/delegates-events/) - `source.Changed -= (_, e) => Update(e);` usually doesn't remove a handler registered with another lambda expression earlier. Why: Matching source text doesn't make the delegates equal. Source: [Event subscription and lifetime](https://codewiki.com/csharp/events/) - After a page object subscribes to a singleton or static event, the publisher can retain that page through its instance-method delegate even after the page closes. Why: The object may stay alive and continue processing stale notifications. Source: [Event subscription and lifetime](https://codewiki.com/csharp/events/) - Do not register an `async` lambda with `EventHandler` creates an `async void` handler. Why: The publisher can't await completion or observe exceptions from the asynchronous portion through an ordinary `try`/`catch`. Source: [Event subscription and lifetime](https://codewiki.com/csharp/events/) - A multicast delegate invokes handlers in order and stops when one throws. Why: Cleanup placed in "the last subscriber" can therefore leave the system in a partial state. Source: [Event subscription and lifetime](https://codewiki.com/csharp/events/) - A null-conditional call handles the current absence of subscribers, but it doesn't marshal handlers to a UI thread or make shared handler data thread-safe. Why: When removal races with raising, the snapshot already obtained for this call can still contain the newly removed handler. Source: [Event subscription and lifetime](https://codewiki.com/csharp/events/) - A publisher doesn't know which subscribers exist and can't guarantee that they succeed. Why: Depending on a handler to update core state makes behavior change when there are no subscribers, registration order changes, or a handler fails. Source: [Event subscription and lifetime](https://codewiki.com/csharp/events/) - An empty `catch (Exception)` turns programming bugs, environmental faults, and cancellation into apparent success. Why: The caller loses the failure signal, and the log has no throw site to investigate. Source: [Exceptions](https://codewiki.com/csharp/exceptions/) - Calling `Parse` for every invalid form field and catching `FormatException`, or testing dictionary absence with an indexer and `KeyNotFoundException`, disguises normal input states as exceptional paths. Source: [Exceptions](https://codewiki.com/csharp/exceptions/) - Writing `throw ex;` inside a `catch` resets the stack trace's starting point to that statement. Why: The method that found the error might still appear in partial information, but the most useful original propagation path is truncated. Source: [Exceptions](https://codewiki.com/csharp/exceptions/) - A new exception from `finally` or `Dispose` becomes the exception leaving that scope, so the exception already in flight might no longer be directly observable. Why: Handwritten code that always ignores cleanup failures goes to the other extreme and permanently loses them. Source: [Exceptions](https://codewiki.com/csharp/exceptions/) - `catch (Exception)` also catches `OperationCanceledException`. Why: Generated background jobs often log a user cancellation as an error, trigger a retry, or translate it into a generic exception that loses the original cancellation token. Source: [Exceptions](https://codewiki.com/csharp/exceptions/) - A generated helper accepts `Func`, so an `IQueryable` call selects an in-memory path, or the code calls `AsEnumerable()` before filtering. Why: The result may be correct while retrieving far more remote data than necessary. Source: [Expression trees](https://codewiki.com/csharp/expression-trees/) - The two `x` parameters in `x => x.Active` and `x => x.Total > 0` don't become bound merely because their names match. Why: Combining their bodies directly leaves the second parameter free, so lambda construction or compilation can fail. Source: [Expression trees](https://codewiki.com/csharp/expression-trees/) - `Expression.Constant(rawValue)` uses the object's current runtime type. Why: Text input doesn't automatically become `decimal`, while untyped `null` can't express every nullable target, so a binary factory can reject the operands. Source: [Expression trees](https://codewiki.com/csharp/expression-trees/) - The fact that `Compile()` can execute a method call doesn't mean a database, search service, or other provider knows how to translate that `MethodInfo`. Why: Testing only against in-memory data hides production translation failures and semantic differences. Source: [Expression trees](https://codewiki.com/csharp/expression-trees/) - Do not assume this is safe: an expression's `ToString()` is a diagnostic aid, not a stable serialization protocol. Why: It can omit object identity and captured-value details, and two similar strings don't necessarily represent reusable execution or translation results. Source: [Expression trees](https://codewiki.com/csharp/expression-trees/) - Do not assume this is safe: generated code often changes `T` to `object`, or uses `dynamic` for comparison and arithmetic, when a generic expression fails to compile. Why: The result may compile while losing input-output relationships and postponing missing-member, conversion, and operator failures until runtime. Source: [Generics](https://codewiki.com/csharp/generics/) - `default(T)` may be valid data: `0`, `false`, an all-zero struct value, and `null` can all occur in input. Why: Returning it for failure prevents the caller from distinguishing “found the default value” from “no result.” Source: [Generics](https://codewiki.com/csharp/generics/) - `where T : new()` guarantees only a public parameterless constructor. Why: It doesn't inject dependencies, supply required data, or validate the created object. Adding it only to enable `new T()` often forces domain types to expose an invalid empty state. Source: [Generics](https://codewiki.com/csharp/generics/) - A conversion from `IEnumerable` to `IEnumerable` doesn't imply a conversion from `List` to `List`. Why: If a writable collection allowed it, the receiver could insert another `Animal` that isn't a `Dog`. Source: [Generics](https://codewiki.com/csharp/generics/) - `where T : notnull` and `where T : class` mainly affect type arguments and nullable analysis. Why: They don't generate a `null` check at method entry or block every call path that escaped nullable analysis. Source: [Generics](https://codewiki.com/csharp/generics/) - More constraints don't make an API safer. Why: An unused `class`, `IComparable`, or `new()` rejects types that would otherwise work and may falsely suggest that the implementation needs those capabilities. Source: [Generics](https://codewiki.com/csharp/generics/) - `var result = source.Where(...)` usually stores only a query. Why: If the source changes before enumeration, result membership can change too. A disposed or invalid source can also delay its exception until the consumer runs. Source: [LINQ](https://codewiki.com/csharp/linq/) - Calling `Count()` and then using `foreach` on the same `IEnumerable` can perform two database requests, file reads, generator runs, or logging predicates. Why: This is multiple enumeration, and the results can also change between passes. Source: [LINQ](https://codewiki.com/csharp/linq/) - Using `FirstOrDefault` to find an account that should be unique silently chooses one when duplicate data exists. Why: Using `Single` on a query that legitimately allows several results turns ordinary input into an exception. Source: [LINQ](https://codewiki.com/csharp/linq/) - `items.OrderBy(x => x.Team).OrderBy(x => x.Score)` creates a new primary ordering by score; it doesn't order by score within each team. Why: Generated code often writes `OrderBy` mechanically for every sorting requirement. Source: [LINQ](https://codewiki.com/csharp/linq/) - Adding to or removing from a list while `foreach` consumes a query over that same list invalidates the active enumerator and normally throws `InvalidOperationException`. Why: Deferred execution makes this harder to locate when mutation is far from query definition. Source: [LINQ](https://codewiki.com/csharp/linq/) - Inserting `AsEnumerable()` to call an untranslatable local method makes subsequent filtering and ordering execute in the current process. Why: If it appears before result limits, the remote side can return far more data than the business operation needs. Source: [LINQ](https://codewiki.com/csharp/linq/) - A `switch` chooses the first successful arm in text order. Why: A broad pattern placed first changes business priority, and the compiler reports an error if it can prove that a later arm is completely unreachable. Source: [Pattern matching](https://codewiki.com/csharp/pattern-matching/) - `age is >= 18 or = 18 && age < 65` into this pattern. Source: [Pattern matching](https://codewiki.com/csharp/pattern-matching/) - A `switch` expression with no matching arm throws at runtime. Why: The compiler usually warns about a nonexhaustive expression, but list patterns do not get a complete-coverage warning, and an enum variable can hold an undefined value of its underlying type. Source: [Pattern matching](https://codewiki.com/csharp/pattern-matching/) - Do not assume this is safe: a property pattern reads members, and business logic must not depend on the order in which subpatterns run. Why: A getter that performs I/O, advances a cursor, or changes state makes the result and invocation count hard to predict; exceptions from the getter also propagate directly. Source: [Pattern matching](https://codewiki.com/csharp/pattern-matching/) - Do not assume this is safe: `IEnumerable` promises enumeration only, not the counting and indexing protocol required by a list pattern. Why: A list pattern also checks fixed positions and length; it does not search arbitrary positions like `Contains` or a LINQ query. Source: [Pattern matching](https://codewiki.com/csharp/pattern-matching/) - `_ => currentState` makes a state machine syntactically cover every input, but it silently interprets new commands, misspellings, and illegal transitions as “no change.” The catch-all lets added types bypass the review they should trigger. Source: [Pattern matching](https://codewiki.com/csharp/pattern-matching/) - A generated setter might say `set => Name = value;`, while a getter might say `get => Name;`. Why: Both invoke the same accessor again until a `StackOverflowException` occurs. Source: [Properties](https://codewiki.com/csharp/properties/) - `Report` looks like ordinary data but might query a database, read a file, or advance a counter on every read. Why: Debuggers, log templates, and serializers can read it repeatedly, causing extra work or changing the result. Source: [Properties](https://codewiki.com/csharp/properties/) - `public List Tags { get; init; }` only prevents `Tags` from pointing at another list after construction. Why: A caller can still execute `item.Tags.Add(...)`, so the object's observable state still changes. Source: [Properties](https://codewiki.com/csharp/properties/) - Do not assume this is safe: `required string Email` can be assigned `null` explicitly, usually producing a nullability warning instead of a missing-required-member error. Why: Reflection, deserializers, and code compiled with older compilers can also bypass creation-expression checking. Source: [Properties](https://codewiki.com/csharp/properties/) - Do not assume this is safe: a setter that writes a field and then discovers another failed condition can leave the object in a state the caller didn't expect. Why: Assigning several properties one by one can also violate a cross-field invariant temporarily. Source: [Properties](https://codewiki.com/csharp/properties/) - Do not assume this is safe: if `Equals` or `GetHashCode` depends on a writable property, changing it after putting the object in a `Dictionary` or `HashSet` can make the collection unable to find that object. Why: Auto-property syntax gives no hint about this identity risk. Source: [Properties](https://codewiki.com/csharp/properties/) - Positional record classes expose `init` properties, but records may declare `set` properties and may hold mutable references. Why: Positional properties on an ordinary `record struct` are settable by default. Source: [Records](https://codewiki.com/csharp/records/) - A generated `with` copy shares reference-valued members. Why: Updating a nested list, array, dictionary, or domain object through the copy can therefore change what the original observes. Source: [Records](https://codewiki.com/csharp/records/) - The record delegates member comparison to each member type. Why: Two arrays or lists with equal elements normally remain unequal when they are different collection instances, while two records sharing one mutable collection can remain equal after its contents change. Source: [Records](https://codewiki.com/csharp/records/) - A settable member that contributes to generated hashing can change after a record enters a `Dictionary` or `HashSet`. Why: Lookup may fail because the object now hashes to a different bucket. Source: [Records](https://codewiki.com/csharp/records/) - `default(MyRecordStruct)` exists even when public constructors reject zero, null, or another invalid combination. Why: Arrays and fields of that struct type can begin with the same zero-initialized state. Source: [Records](https://codewiki.com/csharp/records/) - A stored property initialized from another property can retain the original calculation after `with` changes its input. Why: The copy operation copies stored state before applying the initializer; it does not rerun arbitrary construction logic. Source: [Records](https://codewiki.com/csharp/records/) - Do not assume this is safe: `GetMethod("Run")!` assumes that the name is unique and will always exist. Why: An added overload, rename, or wrong declaring type can produce ambiguity, a null dereference, or a call to a member outside the contract. Source: [Reflection](https://codewiki.com/csharp/reflection/) - Passing only `NonPublic` or only `Instance` often finds nothing because the filter doesn't describe both visibility and member shape. Why: Adding every flag mechanically expands the set and mixes static, inherited, or private members into the result. Source: [Reflection](https://codewiki.com/csharp/reflection/) - Do not treat `GetConstructors()[0]` as the default constructor, or accepting the first scan result, hides selection policy in metadata order. Why: Declaration edits, generated code, and dependency updates can all alter the candidate set. Source: [Reflection](https://codewiki.com/csharp/reflection/) - `PropertyInfo.CanWrite` says that a setter exists; it doesn't say the setter is public or that writing preserves object invariants. Why: Generated mappers often use it to mutate state that only construction should maintain. Source: [Reflection](https://codewiki.com/csharp/reflection/) - Logging only `TargetInvocationException.Message` hides the target method's real exception type and location. Why: Treating every reflection failure as an `InnerException` makes the opposite mistake and misses argument, target, and access errors. Source: [Reflection](https://codewiki.com/csharp/reflection/) - `Type.GetType(userType)` followed by `GetMethod(userAction)` lets a caller explore reachable program types and members. Why: A method-name denylist can't cover new names, inherited members, or equivalent entry points with different side effects. Source: [Reflection](https://codewiki.com/csharp/reflection/) - A member reached only through unanalyzable name strings can be invisible to the static call graph. Why: Working in a framework-dependent development build doesn't prove that a trimmed or Native AOT publish build retains the same metadata and code. Source: [Reflection](https://codewiki.com/csharp/reflection/) - Comparing `attribute.Name.ToString() == "Generate"` recognizes only one spelling. Why: Aliases, `GenerateAttribute`, a fully qualified name, and an unrelated same-named attribute can all produce misses or false matches. Source: [Source generators](https://codewiki.com/csharp/source-generators/) - `ISymbol`, `SyntaxNode`, `Compilation`, and `Location` aren't suitable long-lived value models. Why: Small edits can make them unequal, and a symbol can keep an old compilation and its object graph reachable. Source: [Source generators](https://codewiki.com/csharp/source-generators/) - Collecting every candidate class before emitting one file per class turns a local edit into a change of the whole batch. Why: That aggregation destroys the useful granularity when each input can produce its output independently. Source: [Source generators](https://codewiki.com/csharp/source-generators/) - Generating `partial class Customer` works only for the simplest input. Why: Records, structs, nested types, generic parameters, constraints, accessibility, and the containing namespace can all require another declaration shape. Source: [Source generators](https://codewiki.com/csharp/source-generators/) - Inserting a type name, configuration value, or attribute argument directly into a source string breaks on keyword identifiers, quotes, newlines, backslashes, or Unicode boundaries. Why: If build input isn't trusted, direct concatenation also expands the syntax it can produce. Source: [Source generators](https://codewiki.com/csharp/source-generators/) - Writing the current time, a random GUID, a machine path, or unstable enumeration order into generated source makes identical inputs produce different files. Why: Build caches, snapshots, and code review all receive meaningless churn. Source: [Source generators](https://codewiki.com/csharp/source-generators/) - Do not assume this is safe: generator A assumes its initial compilation contains a type just emitted by generator B. Why: That relies on an ordinary execution order which doesn't exist. Neither IDE nor command-line hosts must run the generators that way. Source: [Source generators](https://codewiki.com/csharp/source-generators/) - Do not assume this is safe: generated or handwritten code may mutate `span[..count]` while assuming the original array is untouched. Why: Slices share storage with the original view, and overlapping slices observe each other's writes. Source: [Span and Memory](https://codewiki.com/csharp/span-memory/) - `ReadOnlySpan` and `ReadOnlyMemory` prevent assignment only through the current view. Why: The array owner, another writable alias, or the pool's next renter can still change the underlying elements. Source: [Span and Memory](https://codewiki.com/csharp/span-memory/) - Putting `Span` in a class field, capturing it in a lambda, or keeping it across `await` violates `ref struct` lifetime rules. Why: AI also gives outdated explanations when it mixes old restrictions with the relaxations added in C# 13. Source: [Span and Memory](https://codewiki.com/csharp/span-memory/) - A method may rent an array from `ArrayPool` or create an `IMemoryOwner`, return its `Memory`, then immediately return the storage in `finally` or dispose it through `using`. Why: The return type is legal, but the caller receives a region with no valid lease. Source: [Span and Memory](https://codewiki.com/csharp/span-memory/) - A `stackalloc` length taken directly from a request can exhaust the thread stack, and allocations inside a loop can accumulate until the method returns. Why: Stack data that wasn't explicitly initialized also can't be treated as zero-filled input. Source: [Span and Memory](https://codewiki.com/csharp/span-memory/)