This project is read-only.
2

Resolved

Enable export of a diagnostic analyzer class for multiple languages

description

The current design of ExportDiagnosticAnalyzerAttribute (with AllowMultiple = false) means that a diagnostic analyzer that can target multiple languages cannot be authored as a single class. Instead, a class must be delivered for each target language (à la CA1715DiagnosticAnalyzer, CSharpCA1715DiagnosticAnalyzer, and BasicCA1715DiagnosticAnalyzer). This is likely to be rather annoying for diagnostic developers.

The initial extra work for the language-specific subclasses is one concern. Needing to verify each subclass in order to discover if it actually contains language-specific logic in eventual longer-term maintenance is likely to be even more problematic. Both could be relatively easily avoided by using the approach described at http://blogs.microsoft.co.il/bnaya/2010/01/29/mef-for-beginner-repeatable-metadata-part-9/ (or the variant at http://stackoverflow.com/questions/10988447/mef-getexportst-tmetadataview-returning-nothing-with-allowmultiple-true/#11010364 if avoiding the singular/plural property names discrepancy is deemed desirable).

comments

heejaechang wrote Apr 12, 2014 at 10:00 AM

one of problem creating one diagnostic provider for both language is that it will always bring in both languages' dlls (CodeAnalysis.CSharp or CodeAnalysis.VisualBasic) even when a solution contains only one of languages (unless code is carefully crafted so that provider references (csharp/vb) types that only appear in a solution.)

but I agree that probably for a quick and dirty implementation of a diagnostic provider, creating one diagnostic for both languages would be more convinient.

calinoiu wrote Apr 12, 2014 at 8:59 PM

@heejaechang,

I'm not sure why you think that either the C# or VB DLLs would be required for the "common" diagnostics. It is quite possible to create a diagnostic analyzer that does not depend on anything language-specific. For example, take a look at http://source.roslyn.codeplex.com/#Microsoft.CodeAnalysis.FxCopAnalyzers/Naming/CA1715DiagnosticAnalyzer.cs. This is a complete, self-contained rule that applies to both C# and VB without any modifications, and it is declared in an assembly with no references to either of the language-specific DLLs. However, because of the way the export attribute is defined, this class cannot be used to define the rule alone. Instead, C# and VB versions must be declared separately (http://source.roslyn.codeplex.com/#Microsoft.CodeAnalysis.CSharp.FxCopAnalyzers/Naming/CSharpCA1715DiagnosticAnalyzer.cs and http://source.roslyn.codeplex.com/#Microsoft.CodeAnalysis.VisualBasic.FxCopAnalyzers/Naming/BasicCA1715DiagnosticAnalyzer.vb, respectively).

These subclasses add nothing besides their export attributes, and they could just as easily have been defined in the same assembly as their base class should the authors have chosen to set things up that way. All I'm proposing is a minor re-design on the export attribute to avoid requiring us to create the essentially useless subclasses. If this were, possible, CA1715DiagnosticAnalyzer could look like the following, and both CSharpCA1715DiagnosticAnalyzer and BasicCA1715DiagnosticAnalyzer could disappear entirely:
[DiagnosticAnalyzer]
[Export(typeof(IDiagnosticAnalyzer))]
[DiagnosticAnalyzerMetadata(RuleId, LanguageNames.CSharp)]
[DiagnosticAnalyzerMetadata(RuleId, LanguageNames.VisualBasic)]
public sealed class CA1715DiagnosticAnalyzer : ISymbolAnalyzer
{
    private const string RuleId = "CA1715";
    //…
}
Or, better yet, DiagnosticAnalyzerAttribute could be modified to subclass ExportAttribute, and the second attribute could be dispensed with entirely.

Hope that makes things a bit clearer,
Nicole

heejaechang wrote Apr 14, 2014 at 8:17 AM

if one can write diagnostic provider all within common layer, then Yes. it won't bring in all language specific dlls since diagnostic engine will lazily bring in dlls only appear in a solution.

let me forward this to diagnostic team.

latkin wrote Apr 18, 2014 at 11:19 PM

One should be able to write symbol-based analyzers as you describe that target both languages.
Manish - can you take a look at enabling this?

angocke wrote May 14, 2014 at 12:29 AM

Fixed in changeset 97af9d216e03e1e145f171092f324bb72b347987