Share DiagnosticStrings add mechanism to generate a DiagnosticDescriptor given a DiagnosticId #2170
Conversation
| EndProject | ||
| Global | ||
| GlobalSection(SharedMSBuildProjectFiles) = preSolution | ||
| src\ILLink.Shared\ILLink.Shared.projitems*{92f5e753-2179-46dc-bdce-736858c18dc7}*SharedItemsImports = 13 |
There was a problem hiding this comment.
To educate myself whats this code doing?
There was a problem hiding this comment.
This code was generated by VS. Couldn't find anything particularly interesting; looks like the first entry (13) is referencing the shared project itself and the bellow ones are the projects that use the shared proj.
tlakollo
left a comment
tlakollo
left a comment
There was a problem hiding this comment.
Definitely a very clever of solving the diagnostics, thanks Mateo!
Update test
agocke
left a comment
agocke
left a comment
There was a problem hiding this comment.
Minor comments. LGTM otherwise
| { | ||
| readonly string _titleFormat; | ||
| readonly string _messageFormat; | ||
| static readonly ResourceManager _resourceManager = SharedStrings.ResourceManager; |
There was a problem hiding this comment.
Why do we need a static field here? It seems like a local variable would suffice
| var analyzerType = applicableAnalyzer.GetType (); | ||
| var rule = diagnostics[i].HasLocation && applicableAnalyzer.SupportedDiagnostics.Length == 1 ? string.Empty : $"{analyzerType.Name}.{diagnosticsId}"; | ||
| var rule = diagnostics[i].HasLocation && | ||
| applicableAnalyzer.SupportedDiagnostics.Length == 1 ? string.Empty : $"DiagnosticId.{(DiagnosticId) Int32.Parse (diagnosticsId.Substring (2))}"; |
There was a problem hiding this comment.
Extract helper method for this usage and below?
|
|
||
| return new DiagnosticDescriptor (diagnosticId.AsString (), | ||
| lrsTitle!, | ||
| lrsMessage!, |
There was a problem hiding this comment.
What if one of the title or message is null?
| { | ||
| public sealed override ImmutableArray<string> FixableDiagnosticIds | ||
| => ImmutableArray.Create (RequiresUnreferencedCodeAnalyzer.IL2026); | ||
| => ImmutableArray.Create (DiagnosticId.RequiresUnreferencedCode.AsString ()); |
There was a problem hiding this comment.
Consider storing the SupportedDiagnostics of the RequiresUnreferencedCodeAnalyzer as a static property, then calling that property to get the full list of diagnostic IDs for this.
Same idea for RequiresAssemblyFilesCodeFixProvider.
…tor given a DiagnosticId (dotnet/linker#2170) * Add DiagnosticId enum Add GetDiagnosticDescriptor * Check that the diagnostic id is in the range of the supported linker warnings * Lint * Share DiagnosticString * Noisy whitespace * Warnings go up to 6000 inclusive * PR feedback * Get diagnostic string Update test * Lint * PR feedback Commit migrated from dotnet/linker@acd72ce
3 participants
Share
DiagnosticString,DiagnosticId, andDiagnosticCategorybetween linker and analyzer.DiagnosticStringreceives a readable diagnostic id (a resource key which maps to exactly oneILXXXXcode) and returns its diagnostic message format; this way we might move warnings in the linker side to a simpler logging mechanism:context.Log(DiagnosticId, params string[] args, ...).The Roslyn Analyzer uses this to get a diagnostic descriptor given a diagnostic id; this saves some diagnostic descriptors handwriting; useful going forward with supporting more diagnostics on the analyzers.