Any bugs in your IDE? Checking AvalonStudio with PVS-Studio
Tags: csharp, dotnet, opensource, testing
Published: 2022-07-27 08:00:44

Developers around the world use their favorite IDEs every day to create software\. Today we are checking one of them and looking at the most interesting errors found\.

![Image description](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/4jz6vuxnpbzjeebzt7sd.png)

## Introduction

The modern developer uses many tools to succeed\. These include IDEs \(Integrated development environment\)\. They contain a whole set of tools to make life easier\. The modern development environment includes a text editor, compiler or interpreter, debugger, and so on\.

In this article I am checking one of open\-source IDEs\. I am using the PVS\-Studio static code analyzer to check the project\. This analysis solution can search for bugs and potential vulnerabilities in C, C\+\+, C\#, and Java code\.

Now let me introduce the project we are going to analyze\.

AvalonStudio is a cross\-platform IDE written in C\#\. This environment is primarily focused on development for Embedded C, C\+\+, \.NET Core, Avalonia, and TypeScript\. AvalonStudio is based on [Avalonia UI](https://avaloniaui.net/) which we [checked](https://pvs-studio.com/en/blog/posts/csharp/0701/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) earlier\. 

![Image description](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/1wr0m5h1713d5p6wdgjh.png)

It wasn't hard to check the source code of AvalonStudio, as it is available [on GitHub](https://github.com/VitalElement/AvalonStudio/tree/2ee2adbf1772c54531f0d5b0bc96a4789d6b826f)\. I have picked the most interesting errors the analyzer found :\)\. Enjoy reading\!

## Checking

In this article, each issue is presented with an analyzer's warning, a code fragment, a description, and a dash of the author's opinion\. If there are any points in the article that you disagree with, I would love to discuss it in the comments\.

**Issue 1**

```cpp
private async void Timer_Tick(object sender, EventArgs e)
{
  ....  
  if (AssociatedObject.IsPointerOver)
  {
    var mouseDevice = (popup.GetVisualRoot() as IInputRoot)?.MouseDevice;
    lastPoint = mouseDevice.GetPosition(AssociatedObject);
    popup.Open();
  }
}
```

[V3105](https://pvs-studio.com/en/docs/warnings/v3105/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) The 'mouseDevice' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. TooltipBehavior\.cs 173

The analyzer noticed something suspicious in the conversion through the *as* operator\. The null\-conditional operator checks the result for *null*\. This means that the developer assumed that conversion via *as* could return *null*\. The developer protected the code with operator '?\.' from *NullReferenceException* only once, but further an exception can be thrown\.

**Issue 2**

The next warning does not indicate an obvious error, but the code fragment looks suspicious nevertheless:

```cpp
private static Signature BuildSignature(IMethodSymbol symbol)
{
  ....
  var returnTypeInfo = CheckForStaticExtension.GetReturnType(symbol);
  if(returnTypeInfo.HasValue)
  {
    if(returnTypeInfo.Value.inbuilt)
    {
      signature.BuiltInReturnType = returnTypeInfo.Value.name;
    }
    else
    {
      signature.ReturnType = returnTypeInfo.Value.name;
    }
  }
  signature.Parameters = symbol.Parameters.Select(parameter =>
  {
    var info = CheckForStaticExtension.GetReturnType(parameter);
    ....
    if(info.HasValue)
    {
      if(info.Value.inbuilt)
      {
        result.BuiltInType = info.Value.name;   // <=
      }
      else
      {
        result.BuiltInType = info.Value.name;   // <=
      }
    }
    ....
  }).ToList();
  ....
}
```

[V3004](https://pvs-studio.com/en/docs/warnings/v3004/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) The 'then' statement is equivalent to the 'else' statement\. InvocationContext\.cs 400

PVS\-Studio detected that then and else branches of the *if* statement are equivalent\. At the same time, if we go a little higher up in the code, we notice that there is a similar fragment, but the branches are different\. It is possible that one of the branches is unnecessary\. By the way, it is possible that there must be a different property in the place of *BuiltInType*\. For example, a similar *Type* property that does exist\. The developer could have been using auto\-completion and did not notice that the auto\-completed code was wrong\. By the way, VS2022 suggested me the wrong prompt, too\. As the saying goes: "Trust but verify\."

**Issue 3**

I formatted the following code because it was unstructured and hard to read, but I left the logic unchanged\.

![Image description](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/smme1lsvqqic8bv2uph8.png)

This is what the code looks like in the editor\. If the code is formatted a bit better, the error becomes much more obvious\.

```cpp
private bool CursorIsValidDeclaration(ClangCursor c)
{
  var result = false;

  if (  (c.Kind == NClang.CursorKind.FunctionDeclaration)  // <=
      || c.Kind == NClang.CursorKind.CXXMethod 
      || c.Kind == NClang.CursorKind.Constructor 
      || c.Kind == NClang.CursorKind.Destructor 
      || c.Kind == NClang.CursorKind.FunctionDeclaration)  // <=
  {
    result = true;
  }
  return result;
}
```

[V3001](https://pvs-studio.com/en/docs/warnings/v3001/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) There are identical sub\-expressions 'c\.Kind == NClang\.CursorKind\.FunctionDeclaration' to the left and to the right of the '\|\|' operator\. CPlusPlusLanguageService\.cs 1275

Such errors are made due to inattentiveness\. One of the comparisons may be unnecessary\. Or it might be that, instead of one of the *FunctionDeclaration* items, there should be something else :\)\.

**Issue 4**

```cpp
public override void Render(DrawingContext context)
{
  ....
  foreach (var breakPoint in _manager?.OfType<Breakpoint>().Where(....))
  {
    ....
  }
  ....
}
```

[V3153](https://pvs-studio.com/en/docs/warnings/v3153/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. BreakPointMargin\.cs 46

The analyzer detected that the result of the '?\.' operator is dereferenced immediately\. This *null* check inside *foreach* makes no sense\. *NullReferenceException* still catches up with the developer when the execution flow reaches a *null* reference :\)\.

In general, this behavior is not obvious to many programmers\. We even wrote an entire article on this topic: "[Using the ?\. operator in foreach: protection against NullReferenceException that doesn't work](https://pvs-studio.com/en/blog/posts/csharp/0832/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio)"\.

![Image description](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/dbjzowt8kvibu0nlvdfa.png)

**Issue 5**

```cpp
public async Task<(....)> LoadProject(....)
{
  ....
  return await Task.Run(() =>
  {
    ....
    if (   loadData.CscCommandLine != null                            // <=
        && loadData.CscCommandLine.Count > 0)
    {
      ....
      return (projectInfo, projectReferences, loadData.TargetPath);
    }
    else
    {
      ....
      return (projectInfo, projectReferences, loadData?.TargetPath);  // <=
    }
  });
}
```

[V3095](https://pvs-studio.com/en/docs/warnings/v3095/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) The 'loadData' object was used before it was verified against null\. Check lines: 233, 262\. MSBuildHost\.cs 233

This error seems pretty straightforward\. If the *loadData* variable potentially equals *null* in the else branch, this variable can also be *null* in the same *if* condition\. Except that there is no *loadData* check for null\-conditional, which means an exception can be thrown\. And yes, *loadData* does not change in else in any way\. Most likely the developer missed the '?\.' operator in the *if* statement\. Although, it is possible that this operator in *return* is unnecessary and should be removed in order not to confuse the developers\.

**Issue 6**

```cpp
public override void Render(DrawingContext context)
{
  if (_lastFrame != null && !(   _lastFrame.Width == 0 
                              || _lastFrame.Width == 0))
  {
    ....
  }
  base.Render(context);
}
```

[V3001](https://pvs-studio.com/en/docs/warnings/v3001/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) There are identical sub\-expressions '\_lastFrame\.Width == 0' to the left and to the right of the '\|\|' operator\. RemoteWidget\.cs 308

The analyzer found two identical sub\-expressions to the left and right of the '\|\|' operator\. I think that fixing such a mistake is pretty easy: you need to change one of the *Width* properties to *Height*\. This property exists and is used further in the method\.

**Issue 7**

```cpp
public GlobalRunSpec(....)
{
  ....
  if (specialEntry.Value != null)
  {
    ....
  }
  RunSpec spec = new(specialOps,
                     specialVariables ?? variables,
                     specialEntry.Value.VariableSetup.FallbackFormat);
  ....
}
```

[V3125](https://pvs-studio.com/en/docs/warnings/v3125/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) The 'specialEntry\.Value' object was used after it was verified against null\. Check lines: 92, 86\. GlobalRunSpec\.cs 92

PVS\-Studio has detected something suspicious\. The *specialEntry\.Value* is checked for *null*, and then is used without the appropriate verification\. Inside the *if* statement, *specialEntry\.Value* does not change its value\. 

**Issue 8**

```cpp
private static StyledText InfoTextFromCursor(ClangCursor cursor)
{
  ....
  if (cursor.ResultType != null)
  {
    result.Append(cursor.ResultType.Spelling + " ",
                  IsBuiltInType(cursor.ResultType) ? theme.Keyword 
                                                   : theme.Type);
  }
  else if (cursor.CursorType != null)
  {
    switch (kind)
    {
      ....
    }
    result.Append(cursor.CursorType.Spelling + " ",
                  IsBuiltInType(cursor.ResultType) ? theme.Keyword   // <=
                                                   : theme.Type);
  }
  ....
}
```

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) Expression 'IsBuiltInType\(cursor\.ResultType\)' is always false\. CPlusPlusLanguageService\.cs 1105

The analyzer expects *IsBuiltInType\(cursor\.ResultType\)* to always return *false*\. If the *IsBuiltInType *method is called in the* *else branch, *cursor\.ResultType *has the *null* value\. The *IsBuiltInType* method contains a check that makes sure that if the parameter is *null*, the method returns *false*\.

```cpp
private static bool IsBuiltInType(ClangType cursor)
{
  var result = false;
  if (cursor != null && ....)
  {
    return true;
  }
  return result;
}
```

It turns out that *IsBuiltInType\(cursor\.ResultType\)* always returns false\.

Hmm, why did this happen? There is a clear copy\-paste problem here\. The developer simply copied a similar code fragment from above — but forgot to change *cursor\.ResultType* to *cursor\.CursorType*\.

**Issue 9**

```cpp
private int BuildCompletionsForMarkupExtension(....)
{
  ....
  if (t.SupportCtorArgument == MetadataTypeCtorArgument.HintValues)
  {
    ....
  }
  else if (attribName.Contains("."))
  {
    if (t.SupportCtorArgument != MetadataTypeCtorArgument.Type)
    {
      ....
      if (   mType != null 
          && t.SupportCtorArgument ==
             MetadataTypeCtorArgument.HintValues)        // <=
      {
        var hints = FilterHintValues(....);
        completions.AddRange(hints.Select(.....));
      }
      ....
    }  
  }
}
```

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) Expression 'mType \!= null && t\.SupportCtorArgument == MetadataTypeCtorArgument\.HintValues' is always false\. CompletionEngine\.cs 601

The *SupportCtorArgument* property is compared to the enumeration value\. This expression is in the else branch of the *if* statement\. *SupportCtorArgument* is a normal auto\-property\. If the property was equal to *MetadataTypeCtorArgument\.HintValues*, the code execution would not get to this *else* branch\. Accordingly, the expression in the condition will always be false, and the code that is in the then branch will never be executed\.

**Issue 10**

```cpp
private void RegisterLanguageService(ISourceFile sourceFile)
{
  ....
  var languageServiceProvider = IoC.Get<IStudio>()
                                   .LanguageServiceProviders
                                   .FirstOrDefault(....)?.Value;

  LanguageService = languageServiceProvider.CreateLanguageService();  // <=

  if (LanguageService != null)
  {
    ....
  }
}
```

[V3105](https://pvs-studio.com/en/docs/warnings/v3105/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) The 'languageServiceProvider' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. CodeEditorViewModel\.cs 309

The analyzer indicates that a variable whose value was obtained by a null\-conditional operator is being dereferenced\. 

And as we can see, the '?\.' operator is really used to calculate the *languageServiceProvider* variable value\. On the next line, this variable is dereferenced without being checked first\. Such code can generate a *NullReferenceException *exception\. One way to fix the code is to use '?\.' when calling *CreateLanguageService*\. 

**Issue 11**

```cpp
public void SetCursorPosition(int column, int row)
{
  ....
  if (LeftAndRightMarginEnabled)
  {
    if (CursorState.OriginMode && CursorState.CurrentColumn < LeftMargin)
      CursorState.CurrentColumn = LeftMargin;
    if (CursorState.CurrentColumn >= RightMargin)
      RightMargin = RightMargin;                   // <=
  }
  ....
}
```

[V3005](https://pvs-studio.com/en/docs/warnings/v3005/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) The 'RightMargin' variable is assigned to itself\. VirtualTerminalController\.cs 1446

The variable *RightMargin* is set to itself\. The code is certainly suspicious\. It's difficult to guess what the developer intended to do here and what *RightMargin* should be set to\. 

**Issue 12**

The next triggering is intriguing and somewhat obscure\.

```cpp
public class ColorScheme
{
  private static List<ColorScheme> s_colorSchemes =
    new List<ColorScheme>();
  private static Dictionary<string, ColorScheme> s_colorSchemeIDs =
    new Dictionary<string, ColorScheme>();
  private static readonly ColorScheme DefaultColorScheme = 
    ColorScheme.SolarizedLight;
  ....
  public static readonly ColorScheme SolarizedLight = new ColorScheme
  {
    ....
  };
}
```

Try to spot an error among these lines\. The task is simplified as much as possible, because behind "\.\.\.\." I have hidden almost 200 lines of code, which are not necessary to detect the error\. Found anything?

Let's delete some more code and add the analyzer message\.

```cpp
public class ColorScheme
{
  private static readonly ColorScheme DefaultColorScheme = 
    ColorScheme.SolarizedLight;
  ....
  public static readonly ColorScheme SolarizedLight = new ColorScheme
  {
    ....
  };
}
```

[V3070](https://pvs-studio.com/en/docs/warnings/v3070/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio) Uninitialized variable 'SolarizedLight' is used when initializing the 'DefaultColorScheme' variable\. ColorScheme\.cs 32

I think the analyzer's message makes everything clear\. The point is that the *DefaultColorScheme* field will be initialized with a different value than what the developer expected\. At the time *DefaultColorScheme* is initialized, the *SolarizedLight* field will have a default value of *null*\. It turns out that the *DefaultColorScheme* field will also be set to a *null* value\. You can find more examples in the documentation of this diagnostic\. The link is above, in the analyzer's message\.

## Conclusion

The PVS\-Studio analyzer found several interesting errors in AvalonStudio\. I hope that this article will help improve this project\. It is worth mentioning that open\-source IDEs are quite scarce these days\. I hope there'll be more of them in the future — so that every developer can choose a development environment to their liking\. They could even participate in the development of the tool they use\.

You can also improve your project in an easy and free way\. All you need to do is to check your code with PVS\-Studio\. And the trial key, which you can get on our [website](https://pvs-studio.com/en/pvs-studio/try-free/?utm_source=dev_to&utm_medium=articles&utm_content=AvalonStudio), will help you do it :\)\.