search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.88k stars 4.01k forks source link

Unify editorconfig parsers #40774

Open jmarolf opened 4 years ago

jmarolf commented 4 years ago

Background

We have an editorconfig parser in the compiler layer here: https://github.com/dotnet/roslyn/blob/9ba266a76ef4ea79f54551c8860a5021d1deffc0/src/Compilers/Core/Portable/CommandLine/AnalyzerConfig.cs#L109 AnalyzerConfig is used to parse all the editorconfig options when analyzers are run in the compiler etc.

There is also Microsoft.VisualStudio.CodingConventions (which is internal) here

This parser is used to provide a ICodingConventionsSnapshot instance in Visual Studio that is used by roslyn and presumably others.

Problem

As it exists today, roslyn uses two different editorconfig parsers that have entirely different runtime semantics regarding casing (see https://github.com/dotnet/roslyn/issues/40705 for an example where an issue will reproduce or not depending on which parser is used). In general I don't think that we want to have to handle the semantics or engineering costs of having two different parsers.

Engineering Proposal

Since the compiler team has already agreed to support an editorconfig parser in-perpetuity, I propose we refactor AnalyzerConfig's parser so it can be consumed in Microsoft.VisualStudio.CodingConventions. I'll leave the details vaugue but it seems at the very least we could produce a source based package that Microsoft.VisualStudio.CodingConventions consumes to ensure that we have consistent behavior.

CC: @olegtk @agocke @sharwell @jasonmalinowski

jasonmalinowski commented 4 years ago

So right now us trying to use both of these parsers was a transition mechanism while we sort out any remaining issues with the new one. Right now the editor one is only used if you explicitly opt into it in Visual Studio, and we're also working to remove the remaining use of it in things like dotnet-format. If Microsoft.VisualStudio.CodingConventions wants to consume ours they're welcome to, but that's their decision for their needs.

jmarolf commented 4 years ago

Absolutely. This if more of a discussion for @olegtk and friends. I assume that they do not want to maintain a parser when the compiler team already needs to.

olegtk commented 4 years ago

Surely we are open to not having to maintain the parser :) What are our options? source package? @aditya-kadam owns it nowadays.

jmarolf commented 4 years ago

Source package was my thought about what would be easiest