﻿# A spy undercover: PVS\-Studio to check ILSpy source code

In PVS\-Studio, we often check various compilers' code and post the results in our blog\. Decompiler programs, however, seem to be a bit neglected\. To restore justice in this world, we analyzed the ILSpy decompiler's source code\. Let's take a look at the peculiar things PVS\-Studio found\.

![0794_ILSpy/image1.png](https://import.viva64.com/docx/blog/0794_ILSpy/image1.png)

## Introduction

Probably almost every programmer used a decompiler at least once\. The reasons could vary: to see how a method is implemented, to check if there is an error inside a library used, or to satisfy curiosity and look up some source code\. At the mention of a decompiler, most \.NET programmers will think of dotPeek or ILSpy\. \.NET Reflector is not as popular anymore\. I remember when I first learned about these utilities and decompiled someone else's library \- a thought of espionage ran through my head\. I was obviously not the only one thinking along these lines \- I am sure ILSpy's name is not accidental\. I was curious what the spy is made of and wanted to reinforce it with a static analyzer\. So I used the PVS\-Studio analyzer on ILSpy's source code and put together an article based on the most interesting and suspicious code fragments I found\.

To be honest, this article on ILSpy just sort of happened\. Some of our clients are game development studios\. This is one of the reasons why we try to make our tool as helpful and handy as possible for game developers, especially for those who employ Unity and Unreal Engine\.

While I know many clients who work with Unreal Engine, I don't encounter that many Unity developers who use our analyzer\. I want to encourage them to try the PVS\-Studio analyzer, because I believe the Unity community can benefit from it\. A cool way to demonstrate it would be to analyze a Unity\-based open\-source game and present the results\. But the problem is \- I could not find such a game\! So please [let me know of any ideas](https://github.com/viva64/pvs-studio-check-list) you have for such games I could analyze with PVS\-Studio\. When I did try to look for a Unity\-based open\-source game, my search yielded unexpected results\. On one [website](https://awesomeopensource.com/projects/unity), I found a list of Unity projects that for some mysterious reason included ILSpy\. In PVS\-Studio, we use a pool of projects to test our C\# analyzer\. That group includes ILSpy, so it's odd that we do not have an article on this project yet\. But since I failed to find a Unity project for analysis, let's take a look at ILSpy\.

Here's the project's description on [GitHub](https://github.com/icsharpcode/ILSpy): ILSpy is the open\-source \.NET assembly browser and decompiler\.

Since there was no information on whether ILSpy's developers use a static analyzer, I am going to assume PVS\-Studio is the first one\. This makes my tests and research even more interesting\. Now, without further discussion, let's move on to analysis results\.

## Replacement that did not work

[V3038](https://pvs-studio.com/en/docs/warnings/v3038/) The '"'"' argument was passed to 'Replace' method several times\. It is possible that other argument should be passed instead\. ICSharpCode\.Decompiler ReflectionDisassembler\.cs 772

```cpp
private static void WriteSimpleValue(ITextOutput output,
                                     object value, string typeName)
{
  switch (typeName)
  {
    case "string":
      output.Write(  "'"
                   + DisassemblerHelpers
                      .EscapeString(value.ToString())
                      .Replace("'", "\'")                   // <=
                   + "'");
      break;
    case "type":
    ....
  }
  ....
}
```

The author seems to be replacing all single quote character occurrences with a string consisting of two characters: a backslash and a single quote character\. However, the developer missed a beat and by accident replaced the "'" with itself, thus performing a meaningless operation\. There is no difference between assigning a string variable a value of "'" or "\\'" \- either way the string is initialized with a single quote character\. To include "\\'" in a string, use escape characters:  "\\\\'" or @"\\'"\. Thus, one can change the _Replace_ method call in the following way:

```cpp
Replace("'", @"\'")
```

## Truth and nothing but the truth

**Warning 1**

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'negatedOp \=\= BinaryOperatorType\.Any' is always true\. ICSharpCode\.Decompiler CSharpUtil\.cs 79

```cpp
static Expression InvertConditionInternal(Expression condition)
{
  var bOp = (BinaryOperatorExpression)condition;

  if (   (bOp.Operator == BinaryOperatorType.ConditionalAnd)
      || (bOp.Operator == BinaryOperatorType.ConditionalOr))
  {
    ....
  }
  else if (   (bOp.Operator == BinaryOperatorType.Equality)
           || (bOp.Operator == BinaryOperatorType.InEquality) 
           || (bOp.Operator == BinaryOperatorType.GreaterThan)
           || (bOp.Operator == BinaryOperatorType.GreaterThanOrEqual)
           || (bOp.Operator == BinaryOperatorType.LessThan) 
           || (bOp.Operator == BinaryOperatorType.LessThanOrEqual))
  {
    ....
  }
  else
  {
    var negatedOp = NegateRelationalOperator(bOp.Operator);
    if (negatedOp == BinaryOperatorType.Any)                  // <=
      return new UnaryOperatorExpression(....);
    bOp = (BinaryOperatorExpression)bOp.Clone();
    bOp.Operator = negatedOp;
    return bOp;
  }
}
```

The analyzer warns that the _negatedOp_ variable always equals to the value of _Any_ from the _BinaryOperatorType_ enumeration\. To verify this, let us take a look at the _NegateRelationalOperator_ method code that provides a value for the _negatedOp_ variable\.

```cpp
public static BinaryOperatorType NegateRelationalOperator(BinaryOperatorType op)
{
  switch (op)
  {
    case BinaryOperatorType.GreaterThan:
      return BinaryOperatorType.LessThanOrEqual;
    case BinaryOperatorType.GreaterThanOrEqual:
      return BinaryOperatorType.LessThan;
    case BinaryOperatorType.Equality:
      return BinaryOperatorType.InEquality;
    case BinaryOperatorType.InEquality:
      return BinaryOperatorType.Equality;
    case BinaryOperatorType.LessThan:
      return BinaryOperatorType.GreaterThanOrEqual;
    case BinaryOperatorType.LessThanOrEqual:
      return BinaryOperatorType.GreaterThan;
    case BinaryOperatorType.ConditionalOr:
      return BinaryOperatorType.ConditionalAnd;
    case BinaryOperatorType.ConditionalAnd:
      return BinaryOperatorType.ConditionalOr;
  }
  return BinaryOperatorType.Any;
}
```

If by the _NegateRelationalOperator_ method call, the _bOp\.Operator_'s value does not match any of the _case_ labels, the method returns _BinaryOperatorType\.Any_\. You can see that the _NegateRelationalOperator_ method is called only when _if_ and _if else_ statements above the method are evaluated to _false_\. Moreover, if you look closely, you can notice that the _if_ and _if_ _else_ statements cover all _case_ labels the _NegateRelationalOperator_ method contains\. By the time the _NegateRelationalOperator_ method is called, the _bOp\.Operator_ does not satisfy any of the _case_ labels and the method returns the _BinaryOperatorType\.Any_ value\. As a result, _negatedOp \=\= BinaryOperatorType\.Any_ always evaluates to _true_, and the next line returns the value from the method\. In addition, we get unreachable code:



```cpp
bOp = (BinaryOperatorExpression)bOp.Clone();
bOp.Operator = negatedOp;
return bOp;
```

By the way, the analyzer kindly issued a warning for this as well: [V3142](https://pvs-studio.com/en/docs/warnings/v3142/) Unreachable code detected\. It is possible that an error is present\. ICSharpCode\.Decompiler CSharpUtil\.cs 81

**Warning 2**

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'pt \!\= null' is always true\. ICSharpCode\.Decompiler FunctionPointerType\.cs 168

```cpp
public override IType VisitChildren(TypeVisitor visitor)
{
  ....
  IType[] pt = (r != ReturnType) ? new IType[ParameterTypes.Length] : null;
  ....
  if (pt == null)
    return this;
  else
    return new FunctionPointerType(
      module, CallingConvention, CustomCallingConventions,
      r, ReturnIsRefReadOnly,
      pt != null ? pt.ToImmutableArray() : ParameterTypes,    // <=
      ParameterReferenceKinds);
}
```

Here everything is straightforward \- the _else_ branch is executed if the _pt_ variable is not _null_\. So I don't see the need in a ternary operator that checks the _pt_ variable for _null_\. I suspect that in the past the code did not contain the _if_\-_else_ statement and the first _return_ operator \- then this check would have made sense\. Right now it's a good idea to remove the extra ternary operator: 

```cpp
public override IType VisitChildren(TypeVisitor visitor)
{
  ....
  IType[] pt = (r != ReturnType) ? new IType[ParameterTypes.Length] : null;
  ....
  if (pt == null)
    return this;
  else
    return new FunctionPointerType(
      module, CallingConvention, CustomCallingConventions,
      r, ReturnIsRefReadOnly,
      pt.ToImmutableArray(), ParameterReferenceKinds);
}
```

**Warning 3**

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'settings\.LoadInMemory' is always true\. ICSharpCode\.Decompiler CSharpDecompiler\.cs 394

```cpp
static PEFile LoadPEFile(string fileName, DecompilerSettings settings)
{
  settings.LoadInMemory = true;
  return new PEFile(
    fileName,
    new FileStream(fileName, FileMode.Open, FileAccess.Read),
    streamOptions: settings.LoadInMemory ?                           // <=
      PEStreamOptions.PrefetchEntireImage : PEStreamOptions.Default,
    metadataOptions: settings.ApplyWindowsRuntimeProjections ? 
        MetadataReaderOptions.ApplyWindowsRuntimeProjections :
        MetadataReaderOptions.None
  );
}
```

This case is similar to the previous one \- we get an unnecessary ternary operator\. The _settings\.LoadInMemory_ property is set to _true_ and this value does not change until the ternary operator checks the value\. Here's the code for the property's getter and setter:

```cpp
public bool LoadInMemory {
  get { return loadInMemory; }
  set {
      if (loadInMemory != value)
      {
        loadInMemory = value;
        OnPropertyChanged();
      }
  }
}
```

It's easy to exclude the unnecessary ternary operator and fix this code\. There is probably no need to provide it here\.

**Warning 4**

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'ta' is always not null\. The operator '??' is excessive\. ICSharpCode\.Decompiler ParameterizedType\.cs 354

```cpp
public IType VisitChildren(TypeVisitor visitor)
{
  ....
  if (ta == null)
      return this;
  else
      return new ParameterizedType(g, ta ?? typeArguments);     // <=
}
```

We can see the unnecessary _null_ _coalescing_ operator right away\. When the _ta_ variable gets to the _else_ branch, it always has a value that is not _null_\. Consequently, the ?? operator is excessive\.

I got a total of 31 warnings under the number of [V3022](https://pvs-studio.com/en/docs/warnings/v3022/)\.

## You don't belong here

**Warning 1**

[V3025](https://pvs-studio.com/en/docs/warnings/v3025/) Incorrect format\. A different number of format items is expected while calling 'Format' function\. Arguments not used: End\. ICSharpCode\.Decompiler Interval\.cs 269

```cpp
public override string ToString()
{
  if (End == long.MinValue)
  {
    if (Start == long.MinValue)
      return string.Format("[long.MinValue..long.MaxValue]", End); // <=
    else
      return string.Format("[{0}..long.MaxValue]", Start);
  }
  else if (Start == long.MinValue)
  {
    return string.Format("[long.MinValue..{0})", End);
  }
  else
  {
    return string.Format("[{0}..{1})", Start, End);
  }
}
```

In the first _string\.Format_ method call, the format string does not match the arguments the method receives\. The _End_ variable's value, passed as an argument, cannot be inserted into the format string, because the string lacks the \{0\} format element\. Following the method's logic, this is not an error and the _return_ operator returns the string the code authors intended\. This, of course, does not cancel the fact, that the code includes a useless _string\.Format_ method call with an unused argument\. It's a good idea to fix this to make the code clean and easy to read\.

**Warning 2**

[V3025](https://pvs-studio.com/en/docs/warnings/v3025/) Incorrect format\. A different number of format items is expected while calling 'AppendFormat' function\. Arguments not used: angle\. ILSpy\.BamlDecompiler XamlPathDeserializer\.cs 177

```cpp
public static string Deserialize(BinaryReader reader)
{
  ....
  var sb = new StringBuilder();
  ....
  sb.AppendFormat(CultureInfo.InvariantCulture,
                  "A{0} {2:R} {2} {3} {4}",
                  size, angle, largeArc ? '1' : '0',
                  sweepDirection ? '1' : '0', pt1);
  ....
}
```

In this case the _angle_ variable was left out\. Though the developer passed the variable to the _AppendFormat_ method, the variable remains unused, because the format string contains two of \{2\} format elements and lacks the \{1\} format element\. The authors probably intended to produce the following string:_"A\{0\} \{1:R\} \{2\} \{3\} \{4\}"_\.

## Double standards

**Warning 1**

[V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'roslynProject' object was used before it was verified against null\. Check lines: 96, 97\. ILSpy\.AddIn OpenILSpyCommand\.cs 96

```cpp
protected Dictionary<string, DetectedReference> GetReferences(....)
{
  ....
  var roslynProject =  owner.Workspace
                            .CurrentSolution
                            .GetProject(projectReference.ProjectId);
  var project = FindProject(owner.DTE.Solution
                                 .Projects.OfType<EnvDTE.Project>(),
                            roslynProject.FilePath);              // <=

  if (roslynProject != null && project != null)                   // <=
  ....
}
```

First we get a _roslynProject_ object's _FilePath_ property with no worry that the _roslynProject_ value may be _null_, and in the next line we check _roslynProject_ for _null_\. Such code does not look safe and may produce a _NullReferenceException_ exception\. To fix this code, one can use the _FilePath_ property along with a null\-conditional operator\. The second step is to plan for the _FindProject_ method to potentially get a _null_ value as the last parameter\.

**Warning 2**

[V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'listBox' object was used before it was verified against null\. Check lines: 46, 52\. ILSpy FlagsFilterControl\.xaml\.cs 46

```cpp
public override void OnApplyTemplate()
{
  base.OnApplyTemplate();

  listBox = Template.FindName("ListBox", this) as ListBox;
  listBox.ItemsSource = FlagGroup.GetFlags(....);         // <=

  var filter = Filter;

  if (filter == null || filter.Mask == -1)
  {
    listBox?.SelectAll();                                 // <=
  }
}
```

This case is similar to the previous example\. First, we assign a value to the _ItemsSource_ property and do not check whether the _listBox_ variable contains _null_\. Then, several lines later, I can see the _listBox_ variable with the null\-conditional operator\. Note that between these two calls the _listBox_ variable did not get a new value\.

Our analyzer displayed 10 warnings with number [V3095](https://pvs-studio.com/en/docs/warnings/v3095/)\. Here is a list of those warnings:

* V3095 The 'pV' object was used before it was verified against null\. Check lines: 761, 765\. ICSharpCode\.Decompiler TypeInference\.cs 761
* V3095 The 'pU' object was used before it was verified against null\. Check lines: 882, 886\. ICSharpCode\.Decompiler TypeInference\.cs 882
* V3095 The 'finalStore' object was used before it was verified against null\. Check lines: 261, 262\. ICSharpCode\.Decompiler TransformArrayInitializers\.cs 261
* V3095 The 'definitionDeclaringType' object was used before it was verified against null\. Check lines: 93, 104\. ICSharpCode\.Decompiler SpecializedMember\.cs 93
* V3095 The 'TypeNamespace' object was used before it was verified against null\. Check lines: 84, 88\. ILSpy\.BamlDecompiler XamlType\.cs 84
* V3095 The 'property\.Getter' object was used before it was verified against null\. Check lines: 1676, 1684\. ICSharpCode\.Decompiler CSharpDecompiler\.cs 1676
* V3095 The 'ev\.AddAccessor' object was used before it was verified against null\. Check lines: 1709, 1717\. ICSharpCode\.Decompiler CSharpDecompiler\.cs 1709
* V3095 The 'targetType' object was used before it was verified against null\. Check lines: 1614, 1657\. ICSharpCode\.Decompiler CallBuilder\.cs 1614

By the way, if you want to check your own project with the PVS\-Studio analyzer or recheck ILSpy to see all warnings by yourself, you can [try the analyzer](https://pvs-studio.com/en/pvs-studio/download/)\. On the PVS\-Studio website, you can both download the analyzer and request a trial license\.

## All roads lead to one place

**Warning 1**

[V3139](https://pvs-studio.com/en/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. ILSpy Images\.cs 251

```cpp
protected override ImageSource GetBaseImage(MemberIcon icon)
{
  ImageSource baseImage;
  switch (icon)
  {
    case MemberIcon.Field:
      baseImage = Images.Field;
      break;
    case MemberIcon.FieldReadOnly:
      baseImage = Images.FieldReadOnly;
      break;
    case MemberIcon.Literal:
      baseImage = Images.Literal;             // <=
      break;
    case MemberIcon.EnumValue:
      baseImage = Images.Literal;             // <=
      break;
    case MemberIcon.Property:
      baseImage = Images.Property;
      break;
    case MemberIcon.Indexer:
      baseImage = Images.Indexer;
      break;
    case MemberIcon.Method:
      baseImage = Images.Method;
      break;
    case MemberIcon.Constructor:
      baseImage = Images.Constructor;
      break;
    case MemberIcon.VirtualMethod:
      baseImage = Images.VirtualMethod;
      break;
    case MemberIcon.Operator:
      baseImage = Images.Operator;
      break;
    case MemberIcon.ExtensionMethod:
      baseImage = Images.ExtensionMethod;
      break;
    case MemberIcon.PInvokeMethod:
      baseImage = Images.PInvokeMethod;
      break;
    case MemberIcon.Event:
      baseImage = Images.Event;
      break;
    default:
      throw new ArgumentOutOfRangeException(nameof(icon), 
                 $"MemberIcon.{icon} is not supported!");
  }

  return baseImage;
}
```

As I see it, this is clearly a mistake\. If the _icon_ variable's value equals _MemberIcon\.EnumValue_, then the _baseImage_ variable in the _case_ branch must get the value of _Images\.EnumValue_\. This is a good example of an error that a static analyzer easily detects and a human eye easily misses when looking through code\.

**Warning 2**

[V3139](https://pvs-studio.com/en/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. ICSharpCode\.Decompiler CSharpConversions\.cs 829

```cpp
bool ImplicitConstantExpressionConversion(ResolveResult rr, IType toType)
{
  ....
  switch (toTypeCode)
  {
    case TypeCode.SByte:
      return val >= SByte.MinValue && val <= SByte.MaxValue;
    case TypeCode.Byte:
      return val >= Byte.MinValue && val <= Byte.MaxValue;
    case TypeCode.Int16:
      return val >= Int16.MinValue && val <= Int16.MaxValue;
    case TypeCode.UInt16:
      return val >= UInt16.MinValue && val <= UInt16.MaxValue;
    case TypeCode.UInt32:
      return val >= 0;                 // <=
    case TypeCode.UInt64:
      return val >= 0;                 // <=
  }
  ....
}
```

I won't claim that the analyzer found here an obvious mistake, but the warning definitely makes sense\. If the _case_ labels for the _TypeCode\.UInt32_ and _TypeCode\.UInt64_ perform the same set of actions, why not write shorter code:

```cpp
bool ImplicitConstantExpressionConversion(ResolveResult rr, IType toType)
{
  switch (toTypeCode)
  {
      ....
      case TypeCode.UInt32:
      case TypeCode.UInt64:
        return val >= 0;
  }
  ....
}
```

The analyzer issued 2 more warnings with the number [V3139](https://pvs-studio.com/en/docs/warnings/v3139/):

* V3139 Two or more case\-branches perform the same actions\. ICSharpCode\.Decompiler EscapeInvalidIdentifiers\.cs 85
* V3139 Two or more case\-branches perform the same actions\. ICSharpCode\.Decompiler TransformExpressionTrees\.cs 370

## Safety comes first

[V3083](https://pvs-studio.com/en/docs/warnings/v3083/) Unsafe invocation of event, NullReferenceException is possible\. Consider assigning event to a local variable before invoking it\. ILSpy MainWindow\.xaml\.cs 787class ResXResourceWriter : IDisposable

```cpp
void assemblyList_Assemblies_CollectionChanged(....)
{
  ....
  if (CurrentAssemblyListChanged != null)
    CurrentAssemblyListChanged(this, e);      // <=
}
```

This way to raise events is fairly common, but the fact that we see this pattern in many projects is not an excuse to use it\. Of course, this is not a critical error, but, as the analyzer's warning says \- this event invocation is not safe and a _NullReferenceException_ exception is possible\. If all handlers unsubscribe from the event after _CurrentAssemblyListChanged _is checked for _null_ and before the event is raised \(for example, in a different thread\), then a _NullReferenceException_ exception is thrown\. One can fix this and write the following safe code instead:

```cpp
void assemblyList_Assemblies_CollectionChanged(....)
{
  ....
  CurrentAssemblyListChanged?.Invoke(this, e);
}
```

PVS\-Studio found 8 more similar cases, they can all be fixed with the approach above\.

## Confident uncertainty

[V3146](https://pvs-studio.com/en/docs/warnings/v3146/) Possible null dereference\. The 'FirstOrDefault' can return default null value\. ILSpy\.BamlDecompiler BamlResourceEntryNode\.cs 76

```cpp
bool LoadBaml(AvalonEditTextOutput output, CancellationToken cancellationToken)
{
  var asm = this.Ancestors().OfType<AssemblyTreeNode>()
                            .FirstOrDefault().LoadedAssembly;       // <=
  ....
  return true;
}
```

Here the developer calls the _FirstOrDefault_ method to get the first available _AssemblyTreeNode_ type element from the collection the _OfType<AssemblyTreeNode\>_ method returns\. If the collection is empty or does not contain any elements that meet the search criteria, the _FirstOrDefault_ method returns the default value \- in our case it's _null_\. A further attempt to access the the _LoadedAssembly_ property means using a null reference and yields a _NullReferenceException_ exception\. To avoid this situation, it's a good idea to use a null\-conditional operator:

```cpp
bool LoadBaml(AvalonEditTextOutput output, CancellationToken cancellationToken)
{
  var asm = this.Ancestors().OfType<AssemblyTreeNode>()
                            .FirstOrDefault()?.LoadedAssembly;     // <=
  ....
  return true;
}
```

We can assume the developer intended for the _FirstOrDefault_ method to never return _null_ in this particular place\. If this is really the case, then it's a good idea to call the _First_ method instead of _FirstOrDefault_, because it is a way to stress the developer's assurance that the method is always able to retrieve the required element from the collection\. Moreover, if the element is not found in the collection, the developer gets the _InvalidOperationException_ exception, which displays the following message: "Sequence contains no elements"\. This is more informative than a _NullReferenceException_ exception that is thrown after the code refers to a null value the _FirstOrDefault_ method returns\.

## Unsafe scanning

[V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The 'm' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. ILSpy MethodVirtualUsedByAnalyzer\.cs 137

```cpp
static bool ScanMethodBody(IMethod analyzedMethod, 
                           IMethod method, MethodBodyBlock methodBody)
{
  ....
  var mainModule = (MetadataModule)method.ParentModule;
  ....
  switch (member.Kind)
  {
    case HandleKind.MethodDefinition:
    case HandleKind.MethodSpecification:
    case HandleKind.MemberReference:
      var m = (mainModule.ResolveEntity(member, genericContext) as IMember)
              ?.MemberDefinition;
      if (   m.MetadataToken == analyzedMethod.MetadataToken               // <=
          && m.ParentModule.PEFile == analyzedMethod.ParentModule.PEFile)  // <=
      {
        return true;
      }
      break;
  }
  ....
}
```

In the code above, the developers used the null conditional operator to initialize the _m_ variable\. They anticipated that _m_ could be assigned a _null_ value\. Interestingly, in the next line the developers get the _m_ variable's properties and do not use the null conditional operator\. This may lead to _NullReferenceException_ type exceptions\. As in some other examples we've reviewed so far, let's fix the problem by introducing the null\-conditional operator:

```cpp
static bool ScanMethodBody(IMethod analyzedMethod, 
                           IMethod method, MethodBodyBlock methodBody)
{
  ....
  var mainModule = (MetadataModule)method.ParentModule;
  ....
  switch (member.Kind)
  {
    case HandleKind.MethodDefinition:
    case HandleKind.MethodSpecification:
    case HandleKind.MemberReference:
      var m = (mainModule.ResolveEntity(member, genericContext) as IMember)
              ?.MemberDefinition;
      if (   m?.MetadataToken == analyzedMethod.MetadataToken
          && m?.ParentModule.PEFile == analyzedMethod.ParentModule.PEFile)
      {
        return true;
      }
      break;
  }
  ....
}
```

## Good old friends

[V3070](https://pvs-studio.com/en/docs/warnings/v3070/) Uninitialized variable 'schema' is used when initializing the 'ResourceSchema' variable\. ICSharpCode\.Decompiler ResXResourceWriter\.cs 63

```cpp
class ResXResourceWriter : IDisposable
{
  ....
  public static readonly string ResourceSchema = schema;
  ....
  static string schema = ....;
  ....
}
```

At first I did not plan to list this warning, because about five years ago we found an identical error in the [Mono](https://pvs-studio.com/en/blog/posts/csharp/0431/) project\. But then I talked to a colleague and we decided the error is worth mentioning\. As the article dedicated to analyzing Mono describes, by the time the _schema_ static field initializes the _ResourceSchema_ static field, the _schema_ static field has not been initialized yet and evaluates to its default value \- _null_\. The ResXResourceWriter\.cs file where we found the error, was kindly borrowed with copyright preservation from the Mono project\. Then developers expanded the file with unique features for the ILSpy project\. This is how bugs from projects spread across the internet and migrate from one project to another\. By the way, the original developers have not yet fixed the bug in the original file\.

## Conclusion

Ultimately, the ILSpy decompiler's code analysis demonstrated that the project would benefit from a static code analyzer\. Some code fragments we described are not errors, but refactoring them will clean up the code\. Other code snippets are clearly incorrect\. It is obvious that the authors expect a different result \- for example the _Replace_ method's behavior that has the same arguments\. Regular use of static analysis allows developers to find and fix incorrect, ambiguous or excessive code\. It is always quicker and cheaper to fix a bug at the stage of writing or testing code, than after the product is released with a bug and the users come and tell you "Hey, there's a bug here" \- and you're lucky if they use these words\. It's always better if the static analyzer tells you this\. Thank you for reading\.

## A note for those looking to test ILSpy on their own

When analyzing the ILSpy project, we found a few problems related to the analyzer itself \- yes, things like this happen\. We fixed the issues, but the changes were not included in the 7\.11 release\. They will be available in the next version\. Also note that ILSpy is compiled slightly differently from what most developers are used to\. This peculiarity requires additional analyzer settings\. So if you want to check ILSpy by yourself \- [let us know](https://pvs-studio.com/en/about-feedback/)\. We will provide you with the analyzer's beta and explain how to set up the analysis\.