Derive type names analyzer from AbstractBuiltInCodeStyleDiagnosticAnalyzer #62869
Conversation
ghost
added
Area-IDE
Community
| protected SimplifyTypeNamesDiagnosticAnalyzerBase() | ||
| : base(ImmutableDictionary<DiagnosticDescriptor, IOption2>.Empty | ||
| .Add(s_descriptorSimplifyNames, null!) | ||
| .Add(s_descriptorSimplifyMemberAccess, null!) | ||
| .Add(s_descriptorPreferBuiltinOrFrameworkType, null!)) // PROTOTYPE: TODO: This was already broken. We have no ctor that matches what this analyzer has (ie, multiple descriptors, with one descriptor having more than one option). | ||
| { | ||
| } |
There was a problem hiding this comment.
@mavasani Two of these descriptors don't have options, and one of them has two options, PreferIntrinsicPredefinedTypeKeywordInMemberAccess and PreferIntrinsicPredefinedTypeKeywordInDeclaration
Are you okay with introducing a new constructor that accepts ImmutableDictionary<DiagnosticDescriptor, ImmutableHashSet<IOption2>>?
Or do you have another suggestion?
There was a problem hiding this comment.
Actually the options are already added by another analyzer that uses the same ID:
There was a problem hiding this comment.
@mavasani This is something to consider as a follow-up. Should we remove s_descriptorPreferBuiltinOrFrameworkType from SimplifyTypeNamesDiagnosticAnalyzerBase in favor of PreferFrameworkTypeDiagnosticAnalyzerBase? They both use the same code-style options and the same diagnostic id.
There was a problem hiding this comment.
Tracked by #50307. I forgot I opened such an issue :)
There was a problem hiding this comment.
Everything you mentioned in the above comments sounds good to me!
|
@mavasani Can you review please? Thanks! |
|
@mavasani There is an unrelated failure here. |
3 participants
I tried to move the analyzer to Analyzers layer, but the language-specific implementations depend on internals of workspaces. But I kept the abstract base class in Analyzers.
Fixes #62639