﻿# Any bugs in your IDE? Checking AvalonStudio with PVS\-Studio

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\.

![0966_AvalonStudio/image1.png](https://import.viva64.com/docx/blog/0966_AvalonStudio/image1.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/) earlier\. 

![0966_AvalonStudio/image2.png](https://import.viva64.com/docx/blog/0966_AvalonStudio/image2.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/) 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/) 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\.

![0966_AvalonStudio/image3.png](https://import.viva64.com/docx/blog/0966_AvalonStudio/image3.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/) 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/) 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/)"\.

![0966_AvalonStudio/image4.png](https://import.viva64.com/docx/blog/0966_AvalonStudio/image4.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/) 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/) 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/) 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/) 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/) 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/) 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/) 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/) 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/), will help you do it :\)\.