Project-system: Dependencies Node does not display Error icon where expected

[natidea]

Repro

Given the following Console App project

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp2.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="NuGet.Commands" Version="4.0.0" />
    <PackageReference Include="NuGet.Packaging" Version="3.5.0" />
  </ItemGroup>

</Project>

The resulting package downgrade diagnostics show up as errors in the Error List, but as Warnings in the Dependencies node:

This is despite the fact that the model attempts to distinguish between Errors and Warnings: DiagnosticDependencyModel.cs#L38-L49

[natidea]

Looks like this is just a missing rule in ResolvedPackageReference.xaml. Fixing with:

    <StringProperty Name="Severity" Visible="True" ReadOnly="True" />

results in:

Odd that the icon is so small though

[terrajobst]

Also odd that the aggregation doesn't work. I'd assume the spine to show error icons. Otherwise, the user is going to hunt for the errors :-)

[natidea]

Currently, the Dependencies tree only maintains resolved/unresolved state for the nodes in the spine. So those nodes are essentially being marked as unresolved, because they contain a diagnostic. I think in future work, we would need to add states for Diagnostic-Error and Diagnostic-Warning. In fact, we should probably stop using the Warning icon for "unresolved" nodes and come up with a more subtle icon. That way, warning and error icons clearly refer to diagnostics that come from the build, and it would help improve the experience in https://github.com/dotnet/project-system/issues/2079

[abpiskunov]

We did have this discussion with Sri to have two or three state nodes, ended up with two state, check with him for more details ....

[srivatsn]

If I remember right we had said that there weren't many errors that could show up in the tree. Now that NuGet has added the ability to set WarnAsError as well we should probably introduce three states.

[abpiskunov]

Sounds, good. We did have icons for error in Dev14 - who ever works in this item ping me to get them ..

[terrajobst]

What would this mean for the spine though?

[drewnoakes]

Testing this again, we now see no diagnostics at all (as per #3780):

The relevant info is available in project.assets.json:

  "logs": [
    {
      "code": "NU1605",
      "level": "Error",
      "message": "Detected package downgrade: NuGet.Packaging from 4.0.0 to 3.5.0. Reference the package directly from the project to select a different version. \r\n Issue2582DepNodeErrorIcons -> NuGet.Commands 4.0.0 -> NuGet.Client 4.0.0 -> NuGet.Packaging (>= 4.0.0) \r\n Issue2582DepNodeErrorIcons -> NuGet.Packaging (>= 3.5.0)",
      "libraryId": "NuGet.Packaging",
      "targetGraphs": [
        ".NETCoreApp,Version=v2.0"
      ]
    },
    {
      "code": "NU1605",
      "level": "Error",
      "message": "Detected package downgrade: NuGet.Packaging from 4.0.0 to 3.5.0. Reference the package directly from the project to select a different version. \r\n Issue2582DepNodeErrorIcons -> NuGet.Commands 4.0.0 -> NuGet.DependencyResolver.Core 4.0.0 -> NuGet.Protocol.Core.v3 4.0.0 -> NuGet.Packaging (>= 4.0.0) \r\n Issue2582DepNodeErrorIcons -> NuGet.Packaging (>= 3.5.0)",
      "libraryId": "NuGet.Packaging",
      "targetGraphs": [
        ".NETCoreApp,Version=v2.0"
      ]
    },
    {
      "code": "NU1605",
      "level": "Error",
      "message": "Detected package downgrade: NuGet.Packaging from 4.0.0 to 3.5.0. Reference the package directly from the project to select a different version. \r\n Issue2582DepNodeErrorIcons -> NuGet.Commands 4.0.0 -> NuGet.DependencyResolver.Core 4.0.0 -> NuGet.Repositories 4.0.0 -> NuGet.Packaging (>= 4.0.0) \r\n Issue2582DepNodeErrorIcons -> NuGet.Packaging (>= 3.5.0)",
      "libraryId": "NuGet.Packaging",
      "targetGraphs": [
        ".NETCoreApp,Version=v2.0"
      ]
    }
  ]

However no items of type Diagnostic are being provided by the IProjectSubscriptionUpdate.

Once #3780 is fixed, I will cycle around to look at this.