Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
94f493c
Remove async/await
CyrusNajmabadi Feb 11, 2025
a95b627
Fix formatting in tests
CyrusNajmabadi Mar 3, 2025
16598c5
Collection expr
CyrusNajmabadi Mar 3, 2025
4f245b5
Collection expr
CyrusNajmabadi Mar 3, 2025
64b0aa5
Make selaed
CyrusNajmabadi Mar 3, 2025
ea67ea0
Make selaed
CyrusNajmabadi Mar 3, 2025
19af62f
Primary constructors
CyrusNajmabadi Mar 3, 2025
73abcff
Fix formatting in tests (#77394)
CyrusNajmabadi Mar 3, 2025
87f8358
Remove async/await (#77397)
CyrusNajmabadi Mar 3, 2025
562c455
Fix out of bounds in text extent computation
CyrusNajmabadi Mar 3, 2025
0e4da9f
Fix out of bounds in text extent computation
CyrusNajmabadi Mar 3, 2025
008b61f
Simplify keyword recommenders. (#77396)
CyrusNajmabadi Mar 3, 2025
c57eb1c
Fix out of bounds in text extent computation (#77402)
CyrusNajmabadi Mar 3, 2025
feefbef
Fix issue where goto-def wouldn't go to the right place after invokin…
CyrusNajmabadi Mar 4, 2025
913e8cb
[main] Update dependencies from dotnet/source-build-reference-package…
dotnet-maestro[bot] Mar 4, 2025
22e54b3
Fix issue where goto-def wouldn't go to the right place after invokin…
CyrusNajmabadi Mar 4, 2025
de4bce1
Detect more null check patterns
CyrusNajmabadi Mar 4, 2025
26e760c
Emit call to ThrowIfNull
CyrusNajmabadi Mar 4, 2025
0d58692
Add string checks
CyrusNajmabadi Mar 4, 2025
41ff5e4
Add string checks
CyrusNajmabadi Mar 4, 2025
f6489ba
Add work items
CyrusNajmabadi Mar 4, 2025
37bbd28
Improve performance of SyntaxReplacer.ReplaceNode (#77314)
ToddGrun Mar 4, 2025
d28cba2
Detect and emit more idiomatic null check patterns (#77412)
CyrusNajmabadi Mar 4, 2025
0b4ebc1
Fix issue where we were changing semantics when converting to a colle…
CyrusNajmabadi Mar 4, 2025
50d57d7
Work item
CyrusNajmabadi Mar 4, 2025
d87dbf4
Update test
CyrusNajmabadi Mar 4, 2025
4a0428b
Update formatting tests
CyrusNajmabadi Mar 4, 2025
c9481b2
File scoped namespace
CyrusNajmabadi Mar 4, 2025
77b0c7d
Fix projects
CyrusNajmabadi Mar 4, 2025
7809289
Update formatting tests (#77418)
CyrusNajmabadi Mar 4, 2025
37ec9cd
Fix issue where we were changing semantics when converting to a colle…
CyrusNajmabadi Mar 4, 2025
8e3fb6a
Configure main-vs-deps (#77408)
jjonescz Mar 5, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions .github/workflows/main-merge.yml
Original file line number Diff line number Diff line change
@@ -1,18 +1,31 @@
# Merges any changes from release/prerelease to main (e.g. servicing changes)
# See https://github.com/dotnet/arcade/blob/e52018a/Documentation/Maestro/New-Inter-Branch-Merge-Approach.md

name: Flow main to release/dev18.0
name: Inter-branch merge
on:
schedule:
# once a day at 13:00 UTC to cleanup old runs
- cron: '0 13 * * *'
# Create a merge every 3 hours (works only for merges from `main`, others would need a `push` trigger).
- cron: '0 */3 * * *'
workflow_dispatch:
inputs:
configuration_file_branch:
description: 'Branch to use for configuration file'
required: true
default: 'main'

permissions:
contents: write
pull-requests: write

jobs:
check-script:
# The config does not support multiple flows from the same source branch,
# so we need to run separately for each duplicate source branch (https://github.com/dotnet/arcade/issues/15586).
merge:
uses: dotnet/arcade/.github/workflows/inter-branch-merge-base.yml@main
with:
configuration_file_path: '.config/branch-merge.json'
configuration_file_path: 'eng/config/branch-merge.jsonc'
configuration_file_branch: ${{ inputs.configuration_file_branch || 'main' }}
merge-2:
uses: dotnet/arcade/.github/workflows/inter-branch-merge-base.yml@main
with:
configuration_file_path: 'eng/config/branch-merge-2.jsonc'
configuration_file_branch: ${{ inputs.configuration_file_branch || 'main' }}
4 changes: 2 additions & 2 deletions eng/Version.Details.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
<SourceBuild RepoName="source-build-externals" ManagedOnly="true" />
</Dependency>
<!-- Intermediate is necessary for source build. -->
<Dependency Name="Microsoft.SourceBuild.Intermediate.source-build-reference-packages" Version="10.0.611901">
<Dependency Name="Microsoft.SourceBuild.Intermediate.source-build-reference-packages" Version="10.0.615303">
<Uri>https://github.com/dotnet/source-build-reference-packages</Uri>
<Sha>d794781cc75921b4ebbefe2eabdb0d6cd1713005</Sha>
<Sha>964c28e59a72516a64d4c8ff06b797703c5cdbfd</Sha>
<SourceBuild RepoName="source-build-reference-packages" ManagedOnly="true" />
</Dependency>
<Dependency Name="System.CommandLine" Version="2.0.0-beta4.24528.1">
Expand Down
10 changes: 10 additions & 0 deletions eng/config/PublishData.json
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,16 @@
"insertionCreateDraftPR": true
},
"main": {
"nugetKind": [
"Shipping",
"NonShipping"
],
"vsBranch": "main",
"vsMajorVersion": 17,
"insertionCreateDraftPR": true,
"insertionTitlePrefix": "[d17.14 P3]"
},
"main-vs-deps": {
"nugetKind": [
"Shipping",
"NonShipping"
Expand Down
10 changes: 10 additions & 0 deletions eng/config/branch-merge-2.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Used by .github/workflows/main-merge.yml
{
"merge-flow-configurations": {
// Merge any main changes to main-vs-deps.
"main": {
"MergeToBranch": "main-vs-deps",
"ExtraSwitches": "-QuietComments"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Used by .github/workflows/main-merge.yml
{
"merge-flow-configurations": {
// Merge any main changes to release/dev18.0.
Expand All @@ -6,4 +7,4 @@
"ExtraSwitches": "-QuietComments"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ protected override bool HasExistingInvalidInitializerForCollection()
protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression(
ArrayBuilder<CollectionMatch<SyntaxNode>> preMatches,
ArrayBuilder<CollectionMatch<SyntaxNode>> postMatches,
out bool mayChangeSemantics,
CancellationToken cancellationToken)
{
mayChangeSemantics = false;

// Constructor wasn't called with any arguments. Nothing to validate.
var argumentList = _objectCreationExpression.ArgumentList;
if (argumentList is null || argumentList.Arguments.Count == 0)
Expand All @@ -63,20 +66,21 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre
if (this.SemanticModel.GetSymbolInfo(_objectCreationExpression, cancellationToken).Symbol is not IMethodSymbol
{
MethodKind: MethodKind.Constructor,
Parameters.Length: 1,
Parameters: [var firstParameter],
} constructor)
{
return false;
}

var ienumerableOfTType = this.SemanticModel.Compilation.IEnumerableOfTType();
var firstParameter = constructor.Parameters[0];
if (Equals(firstParameter.Type.OriginalDefinition, ienumerableOfTType) ||
firstParameter.Type.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType)))
if (CanSpreadFirstParameter(constructor.ContainingType, firstParameter))
{
// Took a single argument that implements IEnumerable<T>. We handle this by spreading that argument as the
// first thing added to the collection.
preMatches.Add(new(argumentList.Arguments[0].Expression, UseSpread: true));

// Can't be certain that spreading the elements will be the same as passing to the constructor. So pass
// that uncertainty up to the caller so they can inform the user.
mayChangeSemantics = true;
return true;
}
else if (firstParameter is { Type.SpecialType: SpecialType.System_Int32, Name: "capacity" })
Expand Down Expand Up @@ -195,5 +199,36 @@ protected override bool AnalyzeMatchesAndCollectionConstructorForCollectionExpre
}

return false;

bool CanSpreadFirstParameter(INamedTypeSymbol constructedType, IParameterSymbol firstParameter)
{
var compilation = this.SemanticModel.Compilation;

var ienumerableOfTType = compilation.IEnumerableOfTType();
if (!Equals(firstParameter.Type.OriginalDefinition, ienumerableOfTType) &&
!firstParameter.Type.AllInterfaces.Any(i => Equals(i.OriginalDefinition, ienumerableOfTType)))
{
return false;
}

// Looks like something passed to the constructor call that we could potentially spread instead. e.g. `new
// HashSet(someList)` can become `[.. someList]`. However, check for certain cases we know where this is
// wrong and we can't do this.

// BlockingCollection<T> and Collection<T> both take ownership of the collection passed to them. So adds to
// them will add through to the original collection. They do not take the original collection and add their
// elements to itself.

var collectionType = compilation.CollectionOfTType();
var blockingCollectionType = compilation.BlockingCollectionOfTType();
if (constructedType.GetBaseTypesAndThis().Any(
t => Equals(collectionType, t.OriginalDefinition) ||
Equals(blockingCollectionType, t.OriginalDefinition)))
{
return false;
}

return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.CodeAnalysis.CSharp.UseCollectionInitializer;
using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Testing;
using Roslyn.Test.Utilities;
using Xunit;

Expand Down Expand Up @@ -1826,4 +1827,42 @@ void M(List<int>? list1)
LanguageVersion = LanguageVersion.CSharp12,
}.RunAsync();
}

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

class A
{
public void Main(ConcurrentQueue<int> queue)
{
BlockingCollection<int> bc = [|new|](queue);
[|bc.Add(|]42);
}
}
""",
FixedCode = """
using System;
using System.Collections.Concurrent;

class A
{
public void Main(ConcurrentQueue<int> queue)
{
BlockingCollection<int> bc = new(queue)
{
42
};
}
}
""",
LanguageVersion = LanguageVersion.CSharp13,
ReferenceAssemblies = ReferenceAssemblies.Net.Net90,
}.RunAsync();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ internal abstract class AbstractObjectCreationExpressionAnalyzer<
{
public readonly record struct AnalysisResult(
ImmutableArray<TMatch> PreMatches,
ImmutableArray<TMatch> PostMatches);
ImmutableArray<TMatch> PostMatches,
bool ChangesSemantics);

protected UpdateExpressionState<TExpressionSyntax, TStatementSyntax> State;

Expand All @@ -48,7 +49,7 @@ public readonly record struct AnalysisResult(
protected SemanticModel SemanticModel => this.State.SemanticModel;

protected abstract bool ShouldAnalyze(CancellationToken cancellationToken);
protected abstract bool TryAddMatches(ArrayBuilder<TMatch> preMatches, ArrayBuilder<TMatch> postMatches, CancellationToken cancellationToken);
protected abstract bool TryAddMatches(ArrayBuilder<TMatch> preMatches, ArrayBuilder<TMatch> postMatches, out bool changesSemantics, CancellationToken cancellationToken);
protected abstract bool IsInitializerOfLocalDeclarationStatement(
TLocalDeclarationStatementSyntax localDeclarationStatement, TObjectCreationExpressionSyntax rootExpression, [NotNullWhen(true)] out TVariableDeclaratorSyntax? variableDeclarator);

Expand Down Expand Up @@ -87,10 +88,10 @@ protected AnalysisResult AnalyzeWorker(CancellationToken cancellationToken)

using var _1 = ArrayBuilder<TMatch>.GetInstance(out var preMatches);
using var _2 = ArrayBuilder<TMatch>.GetInstance(out var postMatches);
if (!TryAddMatches(preMatches, postMatches, cancellationToken))
if (!TryAddMatches(preMatches, postMatches, out var mayChangeSemantics, cancellationToken))
return default;

return new(preMatches.ToImmutableAndClear(), postMatches.ToImmutableAndClear());
return new(preMatches.ToImmutableAndClear(), postMatches.ToImmutableAndClear(), mayChangeSemantics);
}

protected UpdateExpressionState<TExpressionSyntax, TStatementSyntax>? TryInitializeState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ internal abstract class AbstractUseCollectionInitializerAnalyzer<
protected abstract bool IsComplexElementInitializer(SyntaxNode expression);
protected abstract bool HasExistingInvalidInitializerForCollection();
protected abstract bool AnalyzeMatchesAndCollectionConstructorForCollectionExpression(
ArrayBuilder<CollectionMatch<SyntaxNode>> preMatches, ArrayBuilder<CollectionMatch<SyntaxNode>> postMatches, CancellationToken cancellationToken);
ArrayBuilder<CollectionMatch<SyntaxNode>> preMatches,
ArrayBuilder<CollectionMatch<SyntaxNode>> postMatches,
out bool mayChangeSemantics,
CancellationToken cancellationToken);

protected abstract IUpdateExpressionSyntaxHelper<TExpressionSyntax, TStatementSyntax> SyntaxHelper { get; }

Expand All @@ -66,7 +69,7 @@ public AnalysisResult Analyze(
return default;

this.Initialize(state.Value, objectCreationExpression, analyzeForCollectionExpression);
var (preMatches, postMatches) = this.AnalyzeWorker(cancellationToken);
var (preMatches, postMatches, mayChangeSemantics) = this.AnalyzeWorker(cancellationToken);

// If analysis failed entirely, immediately bail out.
if (preMatches.IsDefault || postMatches.IsDefault)
Expand All @@ -81,15 +84,19 @@ public AnalysisResult Analyze(
// other words, we don't want to suggest changing `new List<int>()` to `new List<int>() { }` as that's just
// noise. So convert empty results to an invalid result here.
if (analyzeForCollectionExpression)
return new(preMatches, postMatches);
return new(preMatches, postMatches, mayChangeSemantics);

// Downgrade an empty result to a failure for the normal collection-initializer case.
return postMatches.IsEmpty ? default : new(preMatches, postMatches);
return postMatches.IsEmpty ? default : new(preMatches, postMatches, mayChangeSemantics);
}

protected sealed override bool TryAddMatches(
ArrayBuilder<CollectionMatch<SyntaxNode>> preMatches, ArrayBuilder<CollectionMatch<SyntaxNode>> postMatches, CancellationToken cancellationToken)
ArrayBuilder<CollectionMatch<SyntaxNode>> preMatches,
ArrayBuilder<CollectionMatch<SyntaxNode>> postMatches,
out bool mayChangeSemantics,
CancellationToken cancellationToken)
{
mayChangeSemantics = false;
var seenInvocation = false;
var seenIndexAssignment = false;

Expand Down Expand Up @@ -127,7 +134,10 @@ protected sealed override bool TryAddMatches(
}

if (_analyzeForCollectionExpression)
return AnalyzeMatchesAndCollectionConstructorForCollectionExpression(preMatches, postMatches, cancellationToken);
{
return AnalyzeMatchesAndCollectionConstructorForCollectionExpression(
preMatches, postMatches, out mayChangeSemantics, cancellationToken);
}

return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ public override DiagnosticAnalyzerCategory GetAnalyzerCategory()
isUnnecessary: true);

protected AbstractUseCollectionInitializerDiagnosticAnalyzer()
: base(
[
(s_descriptor, CodeStyleOptions2.PreferCollectionInitializer)
])
: base([(s_descriptor, CodeStyleOptions2.PreferCollectionInitializer)])
{
}

Expand Down Expand Up @@ -117,9 +114,9 @@ private void OnCompilationStart(CompilationStartAnalysisContext context)
// as a non-local diagnostic and would not participate in lightbulb for computing code fixes.
var expressionType = context.Compilation.ExpressionOfTType();
context.RegisterCodeBlockStartAction<TSyntaxKind>(blockStartContext =>
blockStartContext.RegisterSyntaxNodeAction(
nodeContext => AnalyzeNode(nodeContext, ienumerableType, expressionType),
matchKindsArray));
blockStartContext.RegisterSyntaxNodeAction(
nodeContext => AnalyzeNode(nodeContext, ienumerableType, expressionType),
matchKindsArray));
}

private void AnalyzeNode(
Expand Down Expand Up @@ -206,13 +203,13 @@ private void AnalyzeNode(
if (!preferInitializerOption.Value)
return null;

var (_, matches) = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: false, cancellationToken);
var (_, matches, changesSemantics) = analyzer.Analyze(semanticModel, syntaxFacts, objectCreationExpression, analyzeForCollectionExpression: false, cancellationToken);

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

return (matches, shouldUseCollectionExpression: false, changesSemantics: false);
return (matches, shouldUseCollectionExpression: false, changesSemantics);
}

(ImmutableArray<CollectionMatch<SyntaxNode>> matches, bool shouldUseCollectionExpression, bool changesSemantics)? GetCollectionExpressionMatches()
Expand All @@ -224,18 +221,18 @@ private void AnalyzeNode(
if (!this.AreCollectionExpressionsSupported(context.Compilation))
return null;

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

// If analysis failed, we can't change this, no matter what.
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, preMatches, allowSemanticsChange, cancellationToken, out var changesSemantics))
if (!CanUseCollectionExpression(semanticModel, objectCreationExpression, expressionType, preMatches, allowSemanticsChange, cancellationToken, out var changesSemantics2))
return null;

return (preMatches.Concat(postMatches), shouldUseCollectionExpression: true, changesSemantics);
return (preMatches.Concat(postMatches), shouldUseCollectionExpression: true, changesSemantics1 || changesSemantics2);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ protected sealed override bool ShouldAnalyze(CancellationToken cancellationToken
protected sealed override bool TryAddMatches(
ArrayBuilder<Match<TExpressionSyntax, TStatementSyntax, TMemberAccessExpressionSyntax, TAssignmentStatementSyntax>> preMatches,
ArrayBuilder<Match<TExpressionSyntax, TStatementSyntax, TMemberAccessExpressionSyntax, TAssignmentStatementSyntax>> postMatches,
out bool changesSemantics,
CancellationToken cancellationToken)
{
changesSemantics = false;
using var _1 = PooledHashSet<string>.GetInstance(out var seenNames);

var initializer = this.SyntaxFacts.GetInitializerOfBaseObjectCreationExpression(_objectCreationExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ protected sealed override async Task FixAsync(
using var analyzer = GetAnalyzer();

var useCollectionExpression = properties.ContainsKey(UseCollectionInitializerHelpers.UseCollectionExpressionName) is true;
var (preMatches, postMatches) = analyzer.Analyze(
var (preMatches, postMatches, _) = analyzer.Analyze(
semanticModel, syntaxFacts, objectCreation, useCollectionExpression, cancellationToken);

if (preMatches.IsDefault || postMatches.IsDefault)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseCollectionInitializer
Protected Overrides Function AnalyzeMatchesAndCollectionConstructorForCollectionExpression(
preMatches As ArrayBuilder(Of CollectionMatch(Of SyntaxNode)),
postMatches As ArrayBuilder(Of CollectionMatch(Of SyntaxNode)),
ByRef changesSemantics As Boolean,
cancellationToken As CancellationToken) As Boolean
' Only called for collection expressions, which VB does not support
Throw ExceptionUtilities.Unreachable()
Expand Down
Loading
Loading