Throughout 2025, the PVS-Studio team has been actively checking open-source C# projects. Over the year, we discovered plenty of defects. So, we picked the ten most interesting ones from this huge variety. We hope you find this roundup interesting and useful. Enjoy!
How did we compile the top?
There are several criteria the project code should meet to earn a place in our top list:
- it comes from an open-source project;
- the issues were detected by PVS-Studio;
- the code most likely contains errors;
- the code is interesting to check;
- each error is unique.
Since we regularly put together such lists, we’ve gathered an impressive collection of curious errors. You can read articles from previous years here:
- top 10 errors in 2024;
- top 10 errors in 2023;
- top 10 errors in 2022;
- top 10 errors in 2021;
- top 10 errors in 2020;
- top 10 errors in 2019;
Now, let’s dive into the fascinating abyss of C# errors for 2025!
P. S. The article author selected and grouped the errors based on his subjective opinion. If you think this or that bug deserves another place, feel free to leave a comment 🙂
10th place. Try to find it
Today’s top starts with an error mentioned in the article about checking .NET 9. It feels like .NET 9 was just released, but a little over a month ago, .NET 10 replaced it. We have covered the most significant changes in this article.
Let’s get back to the analysis:
public static void SetAsIConvertible(this ref ComVariant variant,
IConvertible value)
{
TypeCode tc = value.GetTypeCode();
CultureInfo ci = CultureInfo.CurrentCulture;
switch (tc)
{
case TypeCode.Empty: break;
case TypeCode.Object:
variant = ComVariant.CreateRaw(....); break;
case TypeCode.DBNull:
variant = ComVariant.Null; break;
case TypeCode.Boolean:
variant = ComVariant.Create<bool>(....)); break;
case TypeCode.Char:
variant = ComVariant.Create<ushort>(value.ToChar(ci)); break;
case TypeCode.SByte:
variant = ComVariant.Create<sbyte>(value.ToSByte(ci)); break;
case TypeCode.Byte:
variant = ComVariant.Create<byte>(value.ToByte(ci)); break;
case TypeCode.Int16:
variant = ComVariant.Create(value.ToInt16(ci)); break;
case TypeCode.UInt16:
variant = ComVariant.Create(value.ToUInt16(ci)); break;
case TypeCode.Int32:
variant = ComVariant.Create(value.ToInt32(ci)); break;
case TypeCode.UInt32:
variant = ComVariant.Create(value.ToUInt32(ci)); break;
case TypeCode.Int64:
variant = ComVariant.Create(value.ToInt64(ci)); break;
case TypeCode.UInt64:
variant = ComVariant.Create(value.ToInt64(ci)); break;
case TypeCode.Single:
variant = ComVariant.Create(value.ToSingle(ci)); break;
case TypeCode.Double:
variant = ComVariant.Create(value.ToDouble(ci)); break;
case TypeCode.Decimal:
variant = ComVariant.Create(value.ToDecimal(ci)); break;
case TypeCode.DateTime:
variant = ComVariant.Create(value.ToDateTime(ci)); break;
case TypeCode.String:
variant = ComVariant.Create(....); break;
default:
throw new NotSupportedException();
}
}
Can you see the issue? It’s definitely there!
case TypeCode.Int64:
variant = ComVariant.Create(value.ToInt64(ci)); break;
case TypeCode.UInt64:
variant = ComVariant.Create(value.ToInt64(ci)); break; // <=
PVS-Studio warning: V3139 Two or more case-branches perform the same actions. DynamicVariantExtensions.cs 68
I hope you’ve given your eyes a good workout, and your keen eyesight hasn’t let you down. In case TypeCode.UInt64 instead of value.ToInt64, developers should have used the existing ToUInt64() method. This may be a copy-paste error.
9th place. Invalid format
The ninth place goes to an error described in the article about checking the Neo and NBitcoin projects:
public override string ToString()
{
var sb = new StringBuilder();
sb.AppendFormat("{1:X04} {2,-10}{3}{4}",
Position,
OpCode,
DecodeOperand());
return sb.ToString();
}
PVS-Studio warning: V3025 [CWE-685] Incorrect format. A different number of format items is expected while calling ‘AppendFormat’ function. Format items not used: {3}, {4}. Arguments not used: 1st. VMInstruction.cs 105
Calling the overridden ToString method inevitably causes an exception. This is due to an incorrect sb.AppendFormat call containing two errors.
- The number of arguments to insert is less than the number of placeholders in the format string, which causes the exception.
- Even if we fix the first issue by matching the number of arguments and placeholders, the call will still throw the exception. This is because placeholder indexing starts at 0, not 1. This means the fifth argument is required for the placeholder with index 4, which is absent.
8th place. Navel-gazing
The next error comes from the article about testing the Lean trading engine:
public override int GetHashCode()
{
unchecked
{
var hashCode = Definition.GetHashCode();
var arr = new int[Legs.Count];
for (int i = 0; i < Legs.Count; i++)
{
arr[i] = Legs[i].GetHashCode();
}
Array.Sort(arr);
for (int i = 0; i < arr.Length; i++)
{
hashCode = (hashCode * 397) ^ arr[i];
}
return hashCode;
}
}
public override bool Equals(object obj)
{
....
return Equals((OptionStrategyDefinitionMatch) obj);
}
PVS-Studio warning: V3192 The ‘Legs’ property is used in the ‘GetHashCode’ method but is missing from the ‘Equals’ method. OptionStrategyDefinitionMatch.cs 176
The analyzer ran an interprocedural check on the Equals method, which calls an overridden Equals, and found that it doesn’t use the Legs property, even though GetHashCode relies on it.
Let’s take a closer look at the Equals method:
public bool Equals(OptionStrategyDefinitionMatch other)
{
....
var positions = other.Legs
.ToDictionary(leg => leg.Position,
leg => leg.Multiplier);
foreach (var leg in other.Legs) // <=
{
int multiplier;
if (!positions.TryGetValue(leg.Position, out multiplier))
{
return false;
}
if (leg.Multiplier != multiplier)
{
return false;
}
}
return true;
}
Note that the method iterates over other.Legs. For each element in that collection, the code tries to find it in the positions dictionary—but that dictionary also comes from other.Legs. As a result, the code checks whether elements of a collection exist in the same collection.
We can fix the code by replacing other.Legs with Legs at the marked location.
7th place. Tricky Equals
The seventh place goes to an error from an article about checking ScottPlot:
public class CoordinateRangeMutable : IEquatable<CoordinateRangeMutable>
{
....
public bool Equals(CoordinateRangeMutable? other)
{
if (other is null)
return false;
return Equals(Min, other.Min) && Equals(Min, other.Min); // <=
}
public override bool Equals(object? obj)
{
if (obj is null)
return false;
if (obj is CoordinateRangeMutable other)
return Equals(other);
return false;
}
public override int GetHashCode()
{
return Min.GetHashCode() ^ Max.GetHashCode(); // <=
}
}
PVS-Studio warnings:
V3192 The ‘Max’ property is used in the ‘GetHashCode’ method but is missing from the ‘Equals’ method. ScottPlot CoordinateRangeMutable.cs 198
V3001 There are identical sub-expressions ‘Equals(Min, other.Min)’ to the left and to the right of the ‘&&’ operator. ScottPlot CoordinateRangeMutable.cs 172
The analyzer issued two warnings for this code fragment. Let’s see why this happened.
We’ll start with the V3192. The analyzer warning says that the Max property is used in the GetHashCode method but not in the Equals method. If we look at the overridden Equals method, we can see that another Equals is called in its body. There we can see the following: Equals(Min, other.Min) && Equals(Min, other.Min). The V3001 diagnostic rule highlighted this fragment.
Clearly, one of the && operands must have the Equals(Max, other.Max) form.
So, the analyzer is right—Max doesn’t appear in the Equals method.
6th place. Bit tricks
An error that, like the previous one, is taken from the article on checking ScottPlot, concludes the first half of the top:
public static Interactivity.Key GetKey(this Keys keys)
{
Keys keyCode = keys & ~Keys.Modifiers; // <=
Interactivity.Key key = keyCode switch
{
Keys.Alt => Interactivity.StandardKeys.Alt, // <=
Keys.Menu => Interactivity.StandardKeys.Alt,
Keys.Shift => Interactivity.StandardKeys.Shift, // <=
Keys.ShiftKey => Interactivity.StandardKeys.Shift,
Keys.LShiftKey => Interactivity.StandardKeys.Shift,
Keys.RShiftKey => Interactivity.StandardKeys.Shift,
Keys.Control => Interactivity.StandardKeys.Control, // <=
Keys.ControlKey => Interactivity.StandardKeys.Control,
Keys.Down => Interactivity.StandardKeys.Down,
Keys.Up => Interactivity.StandardKeys.Up,
Keys.Left => Interactivity.StandardKeys.Left,
Keys.Right => Interactivity.StandardKeys.Right,
_ => Interactivity.StandardKeys.Unknown,
};
....
}
PVS-Studio warning: V3202 Unreachable code detected. The ‘case’ value is out of range of the match expression. ScottPlot.WinForms FormsPlotExtensions.cs 106
Multiple pattern values within switch are impossible in the current context. Let’s see what’s going on here.
First, we should look at the values that correspond to the erroneous elements of the enumeration.
[Flags]
[TypeConverter(typeof(KeysConverter))]
[Editor(....)]
public enum Keys
{
/// <summary>
/// The bit mask to extract modifiers from a key value.
/// </summary>
Modifiers = unchecked((int)0xFFFF0000),
....
/// <summary>
/// The SHIFT modifier key.
/// </summary>
Shift = 0x00010000,
/// <summary>
/// The CTRL modifier key.
/// </summary>
Control = 0x00020000,
/// <summary>
/// The ALT modifier key.
/// </summary>
Alt = 0x00040000
}
Next, let’s convert them to binary:
It’s clear now that Modifiers include each of the erroneous enumeration elements.
The value passed to switch is obtained from the keys & ~Keys.Modifiers expression. This expression excludes the Keys.Modifiers value from keys. In addition to Keys.Modifiers, however, Shift, Control, and Alt will also be excluded, since Modifiers already include these values (Modifiers have a non-zero bit for each non-zero bit of the erroneous enumeration elements).
From all this we can conclude that the bit combination that produces Shift, Control, or Alt for the keys & ~Keys.Modifiers operation doesn’t exist.
The issue may lie in the switch implementation rather than the enumeration values.
5th place. All boxed up
The top five starts with an error mentioned in an article about checking .NET 9:
struct StackValue
{
....
public override bool Equals(object obj)
{
if (Object.ReferenceEquals(this, obj))
return true;
if (!(obj is StackValue))
return false;
var value = (StackValue)obj;
return this.Kind == value.Kind
&& this.Flags == value.Flags
&& this.Type == value.Type;
}
}
PVS-Studio warning: V3161 Comparing value type variables with ‘ReferenceEquals’ is incorrect because ‘this’ will be boxed. ILImporter.StackValue.cs 164
The ReferenceEquals method takes parameters of the Object type. When a value type is passed, it gets boxed. The reference created on the heap won’t match any other reference.
Since this is passed as the first argument, boxing will occur every time the Equals method is called. So, checking via ReferenceEquals always returns false.
Note that this issue doesn’t affect how the method works. However, the check using the ReferenceEquals method was performed to avoid further comparisons if the references were equal. In other words, this is a kind of optimization. In reality, however, the situation is completely opposite:
- the code always executes after the check;
- each call to
Equalsresults in a boxing operation.
It’s funny that the analyzer built into .NET (rule CA2013) also detects this issue. However, the developers of .NET themselves couldn’t avoid it 🙂
Perhaps this rule was disabled for the project. In addition, CA2013 is enabled by default starting with .NET 5.
4th place. Unsubscribing
The fourth place goes to an error from an article about checking MSBuild:
private static void SubscribeImmutablePathsInitialized()
{
NotifyOnScopingReadiness?.Invoke();
FileClassifier.Shared.OnImmutablePathsInitialized -= () =>
NotifyOnScopingReadiness?.Invoke();
}
PVS-Studio warning: V3084. Anonymous function is used to unsubscribe from ‘OnImmutablePathsInitialized’ event. No handlers will be unsubscribed, as a separate delegate instance is created for each anonymous function declaration. CheckScopeClassifier.cs 67
In this case, unsubscribing from a delegate doesn’t take effect because each time an anonymous function is declared, a new delegate instance is created. As a result, an attempt to unsubscribe from this instance won’t take the expected effect. OnImmutablePathsInitialized is subscribed to delegate 1 but unsubscribes from delegate 2, which has no effect.
3rd place. Confusion over operator precedence
So, we’ve reached the top three. The error from an article about checking the Neo and NBitcoin projects takes the honorable third place:
public override int Size => base.Size
+ ChangeViewMessages?.Values.GetVarSize() ?? 0
+ 1 + PrepareRequestMessage?.Size ?? 0
+ PreparationHash?.Size ?? 0
+ PreparationMessages?.Values.GetVarSize() ?? 0
+ CommitMessages?.Values.GetVarSize() ?? 0;
PVS-Studio warning: V3123 [CWE-783] Perhaps the ‘??’ operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. RecoveryMessage.cs 35
The analyzer issued several V3123 warnings for this code, but I’ve included only one for brevity. The ??operator has lower precedence than the + operator. However, the formatting of this expression suggests developers expected the opposite.
Does the order of operations matter here? To answer the question, let’s look at the example of a sub-expression addition if ChangeViewMessages is null:
base.Size + ChangeViewMessages?.Values.GetVarSize() ?? 0
Enter fullscreen mode Exit fullscreen mode
Regardless of the base.Size value, the sub-expression result is always 0 because adding base.Size to null results in null.
If we place ChangeViewMessages?.Values.GetVarSize() ?? 0 in parentheses, changing the operation order, the result becomes base.Size.
2nd place. The treacherous pattern
The second place goes to an error from an article about checking the Files file manager:
protected void ChangeMode(OmnibarMode? oldMode, OmnibarMode newMode)
{
....
var modeSeparatorWidth =
itemCount is not 0 or 1
? _modesHostGrid.Children[1] is FrameworkElement frameworkElement
? frameworkElement.ActualWidth
: 0
: 0;
....
}
PVS-Studio warning: V3207 [CWE-670] The ‘not 0 or 1’ logical pattern may not work as expected. The ‘not’ pattern is matched only to the first expression from the ‘or’ pattern. Files.App.Controls Omnibar.cs 149
Let’s look closer at the itemCount is not 0 or 1 part. Already guessed what’s the issue? This pattern is redundant. Its second part affects nothing.
When saying “x is not 0 or 1”, people usually imply that x is neither 0 nor 1. However, in C#, operator precedence works differently—x is not 0 or 1 actually means x is (not 0) or 1. Such mistakes can lead not only to redundancy but also to errors like NullReferenceException: list is not null or list.Count == 0. This issue was even discussed in a meeting that explored potential solutions. Based on the discussion, either the compiler will likely catch this issue in the future, or the built-in static analysis will flag it as a warning.
1st place. How does LINQ work?
And the winner is an error from an article about checking the Lean trading engine. This error ranks first due to its subtlety. Some developers may not consider the effects of using deferred execution methods in combination with captured variables. All the details are below:
public void FutureMarginModel_MarginEntriesValid(string market)
{
....
var lineNumber = 0;
var errorMessageTemplate = $"Error encountered in file " +
$"{marginFile.Name} on line ";
var csv = File.ReadLines(marginFile.FullName)
.Where(x => !x.StartsWithInvariant("#")
&& !string.IsNullOrWhiteSpace(x))
.Skip(1)
.Select(x =>
{
lineNumber++; // <=
....
});
lineNumber = 0; // <=
foreach (var line in csv)
{
lineNumber++; // <=
....
}
}
PVS-Studio warning: V3219 The ‘lineNumber’ variable was changed after it was captured in a LINQ method with deferred execution. The original value will not be used when the method is executed. FutureMarginBuyingPowerModelTests.cs 720
The lineNumber variable is captured and incremented in the delegate that is passed to the LINQ method. Since Select is a deferred method, the delegate code runs while iterating over the resulting collection, not when Select gets called.
While iterating over the csv collection, the lineNumber variable is also incremented. As a result, each iteration increases lineNumber by 2: when the delegate runs and inside the foreach, which looks odd.
Note the lineNumber = 0 assignment before foreach. It’s likely that developers expected that this variable could hold a non-zero value before the loop. However, that is impossible: lineNumber starts at zero, and the only place that changes it before the foreach sits in the delegate. As mentioned above, the delegate runs during iteration, not before it. Apparently, the developers expected the delegate to execute prior to entering the loop.
Conclusion
That’s it! We’ve gone through the most interesting warnings from the author’s point of view 🙂
I hope you found this collection interesting and thought-provoking in terms of code development.
If you’d like to check whether your project has similar issues, now’s the time to use a static analyzer. Here’s the download link.
