Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

Support of Visual Studio 2019 in PVS-Studio, Part 2

DZone 's Guide to

Support of Visual Studio 2019 in PVS-Studio, Part 2

We crap up this two part series by exploring the toolset made possible by this pairing, and changes this brought to backend and front-end coding practices.

· Web Dev Zone ·
Free Resource

Welcome back! If you missed Part 1, you can check it out here

Toolset

It was obvious that updating the toolset would be the most difficult part. At least that's what it looked like in the beginning, but now I tend to think that support of the plugin was the most difficult part. For one thing, we already had a toolset and a mechanism for evaluating MSBuild projects, which was good as it was even though it had yet to be extended. The fact that we didn't have to write the algorithms from scratch made it way easier. The strategy of relying on "our" toolset, which we preferred to stick to when supporting Visual Studio 2017, once again proved right.

Traditionally, the process starts with updating NuGet packages. The tab for managing NuGet packages for the current solution contains the "Update" button... but it doesn't help. Updating all the packages at once caused multiple version conflicts, and trying to solve them all didn't seem a good idea. A more painful yet presumably safer way was to selectively update target packages of Microsoft.Build/Microsoft.CodeAnalysis.

One difference was spotted right away when testing the diagnostics: the syntax tree's structure changed on an existing node. Not a big deal; we fixed that quickly.

Let me remind you, we test our analyzers (for C#, C++, Java) on open-source projects. This allows us to thoroughly test the diagnostics — for example, check them for false positives or see if we missed any cases (to reduce the number of false negatives). These tests also help us trace possible regression at the initial step of updating the libraries/toolset. This time they caught a number of issues as well.

One was that the behavior inside CodeAnalysis libraries got worse. Specifically, when checking certain projects, we started to get exceptions from the libraries' code on various operations such as obtaining semantic information, opening projects, and so on.

Those of you who have carefully read the article about support of Visual Studio 2017 remember that our distribution comes with a dummy — the file MSBuild.exe of 0 bytes.

Now we had to push this practice even further and include empty dummies for the compilers csc.exe, vbc.exe, and VBCSCompiler.exe. Why? We came up with this solution after analyzing one of the projects from our test base and getting diff reports: the new version of the analyzer wouldn't output some of the expected warnings.

We found that it had to do with conditional compilation symbols, some of which weren't extracted properly when using the new version of the analyzer. In order to get to the root of the problem, we had to dig deeper into the code of Roslyn's libraries.

Conditional compilation symbols are parsed using the  GetDefineConstantsSwitch method of the class Csc from the library Microsoft.Build.Tasks.CodeAnalysis. The parsing is done using the String.Split method on a number of separators:

string[] allIdentifiers 
  = originalDefineConstants.Split(new char[] { ',', ';', ' ' });

This parsing mechanism works perfectly; all the conditional compilation symbols are correctly extracted. Okay, let's keep digging.

The next key point was the call of the  ComputePathToTool method of the class ToolTask. This method computes the path to the executable file (csc.exe) and checks if it's there. If so, the method returns the path to it or null otherwise.

The calling code:

....
string pathToTool = ComputePathToTool();
if (pathToTool == null)
{
    // An appropriate error should have been logged already.
    return false;
}
....

Since there is no csc.exe file (why'd we need it?), pathToTool is assigned the value null at this point, and the current method (ToolTask.Execute) returns false. The results of executing the task, including the extracted conditional compilation symbols, are ignored.

Okay, let's see what happens if we put the csc.exe file where it's expected to be.

Now pathToTool stores the actual path to the now-present file, and ToolTask.Execute keeps executing. The next key point is the call of the ManagedCompiler.ExecuteTool method:

protected override int ExecuteTool(string pathToTool, 
                                   string responseFileCommands, 
                                   string commandLineCommands)
{
  if (ProvideCommandLineArgs)
  {
    CommandLineArgs = GetArguments(commandLineCommands, responseFileCommands)
      .Select(arg => new TaskItem(arg)).ToArray();
  }

  if (SkipCompilerExecution)
  {
    return 0;
  }
  ....
}

The SkipCompilerExecution property is true (logically enough since we are not compiling for real). The calling method (the already mentioned ToolTask.Execute) checks if the return value for ExecuteTool is 0 and, if so, returns true. Whether your csc.exe was an actual compiler or "War and Peace" by Leo Tolstoy doesn't matter at all.

So, the problem has to do with the order in which the steps were defined:

  • check for a compiler
  • check if the compiler should be launched

And we'd expect a reverse order. It's to fix this that the dummies for the compilers were added.

Okay, but how did we manage to get compilation symbols at all, with the csc.exe file absent (and the task results ignored)?

Well, there is a method for this case too: CSharpCommandLineParser.ParseConditionalCompilationSymbols from the library Microsoft.CodeAnalysis.CSharp. It, too, does parsing by calling the String.Split method on a number of separators:

string[] values 
  = value.Split(new char[] { ';', ',' } /*, 
                StringSplitOptions.RemoveEmptyEntries*/);

See how this set of separators is different from that handled by theCsc.GetDefineConstantsSwitch method? Here, a space isn't a separator. It means that conditional compilation symbols separated by spaces won't be parsed properly by this method.

That's what happened when we were checking the problem projects: they used space-separated conditional compilation symbols and, therefore, were successfully parsed by the GetDefineConstantsSwitch method but not the ParseConditionalCompilationSymbols method.

Another problem that showed up after updating the libraries was broken behavior in certain cases — specifically, on projects that didn't build. It affected the Microsoft.CodeAnalysis libraries and manifested itself as exceptions of all kinds: ArgumentNullException (failed initialization of some internal logger), NullReferenceException, and so on.

I'd like to tell you about one particular error that I found pretty interesting.

We encountered it when checking the new version of the Roslyn project: one of the libraries was throwing a NullReferenceException. Thanks to detailed information about its source, we quickly found the problem source code and — just for curiosity — decided to check if the error would persist when working in Visual Studio.

We did manage to reproduce it in Visual Studio (version 16.0.3). To do that, you need a class definition like this:

class C1<T1, T2>
{
  void foo()
  {
    T1 val = default;
    if (val is null)
    { }
  }
}

You'll also need Syntax Visualizer (it comes with the .NET Compiler Platform SDK). Look up the TypeSymbol (by clicking the "View TypeSymbol (if any)" menu item) of the syntax tree node of type ConstantPatternSyntax (null). Visual Studio will restart, and the exception info — specifically, the stack trace — will become available in Event Viewer:

Application: devenv.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.NullReferenceException
   at Microsoft.CodeAnalysis.CSharp.ConversionsBase.
        ClassifyImplicitBuiltInConversionSlow(
          Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol, 
          Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol, 
          System.Collections.Generic.HashSet'1
            <Microsoft.CodeAnalysis.DiagnosticInfo> ByRef)
   at Microsoft.CodeAnalysis.CSharp.ConversionsBase.ClassifyBuiltInConversion(
        Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol, 
        Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol, 
        System.Collections.Generic.HashSet'1
          <Microsoft.CodeAnalysis.DiagnosticInfo> ByRef)
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetTypeInfoForNode(
        Microsoft.CodeAnalysis.CSharp.BoundNode,
        Microsoft.CodeAnalysis.CSharp.BoundNode,
        Microsoft.CodeAnalysis.CSharp.BoundNode)
   at Microsoft.CodeAnalysis.CSharp.MemberSemanticModel.GetTypeInfoWorker(
        Microsoft.CodeAnalysis.CSharp.CSharpSyntaxNode,
        System.Threading.CancellationToken)
   at Microsoft.CodeAnalysis.CSharp.SyntaxTreeSemanticModel.GetTypeInfoWorker(
        Microsoft.CodeAnalysis.CSharp.CSharpSyntaxNode,
        System.Threading.CancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetTypeInfo(
        Microsoft.CodeAnalysis.CSharp.Syntax.PatternSyntax, 
        System.Threading.CancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetTypeInfoFromNode(
        Microsoft.CodeAnalysis.SyntaxNode, System.Threading.CancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetTypeInfoCore(
        Microsoft.CodeAnalysis.SyntaxNode, System.Threading.CancellationToken)
....

As you can see, the problem is caused by a null reference dereference.

As I already mentioned, we encountered a similar problem when testing the analyzer. If you build it using debug libraries from Microsoft.CodeAnalysis, you can get right to the problem spot by looking up the TypeSymbol of the corresponding syntax tree node.

It will eventually take us to the ClassifyImplicitBuiltInConversionSlow method mentioned in the stack trace above:

private Conversion ClassifyImplicitBuiltInConversionSlow(
  TypeSymbol source,
  TypeSymbol destination,
  ref HashSet<DiagnosticInfo> useSiteDiagnostics)
{
  Debug.Assert((object)source != null);
  Debug.Assert((object)destination != null);

  if (source.SpecialType == SpecialType.System_Void ||
      destination.SpecialType == SpecialType.System_Void)
  {
    return Conversion.NoConversion;
  }

  Conversion conversion 
    = ClassifyStandardImplicitConversion(source, destination,
                                         ref useSiteDiagnostics);
  if (conversion.Exists)
  {
    return conversion;
  }

  return Conversion.NoConversion;
}

Here, the destination parameter is null, so calling destination.SpecialType results in throwing a NullReferenceException. Yes, the dereference operation is preceded by Debug.Assert, but it doesn't help because in fact it doesn't protect from anything — it simply allows you to spot the problem in the debug versions of the libraries. Or it doesn't.

Changes to the Mechanism of Evaluating C++ Projects

There wasn't much interesting in this part: the existing algorithms didn't require any big modifications worth mentioning, but you may want to know about two minor issues.

The first was that we had to modify the algorithms that relied on the numerical value of ToolsVersion. Without going into details, there are certain cases when you need to compare toolsets and choose, say, the most recent version. The new version, naturally, has a larger value. We expected that ToolsVersion for the new MSBuild/Visual Studio would have the value 16.0. Yeah, sure! The table below shows how the values of different properties changed throughout Visual Studio's development history:

Visual Studio product name

Visual Studio version number

Tools Version

PlatformToolset version

Visual Studio 2010

10.0

4.0

100

Visual Studio 2012

11.0

4.0

110

Visual Studio 2013

12.0

12.0

120

Visual Studio 2015

14.0

14.0

140

Visual Studio 2017

15.0

15.0

141

Visual Studio 2019

16.0

Current

142

I know that the joke about the messed version numbers of Windows and Xbox is an old one, but it proves that you can't make any reliable predictions about the values (whether in the name or the version) of future products. 

We solved that easily by adding prioritizing for toolsets (i.e. singling out priority as a separate entity).

The second issue involved problems with working in Visual Studio 2017 or related environment (for instance, when the VisualStudioVersion environment variable is set). It occurs because computing parameters needed to evaluate a C++ project is a much more difficult task than evaluating a .NET project. For .NET, we use our own toolset and the corresponding value of ToolsVersion. For C++, we can utilize both our own toolset and the ones provided by the system. Starting with Build Tools for Visual Studio 2017, toolsets are defined in the file MSBuild.exe.config instead of the registry. That's why we couldn't get them from the global list of toolsets anymore (using Microsoft.Build.Evaluation.ProjectCollection.GlobalProjectCollection.Toolsets, for example) unlike those defined in the registry (i.e. for Visual Studio 2015 and earlier).

All this prevents us from evaluating a project using ToolsVersion 15.0 because the system won't see the required toolset. The most recent toolset, Current, will still be available since it's our own toolset, and, therefore, there's no such problem in Visual Studio 2019. The solution was quite simple and allowed us to fix that without changing the existing evaluation algorithms: we just had to include another toolset, 15.0, into the list of our own toolsets in addition to Current.

Changes to the Mechanism of Evaluating C# .NET Core Projects

This task involved two interrelated issues:

  • Adding the 'Current' toolset broke analysis of .NET Core projects in Visual Studio 2017.
  • Analysis wouldn't work for .NET Core projects on systems without at least one copy of Visual Studio installed.

Both problems were coming from the same source: some of the base .targets/.props files were looked up at wrong paths. This prevented us from evaluating a project using our toolset.

If you had no Visual Studio instance installed, you'd get the following error (with the previous toolset version, 15.0):

The imported project
"C:\Windows\Microsoft.NET\Framework64\
15.0\Microsoft.Common.props" was not found.

When evaluating a C# .NET Core project in Visual Studio 2017, you'd get the following error (with the current toolset version, Current):

The imported project 
"C:\Program Files (x86)\Microsoft Visual Studio\
2017\Community\MSBuild\Current\Microsoft.Common.props" was not found. 
....

Since these problems are similar (which they do seem to be), we could try killing two birds with one stone.

In the next paragraphs, I'll explain how we accomplished that without going into details. These details (about how C# .NET Core projects are evaluated as well as changes to the evaluation mechanism in our toolset) will be the topic of one of our future articles. By the way, if you were reading this article carefully, you probably noticed that this is the second reference to our future articles.

Now, how did we solve that problem? We extended our own toolset with the base .targets/.props files from .NET Core SDK (Sdk.props, Sdk.targets). That gave us more control over the situation and more flexibility in import management as well as evaluation of .NET Core projects in general. Yes, our toolset got a bit larger again, and we also had to add logic for setting up the environment required for evaluation of .NET Core projects, but it seems worth it.

Until then, we had evaluated .NET Core projects by simply requesting the evaluation and relying on MSBuild to do the job.

Now that we had more control over the situation, the mechanism changed a bit:

  • Set up the environment required for evaluating .NET Core projects.
  • Evaluation:
    • Start evaluation using .targets/.props files from our toolset.
    • Continue evaluation using external files.

This sequence suggests that setting up the environment pursues two main goals:

  • Initiate evaluation using .targets/.props files from our toolset.
  • Redirect all subsequent operations to external .targets/.props files.

A special library Microsoft.DotNet.MSBuildSdkResolver is used to look up the necessary .targets/.props files. In order to initiate the setting up of the environment using files from our toolset, we utilized a special environment variable used by that library so that we could point at the source where to import the necessary files from (i.e. our toolset). Since the library is included into our distribution, there's no risk of a sudden logic failure.

Now we have the SDK files from our toolset imported first, and since we can easily change them now, we fully control the rest of the evaluation logic. It means we can now decide which files and from what location to import. The same applies to Microsoft.Common.props mentioned above. We import this and other base files from our toolset so we don't have to worry about their existence or contents.

Once all the necessary imports are done and properties set, we pass control over the evaluation process to the actual .NET Core SDK, where all the rest required operations are performed.

Conclusion

Supporting Visual Studio 2019 was generally easier than supporting Visual Studio 2017 for a number of reasons. First, Microsoft didn't change as many things as they had when updating from Visual Studio 2015 to Visual Studio 2017. Yes, they did change the base toolset and forced Visual Studio plugins to switch to asynchronous load mode, but this change wasn't that drastic. Second, we already had a ready-made solution involving our own toolset and project evaluation mechanism and we simply didn't have to work it all up from scratch — only build on what we already had. The relatively painless process of supporting analysis of .NET Core projects under new conditions (and on computers with no Visual Studio copies installed) by extending our project evaluation system also gives us hope that we have made the right choice by taking some of the control in our hands.

But I'd like to repeat the idea communicated in the previous article: sometimes using ready-made solutions isn't that easy as it may seem.

Topics:
web dev ,web application development ,visual studio 2019 ,pvs-studio ,web development tools

Published at DZone with permission of

Opinions expressed by DZone contributors are their own.

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}