Don't offer to simply to an alias if it has a different meaning#57001
Don't offer to simply to an alias if it has a different meaning#57001davidwengier merged 7 commits intodotnet:mainfrom
Conversation
src/Workspaces/CSharp/Portable/Simplification/Simplifiers/AbstractCSharpSimplifier.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/CSharp/Portable/Simplification/Simplifiers/AbstractCSharpSimplifier.cs
Outdated
Show resolved
Hide resolved
ryzngard
left a comment
There was a problem hiding this comment.
Looks reasonable. One question about implementation, but I honestly don't know of a scenario where it would break.
src/EditorFeatures/CSharpTest/SimplifyTypeNames/SimplifyTypeNamesTests.cs
Show resolved
Hide resolved
src/Workspaces/CSharp/Portable/Simplification/Simplifiers/AbstractCSharpSimplifier.cs
Outdated
Show resolved
Hide resolved
| var symbols = semanticModel.LookupSymbols(node.SpanStart, name: aliasName); | ||
| if (symbols.Any(s => !s.Equals(aliasReplacement))) | ||
| var aliasIdentifier = SyntaxFactory.IdentifierName(aliasName); | ||
| var typeInfo = semanticModel.GetSpeculativeTypeInfo(node.SpanStart, aliasIdentifier, SpeculativeBindingOption.BindAsExpression); |
There was a problem hiding this comment.
aliases can alias more than just types. they can also alias namespaces. i would get GetSpeculativeSymbolInfo and validate that the symbols are the same. note: plz add test for this as well :)
There was a problem hiding this comment.
Updated to check symbol info first, you were right namespaces were broken. Added a bunch of tests.
validate that the symbols are the same
so I'm still using TypeInfo too because the symbols don't have to be the same in all cases. Hopefully thats okay. I could avoid it and do the "long hand" way of checking for IPropertySymbol and then its type, or IParameterSymbol and its type etc. so let me know if thats preferrable.
Fixes #56882