C#: Add a MaD model for Microsoft.AspNetCore.Mvc.Controller.View

[MathiasVP]

This is necessary for some internal Microsoft work.

The intended flow is something along the lines of:

public class A : Controller {
  public void Foo() {
    ViewData["Foo"] = source;
    Sink(View()); // reaches Sink argument with acces path [ViewData, element, Value]
  }
}

and

public class B : Controller {
  public void Bar() {
    ViewBag.Bar = source;
    Sink(View()); // reaches Sink argument with acces path [ViewData, element, Value, Bar]
  }
}

Ideally, I would have liked to pop off Bar off the access path by specifying the ViewBag-related MaD rows as:

Argument[this].Property[Microsoft.AspNetCore.Mvc.Controller.ViewBag].AnyDynamicProperty
->
ReturnValue.Property[Microsoft.AspNetCore.Mvc.Controller.ViewData].Element.Property[System.Collections.Generic.KeyValuePair`2.Value]

where AnyDynamicProperty would be any DynamicPropertyContent, but since that's not currently possible I've simply accepted that there's a somewhat spurious property content on the access path 🤷

Let me know if you have any suggestions to doing it in a better way!

[github-actions]

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

• Changes to framework-coverage-csharp.rst:

-    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``Microsoft.AspNetCore.WebUtilities``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",59,2071,150,2
+    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``JsonToItemsTaskFactory``, ``Microsoft.Android.Build``, ``Microsoft.Apple.Build``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``Microsoft.AspNetCore.Mvc``, ``Microsoft.AspNetCore.WebUtilities``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.NET.Sdk.WebAssembly``, ``Microsoft.NET.WebAssembly.Webcil``, ``Microsoft.VisualBasic``, ``Microsoft.WebAssembly.Build.Tasks``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",59,2073,150,2
-    Totals,,106,12897,398,7
+    Totals,,106,12899,398,7

• Changes to framework-coverage-csharp.csv:

+ Microsoft.AspNetCore.Mvc,,,2,,,,,,,,,,,,,,,,,,,,2

[michaelnebel]

LGTM!

[michaelnebel]

nit: Consider collapsing the summaries into two summaries (as they don't mention the arguments of View).
A summary that covers the first four summaries would look like:

["Microsoft.AspNetCore.Mvc", "Controller", True, "View", "", "", "Argument[this].Property[Microsoft.AspNetCore.Mvc.Controller.ViewData].Element.Property[System.Collections.Generic.KeyValuePair`2.Value]", "ReturnValue.Property[Microsoft.AspNetCore.Mvc.Controller.ViewData].Element.Property[System.Collections.Generic.KeyValuePair`2.Value]", "value", "manual"]

[MathiasVP]

Oh, fantastic! I wasn't sure if the signature was optional or not since they seemed to be present for all the C# MaD rows I looked at. But I'll definitely do that!

[MathiasVP]

Fixed in 0d616ca

[michaelnebel]

It should be used with caution, but I think this seems like an appropriate case :-)
The test needs to be updated (it looks like one of the eight original summaries was a taint step (I didn't notice this in the review) - I suppose it should have been a value step?).

[MathiasVP]

Oh, yes it should definitely be a value-preserving step. I think I was initially intending to model this as simple taint from the qualifier to the return value, but with these access paths I convinced myself that value was correct and probably just forgot to update one of them 😅

[MathiasVP]

@michaelnebel do you mind re-approving now that I've accepted the test changes? 🥺

[michaelnebel]

LGTM