Roslyn: Passing MSBuild settings to a DiagnosticAnalyzer

Created on 30 Mar 2017  路  10Comments  路  Source: dotnet/roslyn

The only way to pass settings to analyzers is via <AdditionalFiles>. Unfortunately that requires a file on disk. In many instances, analyzer authors want to pass custom settings to their analyzers simply by letting the developer set some properties in their project file. Or they may want get the value of existing properties, e.g. TargetFramework.

Proposal

Introduce a new item group <AdditionalFileContent>. This allows analyzer authors to include a props file in their NuGet package like this:

<ItemGroup>
    <AdditionalFileContent Include="MyCustomProp=$(MyCustomProp);
                                    TargetFramework=$(TargetFramework)"
                           FileName="MyCompany.MyAnalyzer.settings" />
</ItemGroup>

The expectation is that <AdditionalFileContent> is a well-known item group that is handled by the built-in targets and does something like this:

  1. Creates a file with the name FileName somewhere in obj
  2. Updates the file only if the contents change
  3. Registers the file for Clean
  4. Turns the (fully qualified) file names into <AdditionalFiles> that are then passed to the compiler as usual

Consumption

The analyzer author will be able to access the data like this:

```C#
// Get text for my settings
var additionalFiles = compilationStartContext.Options.AdditionalFiles;
var mySettings = additionalFiles.Single(file => Path.GetFileName(file.Path).Equals("MyCompany.MyAnalyzer.settings", StringComparer.OrdinalIgnoreCase));

// Parse options (of course not very robust)
var mySettings = mySettings.Split(Environment.NewLine)
.Select(l => line.Split("="))
.ToDictionary(a => a[0], a => a[1]);

Of course, this requires parsing by the developer right now. So ideally the `AnalyzerOptions` class would get a new API like this:

```C#
namespace Microsoft.CodeAnalysis.Diagnostics
{
    public partial class AnalyzerOptions
    {
        ImmutableDictionary<string, string> GetFileOptions(string fileName);
    }
}

This would cut down the ceremony above to a single line:

C# var myOptions = compilationStartContext.Options.GetFileOptions("MyCompany.MyAnalyzer.settings");

Area-Analyzers Feature Request

Most helpful comment

Thanks @PathogenDavid!
Closing this feature request as this has been implemented.

All 10 comments

@srivatsn @jaredpar

This is closely related to the work here:
https://github.com/dotnet/roslyn/projects/18

I'm planning to write up a specific proposal Monday with @jasonmalinowski and @dpoeschl regarding an API to support features like the one described above as well as other options sources like .editorconfig or Visual Studio's IDE settings.

Just make sure to handle MSBuild properties as well. Editorconfig doesn't help for getting access to build configuration, which is very powerful and otherwise tedious to flow.

@terrajobst There are two sides to this:

  1. Analyzers need access to "options"
  2. "Options" need to be defined somewhere

The part I'm working on first is what the first item should look like (as a supporting contributor, not the feature owner), without constraining the second option. When you count the request in this issue, we're up to 3 places where options can be defined:

  1. In .editorconfig
  2. In Visual Studio (whatever a user has configured in Tools → Options...)
  3. In the build configuration (the feature requested in this issue)

Most of the discussion centers around .editorconfig because it offers the most consistent long-term behavior which a variety of analyzers will be able to rely on. Visual Studio options are important because we want code fixes to be able to account for user formatting configuration, and to date developers have relied on Visual Studio's options pages to configure these formatting preferences. However, Visual Studio options aren't the best for new settings or for analyzer-specific settings because they aren't consistently available in all the places where analyzers are usable. In a similar situation, MSBuild properties are valuable in some contexts, but we already face problems with the approach considering there are two totally-different project systems available for C# projects, both of which could want to use analyzers.

It would be nice if we found a way to read & write settings as MSBuild properties. Editor config is a static file. The power of MSBuild is that I can attach conditions and pretty much share arbitrary settings with an arbitrary set of projects. It would be a shame not being able to leverage that power.

Also, not everything is what you'd consider an option in the sense that the user can configure it for the analyzer. It might just be ambient build configuration that the analyzer would like to be aware of (in my case the framework the developer is targeting).

Chiming in now that I've chatted to understand @terrajobst's scenarios a bit better and am not on vacation.

I think the interesting bit to me is as @sharwell points out the options API we have (where you specify storage locations) does align well with this sort of model. Some options come from .editorconfig, some come from MSBuild, and "both" is a terrifying question we can address if needed. It does require fleshing out the "apply options back" on the workspaces API side of thing, but it fits into existing patterns well. And we will have to solve that same problem for .editorconfig So the API shape is nice.

Doing MSBuild doesn't really increase API concepts, but it definitely increases "time on keyboard plumbing stuff" implementation time. I think the lesson for now is to make sure that what we do for .editorconfig doesn't paint us into a corner for this.

@mavasani this is now supported, right?

Yep, @chsienki has made this possible.

For anyone interested, here's documentation on using the feature: Source Generators Cookbook: Consume MSBuild properties and metadata

(It's written in the context of source generators, but it looks like it applies to analyzers too.)

Thanks @PathogenDavid!
Closing this feature request as this has been implemented.

Was this page helpful?
0 / 5 - 0 ratings