Skip to content

Offer use-collection-expr when targeting interfaces#71373

Merged
CyrusNajmabadi merged 19 commits intodotnet:mainfrom
CyrusNajmabadi:collectionExprInterfaces
Dec 21, 2023
Merged

Offer use-collection-expr when targeting interfaces#71373
CyrusNajmabadi merged 19 commits intodotnet:mainfrom
CyrusNajmabadi:collectionExprInterfaces

Conversation

@CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Dec 20, 2023

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:

dotnet_style_prefer_collection_expression=never|when_types_strictly_match|when_types_loosely_match

The existing false|true values for this option map to never and when_types_strictly_match respectively, to preserve behavior with anyone who has explicitly set this.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner December 20, 2023 22:27
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 20, 2023
// 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new case we allow. though we let the caller now this may change semantics.

ImplicitArrayCreationExpressionSyntax arrayCreation
=> CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.TryGetMatches(
semanticModel, arrayCreation, expressionType, cancellationToken),
semanticModel, arrayCreation, expressionType, allowInterfaceConversion: true, cancellationToken, out _),
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Dec 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@CyrusNajmabadi CyrusNajmabadi merged commit 99e1c4c into dotnet:main Dec 21, 2023
@ghost ghost added this to the Next milestone Dec 21, 2023
@CyrusNajmabadi CyrusNajmabadi deleted the collectionExprInterfaces branch December 21, 2023 23:53
@Cosifne Cosifne modified the milestones: Next, 17.9 P3 Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Offer 'use collection expressions' even in locations that change semantics

3 participants