Merge pull request #870 from icsharpcode/vs2019

Vs2019

Author: GrahamTheCoder

CodeConverter.sln

@@ -1,16 +1,13 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio Version 16
-VisualStudioVersion = 16.0.29721.120
+# Visual Studio Version 17
+VisualStudioVersion = 17.1.31911.260
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "CodeConverter", "CodeConverter\CodeConverter.csproj", "{7EA075C6-6406-445C-AB77-6C47AFF88D58}"
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Tests", "Tests\Tests.csproj", "{21DBA1CE-AF55-4159-B04B-B8C621BE8921}"
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Vsix", "Vsix\Vsix.csproj", "{99498EF8-C9E0-433B-8D7B-EA8E9E66F0C7}"
-	ProjectSection(ProjectDependencies) = postProject
-		{CF18CBCF-67FF-46CA-8D1D-3221B7706D7D} = {CF18CBCF-67FF-46CA-8D1D-3221B7706D7D}
-	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{40A4828C-D6D2-43C5-A452-36CB06649680}"
 	ProjectSection(SolutionItems) = preProject

CodeConverter/CSharp/CommentConvertingVisitorWrapper.cs

@@ -1,28 +1,26 @@
-using Microsoft.CodeAnalysis.CSharp;
-using Microsoft.CodeAnalysis.Text;
-using Microsoft.CodeAnalysis.VisualBasic;
+using Microsoft.CodeAnalysis.Text;
 
 namespace ICSharpCode.CodeConverter.CSharp;
 
 [System.Diagnostics.DebuggerStepThrough]
 internal class CommentConvertingVisitorWrapper
 {
-    private readonly VBasic.VisualBasicSyntaxVisitor<Task<CS.CSharpSyntaxNode>> _wrappedVisitor;
+    private readonly VBasic.VisualBasicSyntaxVisitor<Task<CSharpSyntaxNode>> _wrappedVisitor;
     private readonly SyntaxTree _syntaxTree;
     private static readonly CSSyntax.LiteralExpressionSyntax _dummyLiteral = ValidSyntaxFactory.DefaultExpression;
     private static readonly CSSyntax.StatementSyntax _dummyStatement = CS.SyntaxFactory.EmptyStatement();
     private static readonly CSSyntax.CompilationUnitSyntax _dummyCompilationUnit = CS.SyntaxFactory.CompilationUnit();
 
-    public CommentConvertingVisitorWrapper(VisualBasicSyntaxVisitor<Task<CSharpSyntaxNode>> wrappedVisitor, SyntaxTree syntaxTree)
+    public CommentConvertingVisitorWrapper(VBasic.VisualBasicSyntaxVisitor<Task<CSharpSyntaxNode>> wrappedVisitor, SyntaxTree syntaxTree)
     {
         _wrappedVisitor = wrappedVisitor;
         _syntaxTree = syntaxTree;
     }
 
-    public async Task<T> AcceptAsync<T>(VisualBasicSyntaxNode vbNode, SourceTriviaMapKind sourceTriviaMap) where T : CS.CSharpSyntaxNode =>
+    public async Task<T> AcceptAsync<T>(VisualBasicSyntaxNode vbNode, SourceTriviaMapKind sourceTriviaMap) where T : CSharpSyntaxNode =>
         await ConvertHandledAsync<T>(vbNode, sourceTriviaMap);
 
-    public async Task<SeparatedSyntaxList<TOut>> AcceptAsync<TIn, TOut>(SeparatedSyntaxList<TIn> vbNodes, SourceTriviaMapKind sourceTriviaMap) where TIn : VBasic.VisualBasicSyntaxNode where TOut : CS.CSharpSyntaxNode
+    public async Task<SeparatedSyntaxList<TOut>> AcceptAsync<TIn, TOut>(SeparatedSyntaxList<TIn> vbNodes, SourceTriviaMapKind sourceTriviaMap) where TIn : VisualBasicSyntaxNode where TOut : CSharpSyntaxNode
     {
         var convertedNodes = await vbNodes.SelectAsync(n => ConvertHandledAsync<TOut>(n, sourceTriviaMap));
         var convertedSeparators = vbNodes.GetSeparators().Select(s =>
@@ -33,7 +31,7 @@ public async Task<SeparatedSyntaxList<TOut>> AcceptAsync<TIn, TOut>(SeparatedSyn
         return CS.SyntaxFactory.SeparatedList(convertedNodes, convertedSeparators);
     }
 
-    private async Task<T> ConvertHandledAsync<T>(VisualBasicSyntaxNode vbNode, SourceTriviaMapKind sourceTriviaMap) where T : CS.CSharpSyntaxNode
+    private async Task<T> ConvertHandledAsync<T>(VisualBasicSyntaxNode vbNode, SourceTriviaMapKind sourceTriviaMap) where T : CSharpSyntaxNode
     {
         try {
             var converted = (T)await _wrappedVisitor.Visit(vbNode);
@@ -56,7 +54,7 @@ private async Task<T> ConvertHandledAsync<T>(VisualBasicSyntaxNode vbNode, Sourc
     /// <remarks>
     /// If lots of special cases, move to wrapping the wrappedVisitor in another visitor, but I'd rather use a simple switch here initially.
     /// </remarks>
-    private static T WithSourceMapping<T>(SyntaxNode vbNode, T converted) where T : CS.CSharpSyntaxNode
+    private static T WithSourceMapping<T>(SyntaxNode vbNode, T converted) where T : CSharpSyntaxNode
     {
         converted = vbNode.CopyAnnotationsTo(converted);
         switch (vbNode) {

CodeConverter/CSharp/CommonConversions.cs

@@ -75,7 +75,7 @@ public CommonConversions(Document document, SemanticModel semanticModel,
         foreach (var name in declarator.Names) {
 
             var declaredSymbol = SemanticModel.GetDeclaredSymbol(name);
-            if (symbolsToSkip?.Contains(declaredSymbol) == true) continue;
+            if (symbolsToSkip?.Contains(declaredSymbol, SymbolEqualityComparer.IncludeNullability) == true) continue;
             var declaredSymbolType = declaredSymbol.GetSymbolType();
             var equalsValueClauseSyntax = await ConvertEqualsValueClauseSyntaxAsync(declarator, name, vbInitValue, declaredSymbolType, declaredSymbol, initializerOrMethodDecl);
             var v = SyntaxFactory.VariableDeclarator(ConvertIdentifier(name.Identifier), null, equalsValueClauseSyntax);
@@ -107,7 +107,7 @@ public bool ShouldPreferExplicitType(VBSyntax.ExpressionSyntax exp,
         exp = op.Syntax as VBSyntax.ExpressionSyntax;
         var vbInitConstantValue = SemanticModel.GetConstantValue(exp);
         isNothingLiteral = vbInitConstantValue.HasValue && vbInitConstantValue.Value == null || exp is VBSyntax.LiteralExpressionSyntax les && les.IsKind(SyntaxKind.NothingLiteralExpression);
-        bool shouldPreferExplicitType = expConvertedType != null && (expConvertedType.HasCsKeyword() || !expConvertedType.Equals(op.Type));
+        bool shouldPreferExplicitType = expConvertedType != null && (expConvertedType.HasCsKeyword() || !expConvertedType.Equals(op.Type, SymbolEqualityComparer.IncludeNullability));
         return shouldPreferExplicitType;
     }
 
@@ -172,7 +172,7 @@ private CSSyntax.VariableDeclarationSyntax CreateVariableDeclaration(VariableDec
         CSSyntax.EqualsValueClauseSyntax equalsValueClauseSyntax, IMethodSymbol initSymbol, CSSyntax.VariableDeclaratorSyntax v)
     {
         var requireExplicitType = requireExplicitTypeForAll ||
-                                  vbInitializerType != null && !Equals(declaredSymbolType, vbInitializerType);
+                                  vbInitializerType != null && !SymbolEqualityComparer.IncludeNullability.Equals(declaredSymbolType, vbInitializerType);
         bool useVar = equalsValueClauseSyntax != null && !preferExplicitType && !requireExplicitType;
         var typeSyntax = initSymbol == null || !initSymbol.IsAnonymousFunction()
             ? GetTypeSyntax(declaredSymbolType, useVar)
@@ -267,7 +267,7 @@ public SyntaxToken ConvertIdentifier(SyntaxToken id, bool isAttribute = false, S
                     text = idSymbol.ContainingType.Name;
                     if (normalizedText.EndsWith("Attribute", StringComparison.OrdinalIgnoreCase))
                         text = text.Remove(text.Length - "Attribute".Length);
-                } else if (idSymbol.IsKind(SymbolKind.Parameter) && idSymbol.ContainingSymbol.IsAccessorWithValueInCsharp() && ((idSymbol.IsImplicitlyDeclared && idSymbol.Name.WithHalfWidthLatinCharacters().Equals("value", StringComparison.OrdinalIgnoreCase)) || idSymbol.Equals(idSymbol.ContainingSymbol.GetParameters().FirstOrDefault(x => !x.IsImplicitlyDeclared)))) {
+                } else if (idSymbol.IsKind(SymbolKind.Parameter) && idSymbol.ContainingSymbol.IsAccessorWithValueInCsharp() && ((idSymbol.IsImplicitlyDeclared && idSymbol.Name.WithHalfWidthLatinCharacters().Equals("value", StringComparison.OrdinalIgnoreCase)) || idSymbol.Equals(idSymbol.ContainingSymbol.GetParameters().FirstOrDefault(x => !x.IsImplicitlyDeclared), SymbolEqualityComparer.IncludeNullability))) {
                     // The case above is basically that if the symbol is a parameter, and the corresponding definition is a property set definition
                     // AND the first explicitly declared parameter is this symbol, we need to replace it with value.
                     text = "value";

CodeConverter/CSharp/DefiniteAssignmentAnalyzer.cs

@@ -7,6 +7,6 @@ public static bool IsDefinitelyAssignedBeforeRead(ISymbol localSymbol, DataFlowA
     {
         if (!methodFlow.ReadInside.Contains(localSymbol)) return true;
         var unassignedVariables = methodFlow.GetVbUnassignedVariables();
-        return unassignedVariables != null && !unassignedVariables.Contains(localSymbol);
+        return unassignedVariables != null && !unassignedVariables.Contains(localSymbol, SymbolEqualityComparer.IncludeNullability);
     }
 }

CodeConverter/CSharp/ExpressionNodeVisitor.cs

@@ -1,6 +1,7 @@
 using System.Collections.Immutable;
 using System.Data;
 using System.Globalization;
+using System.Linq;
 using ICSharpCode.CodeConverter.CSharp.Replacements;
 using ICSharpCode.CodeConverter.Util.FromRoslyn;
 using Microsoft.CodeAnalysis.CSharp;
@@ -70,9 +71,13 @@ private static Dictionary<ITypeSymbol, string> CreateConvertMethodsLookupByRetur
 
         var convertMethods = convertType.GetMembers().Where(m =>
             m.Name.StartsWith("To", StringComparison.Ordinal) && m.GetParameters().Length == 1);
+
+#pragma warning disable RS1024 // Compare symbols correctly - GroupBy and ToDictionary use the same logic to dedupe as to lookup, so it doesn't matter which equality is used
         var methodsByType = convertMethods
             .GroupBy(m => new { ReturnType = m.GetReturnType(), Name = $"{ConvertType.FullName}.{m.Name}" })
             .ToDictionary(m => m.Key.ReturnType, m => m.Key.Name);
+#pragma warning restore RS1024 // Compare symbols correctly
+
         return methodsByType;
     }
 
@@ -519,7 +524,7 @@ private ExpressionSyntax HoistByRefDeclaration(VBSyntax.SimpleArgumentSyntax nod
     {
         string prefix = $"arg{argName}";
         var expressionTypeInfo = _semanticModel.GetTypeInfo(node.Expression);
-        bool useVar = expressionTypeInfo.Type?.Equals(expressionTypeInfo.ConvertedType) == true && !CommonConversions.ShouldPreferExplicitType(node.Expression, expressionTypeInfo.ConvertedType, out var _);
+        bool useVar = expressionTypeInfo.Type?.Equals(expressionTypeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability) == true && !CommonConversions.ShouldPreferExplicitType(node.Expression, expressionTypeInfo.ConvertedType, out var _);
         var typeSyntax = CommonConversions.GetTypeSyntax(expressionTypeInfo.ConvertedType, useVar);
 
         if (refLValue is ElementAccessExpressionSyntax eae) {
@@ -866,7 +871,7 @@ public override async Task<CSharpSyntaxNode> VisitBinaryExpression(VBasic.Syntax
         }
 
         omitConversion |= lhsTypeInfo.Type != null && rhsTypeInfo.Type != null &&
-                          lhsTypeInfo.Type.IsEnumType() && Equals(lhsTypeInfo.Type, rhsTypeInfo.Type)
+                          lhsTypeInfo.Type.IsEnumType() && SymbolEqualityComparer.IncludeNullability.Equals(lhsTypeInfo.Type, rhsTypeInfo.Type)
                           && !node.IsKind(VBasic.SyntaxKind.AddExpression, VBasic.SyntaxKind.SubtractExpression, VBasic.SyntaxKind.MultiplyExpression, VBasic.SyntaxKind.DivideExpression, VBasic.SyntaxKind.IntegerDivideExpression, VBasic.SyntaxKind.ModuloExpression)
                           && forceLhsTargetType == null && forceRhsTargetType == null;
         lhs = omitConversion ? lhs : CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Left, lhs, forceTargetType: forceLhsTargetType);
@@ -1032,7 +1037,7 @@ private static bool IsElementAtOrDefaultInvocation(ISymbol invocationSymbol, ISy
     {
         return (expressionSymbol != null
                 && (invocationSymbol?.Name == nameof(Enumerable.ElementAtOrDefault)
-                    && !expressionSymbol.Equals(invocationSymbol)));
+                    && !expressionSymbol.Equals(invocationSymbol, SymbolEqualityComparer.IncludeNullability)));
     }
 
     private ExpressionSyntax GetElementAtOrDefaultExpression(ISymbol expressionType,
@@ -1671,7 +1676,7 @@ RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expression)
                 IsRefArrayAcces(expression)) {
 
                 var typeInfo = _semanticModel.GetTypeInfo(expression);
-                bool isTypeMismatch = typeInfo.Type == null || !typeInfo.Type.Equals(typeInfo.ConvertedType);
+                bool isTypeMismatch = typeInfo.Type == null || !typeInfo.Type.Equals(typeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability);
 
                 if (isTypeMismatch) {
                     return RefConversion.PreAndPostAssignment;

CodeConverter/CSharp/HandledEventsAnalysis.cs

@@ -68,7 +68,7 @@ public IEnumerable<MethodDeclarationSyntax> CreateDelegatingMethodsRequiredByIni
     public IEnumerable<MemberDeclarationSyntax> GetDeclarationsForHandlingBaseMembers()
     {
         return _handlingMethodsByPropertyName.Values
-            .Where(m => m.EventContainer.Kind == EventContainerKind.Property && !_type.Equals(m.PropertyDetails.Property?.ContainingType))
+            .Where(m => m.EventContainer.Kind == EventContainerKind.Property && !_type.Equals(m.PropertyDetails.Property?.ContainingType, SymbolEqualityComparer.IncludeNullability))
             .Select(x => GetDeclarationsForHandlingBaseMembers(x));
     }
 

CodeConverter/CSharp/HandledEventsAnalyzer.cs

@@ -30,17 +30,19 @@ public static Task<HandledEventsAnalysis> AnalyzeAsync(CommonConversions commonC
 
     private async Task<HandledEventsAnalysis> AnalyzeAsync()
     {
+#pragma warning disable RS1024 // Compare symbols correctly - bug in analyzer https://github.com/dotnet/roslyn-analyzers/issues/3427#issuecomment-929104517
         var ancestorPropsMembersByName = _type.GetBaseTypesAndThis().SelectMany(t => t.GetMembers().Where(m => m.IsKind(SymbolKind.Property)))
             .GroupBy(m => m.Name, StringComparer.OrdinalIgnoreCase)
             .ToDictionary(m => m.Key, g => g.First(), StringComparer.OrdinalIgnoreCase); // Uses the fact that GroupBy maintains addition order to get the closest declaration
+#pragma warning restore RS1024 // Compare symbols correctly
 
         var writtenWithEventsProperties = await ancestorPropsMembersByName.Values.OfType<IPropertySymbol>().ToAsyncEnumerable().ToDictionaryAwaitAsync(async p => p.Name, async p => (p, await IsNeverWrittenOrOverriddenAsync(p)), StringComparer.OrdinalIgnoreCase);
 
         var eventContainerToMethods = _type.GetMembers().OfType<IMethodSymbol>()
             .SelectMany(HandledEvents)
             .ToLookup(eventAndMethod => eventAndMethod.EventContainer);
         var eventsThatMayNeedInheritors = writtenWithEventsProperties
-            .Where(p => !p.Value.Item2 && p.Value.p.ContainingType.Equals(_type))
+            .Where(p => !p.Value.Item2 && p.Value.p.ContainingType.Equals(_type, SymbolEqualityComparer.IncludeNullability))
             .Select(p =>
                 (EventContainer: new HandledEventsAnalysis.EventContainer(HandledEventsAnalysis.EventContainerKind.Property, p.Key), PropertyDetails: p.Value, Array.Empty<(EventDescriptor Event, IMethodSymbol HandlingMethod, int ParametersToDiscard)>()))
             .Where(e => !eventContainerToMethods.Contains(e.EventContainer));

CodeConverter/CSharp/LocalVariableAnalyzer.cs

@@ -9,7 +9,9 @@ internal static class LocalVariableAnalyzer
     public static async Task<HashSet<ILocalSymbol>> GetDescendantsToInlineInLoopAsync(this Solution solution, SemanticModel semanticModel, VisualBasicSyntaxNode methodNode)
     {
         var forEachControlVariables = await methodNode.DescendantNodes().OfType<ForEachBlockSyntax>().SelectAsync(forEach => GetLoopVariablesToInlineAsync(solution, semanticModel, forEach));
-        return new HashSet<ILocalSymbol>(forEachControlVariables.Where(f => f != null), SymbolEquivalenceComparer.Instance);
+#pragma warning disable RS1024 // Compare symbols correctly - analyzer bug, this is the comparer the docs recommend
+        return new HashSet<ILocalSymbol>(forEachControlVariables.Where(f => f != null), SymbolEqualityComparer.IncludeNullability);
+#pragma warning restore RS1024 // Compare symbols correctly
     }
 
     private static async Task<ILocalSymbol> GetLoopVariablesToInlineAsync(Solution solution, SemanticModel semanticModel, ForEachBlockSyntax block)

CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs

@@ -662,7 +662,7 @@ public override async Task<SyntaxList<StatementSyntax>> VisitForEachBlock(VBSynt
             var explicitCastWouldHaveNoEffect = variableType?.SpecialType == SpecialType.System_Object || _semanticModel.GetTypeInfo(stmt.Expression).ConvertedType.IsEnumerableOfExactType(variableType);
             type = CommonConversions.GetTypeSyntax(varSymbol.GetSymbolType(), explicitCastWouldHaveNoEffect);
             var v = await stmt.ControlVariable.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
-            if (_localsToInlineInLoop.Contains(varSymbol) && v is IdentifierNameSyntax vId) {
+            if (_localsToInlineInLoop.Contains(varSymbol, SymbolEqualityComparer.IncludeNullability) && v is IdentifierNameSyntax vId) {
                 id = vId.Identifier;
             } else {
                 id = CommonConversions.CsEscapedIdentifier(GetUniqueVariableNameInScope(node, "current" + varSymbol.Name.ToPascalCase()));

CodeConverter/CSharp/ProjectMergedDeclarationExtensions.cs

@@ -63,7 +63,7 @@ public static async Task<Project> WithRenamedMergedMyNamespaceAsync(this Project
         var projectDir = Path.Combine(vbProject.GetDirectoryPath(), "My Project");
 
         var compilation = await vbProject.GetCompilationAsync(cancellationToken);
-        var embeddedSourceTexts = await GetAllEmbeddedSourceText(compilation).Select((r, i) => (Text: r, Suffix: $".Static.{i+1}")).ToArrayAsync(cancellationToken);
+        var embeddedSourceTexts = await GetAllEmbeddedSourceTextAsync(compilation).Select((r, i) => (Text: r, Suffix: $".Static.{i+1}")).ToArrayAsync(cancellationToken);
         var generatedSourceTexts = (Text: await GetDynamicallyGeneratedSourceTextAsync(compilation), Suffix: ".Dynamic").Yield();
 
         foreach (var (text, suffix) in embeddedSourceTexts.Concat(generatedSourceTexts)) {
@@ -79,7 +79,7 @@ private static Project WithRenamespacedDocument(string baseName, Project vbProje
         return vbProject.AddDocument(baseName, sourceText.ReNamespace(), filePath: Path.Combine(myProjectDirPath, baseName + ".Designer.vb")).Project;
     }
 
-    private static async IAsyncEnumerable<string> GetAllEmbeddedSourceText(Compilation compilation)
+    private static async IAsyncEnumerable<string> GetAllEmbeddedSourceTextAsync(Compilation compilation)
     {
         var roots = await compilation.SourceModule.GlobalNamespace.Locations.
             Where(l => !l.IsInSource).Select(CachedReflectedDelegates.GetEmbeddedSyntaxTree)