Offer use-collection-expr when targeting interfaces #71373
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs
Outdated
Show resolved
Hide resolved
…ectionExpressionHelpers.cs
…jmabadi/roslyn into collectionExprInterfaces
| @@ -46,33 +46,38 @@ private void AnalyzeArrayCreationExpression(SyntaxNodeAnalysisContext context, I | |||
| return; | |||
|
|
|||
| // Analyze the statements that follow to see if they can initialize this array. | |||
| var matches = TryGetMatches(semanticModel, arrayCreationExpression, expressionType, cancellationToken); | |||
| var allowInterfaceConversion = context.GetAnalyzerOptions().PreferCollectionExpressionForInterfaces.Value; | |||
| var matches = TryGetMatches(semanticModel, arrayCreationExpression, expressionType, allowInterfaceConversion, cancellationToken, out var changesSemantics); | |||
There was a problem hiding this comment.
most of the change is getting hte info about if sematnics changed, passing it into the diagnostic, and presenting it in the fix.
| IsWellKnownInterface(convertedType) && | ||
| type.AllInterfaces.Contains(convertedType)) | ||
| { | ||
| changesSemantics = true; |
There was a problem hiding this comment.
the new case we allow. though we let the caller now this may change semantics.
| @@ -111,11 +105,11 @@ static bool IsOnSingleLine(SourceText sourceText, SyntaxNode node) | |||
| { | |||
| ImplicitArrayCreationExpressionSyntax arrayCreation | |||
| => CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.TryGetMatches( | |||
| semanticModel, arrayCreation, expressionType, cancellationToken), | |||
| semanticModel, arrayCreation, expressionType, allowInterfaceConversion: true, cancellationToken, out _), | |||
There was a problem hiding this comment.
during the fix portion, this value isn't relevant. we're already only being called on cases we already analyzed and have already decided the user wants to fix.
| [method: ImportingConstructor] | ||
| [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
| internal partial class CSharpUseCollectionExpressionForBuilderCodeFixProvider() | ||
| : AbstractUseCollectionExpressionCodeFixProvider<InvocationExpressionSyntax>( |
There was a problem hiding this comment.
new subclass which handles a bit of hte fix-all behavior (wrt to which diagnostics shoudl be fixed depending on if they change semantics or not).
| return true; | ||
|
|
||
| return !UseCollectionInitializerHelpers.ChangesSemantics(diagnostic); | ||
| } |
There was a problem hiding this comment.
logic for fix-all depending on if the user initially invoked fix-all on a fix that changes semantics or not. if they invoke it on one that doesn't, then we must only fix other diagnostics that don't change semantics. if they do invoke it on a fix that will change seamtnics, then we can fix any type of diagnostics (safe or unsafe).
...s/Core/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs
Outdated
Show resolved
Hide resolved
…UseCollectionInitializerCodeFixProvider.cs
...Analyzers/Core/CodeFixes/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs
Outdated
Show resolved
Hide resolved
…bjectInitializerCodeFixProvider.cs
ghost
added this to the 
Fixes #70996.
This will let the user know that semantics may change. There is an option to globally disable this and never offer 'use collection expr' in these cases.
By default, these loose semantics will be allowed.
A user can control this with the three options:
The existing
false|truevalues for this option map toneverandwhen_types_strictly_matchrespectively, to preserve behavior with anyone who has explicitly set this.