Skip to content

Commit

Permalink
Fix case where we were erroneously offering to convert a dictionary t…
Browse files Browse the repository at this point in the history
…o use a collection expression. (#75897)
  • Loading branch information
CyrusNajmabadi authored Nov 14, 2024
2 parents 30cd2b3 + 0307d39 commit 2a67954
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.LanguageService;
Expand All @@ -10,10 +13,13 @@
using Microsoft.CodeAnalysis.CSharp.UseCollectionExpression;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.UseCollectionExpression;
using Microsoft.CodeAnalysis.UseCollectionInitializer;

namespace Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer;

using static SyntaxFactory;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal sealed class CSharpUseCollectionInitializerDiagnosticAnalyzer :
AbstractUseCollectionInitializerDiagnosticAnalyzer<
Expand All @@ -40,13 +46,39 @@ protected override bool AreCollectionInitializersSupported(Compilation compilati
protected override bool AreCollectionExpressionsSupported(Compilation compilation)
=> compilation.LanguageVersion().SupportsCollectionExpressions();

protected override bool CanUseCollectionExpression(SemanticModel semanticModel, BaseObjectCreationExpressionSyntax objectCreationExpression, INamedTypeSymbol? expressionType, bool allowSemanticsChange, CancellationToken cancellationToken, out bool changesSemantics)
protected override bool CanUseCollectionExpression(
SemanticModel semanticModel,
BaseObjectCreationExpressionSyntax objectCreationExpression,
INamedTypeSymbol? expressionType,
ImmutableArray<CollectionMatch<SyntaxNode>> preMatches,
bool allowSemanticsChange,
CancellationToken cancellationToken,
out bool changesSemantics)
{
// Synthesize the final collection expression we would replace this object-creation with. That will allow us to
// determine if we end up calling the right overload in cases of overloaded methods.
var replacement = UseCollectionExpressionHelpers.CreateReplacementCollectionExpressionForAnalysis(objectCreationExpression.Initializer);
var replacement = CollectionExpression(SeparatedList(
GetMatchElements(preMatches).Concat(GetInitializerElements(objectCreationExpression.Initializer))));

return UseCollectionExpressionHelpers.CanReplaceWithCollectionExpression(
semanticModel, objectCreationExpression, replacement, expressionType, isSingletonInstance: false, allowSemanticsChange, skipVerificationForReplacedNode: true, cancellationToken, out changesSemantics);

static IEnumerable<CollectionElementSyntax> GetMatchElements(ImmutableArray<CollectionMatch<SyntaxNode>> preMatches)
{
foreach (var match in preMatches)
{
if (match.Node is ExpressionSyntax expression)
yield return match.UseSpread ? SpreadElement(expression) : ExpressionElement(expression);
}
}

static IEnumerable<CollectionElementSyntax> GetInitializerElements(InitializerExpressionSyntax? initializer)
{
if (initializer != null)
{
foreach (var expression in initializer.Expressions)
yield return ExpressionElement(expression);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5866,4 +5866,26 @@ public class Class2 { }
ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
}.RunAsync();
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75894")]
public async Task TestNotOnDictionaryConstructor()
{
await new VerifyCS.Test
{
TestCode = """
using System.Collections.Generic;

class C
{
void Main()
{
Dictionary<string, string> a = null;
Dictionary<string, string> d = new(a);
}
}
""",
LanguageVersion = LanguageVersion.CSharp13,
ReferenceAssemblies = ReferenceAssemblies.Net.Net90,
}.RunAsync();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,13 @@ protected AbstractUseCollectionInitializerDiagnosticAnalyzer()
protected abstract bool AreCollectionInitializersSupported(Compilation compilation);
protected abstract bool AreCollectionExpressionsSupported(Compilation compilation);
protected abstract bool CanUseCollectionExpression(
SemanticModel semanticModel, TObjectCreationExpressionSyntax objectCreationExpression, INamedTypeSymbol? expressionType, bool allowSemanticsChange, CancellationToken cancellationToken, out bool changesSemantics);
SemanticModel semanticModel,
TObjectCreationExpressionSyntax objectCreationExpression,
INamedTypeSymbol? expressionType,
ImmutableArray<CollectionMatch<SyntaxNode>> preMatches,
bool allowSemanticsChange,
CancellationToken cancellationToken,
out bool changesSemantics);

protected abstract TAnalyzer GetAnalyzer();

Expand Down Expand Up @@ -218,18 +224,18 @@ private void AnalyzeNode(
if (!this.AreCollectionExpressionsSupported(context.Compilation))
return null;

var (_, matches) = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: true, cancellationToken);
var (preMatches, postMatches) = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: true, cancellationToken);

// If analysis failed, we can't change this, no matter what.
if (matches.IsDefault)
if (preMatches.IsDefault || postMatches.IsDefault)
return null;

// Check if it would actually be legal to use a collection expression here though.
var allowSemanticsChange = preferExpressionOption.Value == CollectionExpressionPreference.WhenTypesLooselyMatch;
if (!CanUseCollectionExpression(semanticModel, objectCreationExpression, expressionType, allowSemanticsChange, cancellationToken, out var changesSemantics))
if (!CanUseCollectionExpression(semanticModel, objectCreationExpression, expressionType, preMatches, allowSemanticsChange, cancellationToken, out var changesSemantics))
return null;

return (matches, shouldUseCollectionExpression: true, changesSemantics);
return (preMatches.Concat(postMatches), shouldUseCollectionExpression: true, changesSemantics);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
' The .NET Foundation licenses this file to you under the MIT license.
' See the LICENSE file in the project root for more information.

Imports System.Collections.Immutable
Imports System.Threading
Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.LanguageService
Imports Microsoft.CodeAnalysis.UseCollectionExpression
Imports Microsoft.CodeAnalysis.UseCollectionInitializer
Imports Microsoft.CodeAnalysis.VisualBasic.LanguageService
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Expand Down Expand Up @@ -38,7 +40,14 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer
Return False
End Function

Protected Overrides Function CanUseCollectionExpression(semanticModel As SemanticModel, objectCreationExpression As ObjectCreationExpressionSyntax, expressionType As INamedTypeSymbol, allowSemanticsChange As Boolean, cancellationToken As CancellationToken, ByRef changesSemantics As Boolean) As Boolean
Protected Overrides Function CanUseCollectionExpression(
semanticModel As SemanticModel,
objectCreationExpression As ObjectCreationExpressionSyntax,
expressionType As INamedTypeSymbol,
matches As ImmutableArray(Of CollectionMatch(Of SyntaxNode)),
allowSemanticsChange As Boolean,
cancellationToken As CancellationToken,
ByRef changesSemantics As Boolean) As Boolean
Throw ExceptionUtilities.Unreachable()
End Function
End Class
Expand Down

0 comments on commit 2a67954

Please sign in to comment.