Feature/convert name of #45806
Feature/convert name of #45806
Conversation
This reverts commit 1167407.
This reverts commit 2e219a6.
WIP Analyzer, just bare bones diagnostic
Feature/convert name of
Needs Testing
Changed how ConvertNameOfAsync in the CodeFixeProvider finds the identifier name of the typeof expression
- Removed unused using statements - Cleaned up ConvertNameOf method
The compiler handles it today. If a user uses the compiler APIs on a release build (which would be nearly everyone) the compiler handles this case just fine. Its only on debug builds that this is not accepted. If this is not valid then the compiler should throw an error in release builds instead of using |
|
I guess my general question is: Do we think it is correct and good for the compiler to accept some syntax trees on a Release build but not a Debug build (of the compiler)? |
What does it mean "just fine"? Doesn't crash? That is not sufficient to be fine. It looks to me that the scenario is not handled, i.e. the name is not extracted from the syntax. |
|
Why do you need to feed an invalid syntax to the compiler? |
We give ourselves a lot of flexibility in dealing with syntax trees that would never be produced by a parser. |
|
cc: @CyrusNajmabadi as he has more knowledge than me on the This is the code that created an invalid syntax node according to the VB compiler: public void ConvertTypeOfToNameOf(SemanticModel semanticModel, SyntaxEditor editor, SyntaxNode nodeToReplace)
{
var symbolType = GetSymbolType(semanticModel, nodeToReplace);
var typeExpression = editor.Generator.TypeExpression(symbolType);
var nameOfSyntax = editor.Generator.NameOfExpression(typeExpression);
editor.ReplaceNode(nodeToReplace, nameOfSyntax);
}Seems likely that this is a bug in the syntax editor then. I'll revert the feature work for VB and we can figure out how to create a nameof syntax in a safe way. |
|
|
||
| protected override Diagnostic LanguageReportDiagnostic(Location location, DiagnosticDescriptor cSharpDescriptor, DiagnosticDescriptor visualBasicDescriptor, CompilationOptions options) | ||
| { | ||
| return DiagnosticHelper.Create(visualBasicDescriptor, |
There was a problem hiding this comment.
Why VB descriptor in C# implementation?
| nameof(AnalyzersResources.Convert_gettype_to_nameof), AnalyzersResources.ResourceManager, typeof(AnalyzersResources))) | ||
| { | ||
| var csharpMessage = new LocalizableResourceString(nameof(AnalyzersResources.Convert_typeof_to_nameof), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)); | ||
| CSharpDescriptor = CreateDescriptorWithId(DescriptorId, csharpMessage, csharpMessage); |
There was a problem hiding this comment.
Why are language specific messages and descriptors being created in language agnostic layer? This seems wrong, the sub-type should have all language specific code and abstract type should have no mention of language specific types, objects or resource strings.
| var csharpMessage = new LocalizableResourceString(nameof(AnalyzersResources.Convert_typeof_to_nameof), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)); | ||
| CSharpDescriptor = CreateDescriptorWithId(DescriptorId, csharpMessage, csharpMessage); | ||
| } | ||
| internal DiagnosticDescriptor VBDescriptor => Descriptor; |
There was a problem hiding this comment.
Both this and below property must be removed.
| { | ||
| } | ||
|
|
||
| protected override string GetCodeFixTitle(string visualbasic, string csharp) |
There was a problem hiding this comment.
This should be changed so this fixer sub-type returns the correct resource string instead of base type being aware about supported languages.
jmarolf
force-pushed
the
feature/convertNameOf
branch
from
35d0198
to
9354fc0
Compare
jmarolf
force-pushed
the
feature/convertNameOf
branch
from
6459df0
to
10d7b26
Compare
ghost
added this to the 
|
Yay! Great job @m-redding @sigmachirality @JavierMatosD! |
CC: @JavierMatosD @sigmachirality
This feature offers an analyzer and code fix for issue #44162
It reports diagnostics and offers refactorings for relevant instances of typeof(SomeType).Name, to be changed to nameof(SomeType). It does not offer the refactoring on generic types because they are not semantically equivalent. In cases of system type keywords (int, string, etc), the code fix changes the keyword to the System.Type, because they are semantically equivalent, and the nameof() method does not accept keyword arguments.
See the attached document for more information.
convertNameOfSpec.docx