Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into primaryConstructors
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi committed Jun 7, 2023
2 parents 9513cf5 + 78cc604 commit 9af8b0d
Show file tree
Hide file tree
Showing 34 changed files with 932 additions and 223 deletions.
4 changes: 2 additions & 2 deletions azure-pipelines-official.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ stages:
demands: ImageOverride -equals windows.vs2022.amd64

jobs:
- ${{ if eq(variables['Build.SourceBranch'], 'refs/heads/release/dev17.6-vs-deps') }}:
- ${{ if eq(variables['Build.SourceBranch'], 'refs/heads/release/dev17.7') }}:
- template: /eng/common/templates/job/onelocbuild.yml
parameters:
MirrorRepo: roslyn
MirrorBranch: release/dev17.6-vs-deps
MirrorBranch: release/dev17.7
LclSource: lclFilesfromPackage
LclPackageId: 'LCL-JUNO-PROD-ROSLYN'

Expand Down
15 changes: 8 additions & 7 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,14 @@ stages:
testArguments: --testCoreClr
queueName: Build.Ubuntu.1804.Amd64.Open

- template: eng/pipelines/test-unix-job.yml
parameters:
testRunName: 'Test macOS Debug'
jobName: Test_macOS_Debug
testArtifactName: Transport_Artifacts_Unix_Debug
configuration: Debug
testArguments: --testCoreClr --helixQueueName OSX.1015.Amd64.Open
- ${{ if ne(variables['Build.Reason'], 'PullRequest') }}:
- template: eng/pipelines/test-unix-job.yml
parameters:
testRunName: 'Test macOS Debug'
jobName: Test_macOS_Debug
testArtifactName: Transport_Artifacts_Unix_Debug
configuration: Debug
testArguments: --testCoreClr --helixQueueName OSX.1015.Amd64.Open

- stage: Correctness
dependsOn: []
Expand Down
78 changes: 39 additions & 39 deletions docs/Language Feature Status.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
**Introduced in VS 17.6**

As part of implementing VB support for consuming `required` APIs, it is now an error to manually apply `RequiredMembersAttribute` in
source. VB will now correctly interpret these attributes in metadata and allow instanciating types with required members.
source. VB will now correctly interpret these attributes in metadata and allow instantiating types with required members.
14 changes: 14 additions & 0 deletions eng/Version.Details.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,19 @@
<Uri>https://github.com/dotnet/roslyn-analyzers</Uri>
<Sha>31e5d2773251838851d273edba1babcf60321f24</Sha>
</Dependency>
<!-- Entries below are necessary for source-build. This allows the packages to be retrieved from previously-source-built
artifacts and flow in as dependencies of the packages produced by roslyn. -->
<Dependency Name="System.Composition" Version="7.0.0">
<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>d099f075e45d2aa6007a22b71b45a08758559f80</Sha>
</Dependency>
<Dependency Name="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4">
<Uri>https://github.com/dotnet/roslyn-analyzers</Uri>
<Sha>22ea6422f85b05ca0793cc3b76375487be407f5d</Sha>
</Dependency>
<Dependency Name="Microsoft.CodeAnalysis.AnalyzerUtilities" Version="3.3.0">
<Uri>https://github.com/dotnet/roslyn-analyzers</Uri>
<Sha>596f8f422003d89b65e2bd50dc5a1aea7ce4ce91</Sha>
</Dependency>
</ToolsetDependencies>
</Dependencies>
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<MajorVersion>4</MajorVersion>
<MinorVersion>7</MinorVersion>
<PatchVersion>0</PatchVersion>
<PreReleaseVersionLabel>2</PreReleaseVersionLabel>
<PreReleaseVersionLabel>3</PreReleaseVersionLabel>
<VersionPrefix>$(MajorVersion).$(MinorVersion).$(PatchVersion)</VersionPrefix>
<!--
By default the assembly version in official builds is "$(MajorVersion).$(MinorVersion).0.0".
Expand Down
6 changes: 3 additions & 3 deletions eng/config/PublishData.json
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
],
"vsBranch": "rel/d17.7",
"vsMajorVersion": 17,
"insertionTitlePrefix": "[d17.7P1]"
"insertionTitlePrefix": "[d17.7P2]"
},
"main": {
"nugetKind": [
Expand All @@ -164,8 +164,8 @@
],
"vsBranch": "main",
"vsMajorVersion": 17,
"insertionCreateDraftPR": false,
"insertionTitlePrefix": "[d17.7P2]"
"insertionCreateDraftPR": true,
"insertionTitlePrefix": "[d17.7P3]"
},
"dev/jorobich/fix-pr-val": {
"nugetKind": [
Expand Down
5 changes: 3 additions & 2 deletions eng/targets/GeneratePkgDef.targets
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@
Populates PkgDefBindingRedirect and PkgDefCodeBase items from references.
-->
<Target Name="_AddPkgDefEntriesFromReferences"
BeforeTargets="AfterResolveReferences;GeneratePkgDef">
BeforeTargets="GeneratePkgDef"
DependsOnTargets="ResolveReferences">
<ItemGroup>
<PkgDefBindingRedirect Include="@(ReferencePath)" Condition="'%(ReferencePath.PkgDefEntry)' == 'BindingRedirect'" />
<PkgDefCodeBase Include="@(ReferencePath)" Condition="'%(ReferencePath.PkgDefEntry)' == 'CodeBase'" />
Expand All @@ -155,7 +156,7 @@
<!--
Generates a .pkgdef file based on items in PkgDef* groups.
-->
<Target Name="GeneratePkgDef"
<Target Name="GeneratePkgDef"
DependsOnTargets="$(GeneratePkgDefDependsOn);_SetGeneratePkgDefInputsOutputs"
Inputs="$(MSBuildAllProjects);@(PkgDefFileContent)"
Outputs="$(_GeneratePkgDefOutputFile)">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.LanguageService;
using Microsoft.CodeAnalysis.CSharp.Shared.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
Expand Down Expand Up @@ -107,6 +108,17 @@ public static bool IsViableMemberToAssignTo(
if (nodeToRemove is not VariableDeclaratorSyntax and not PropertyDeclarationSyntax)
return false;

// If it's a property, then it has to be an auto property in order for us to be able to initialize is
// directly outside of a constructor.
if (nodeToRemove is PropertyDeclarationSyntax property)
{
if (property.AccessorList is null ||
property.AccessorList.Accessors.Any(static a => a.ExpressionBody != null || a.Body != null))
{
return false;
}
}

return true;
}

Expand Down Expand Up @@ -152,7 +164,9 @@ private void OnSymbolEnd(SymbolAnalysisContext context)
// Pass along a mapping of field/property name to the constructor parameter name that will replace it.
var properties = _candidateMembersToRemove
.Where(kvp => !_membersThatCannotBeRemoved.Contains(kvp.Key))
.ToImmutableDictionary(static kvp => kvp.Key.Name, static kvp => (string?)kvp.Value.Name);
.ToImmutableDictionary(
static kvp => kvp.Key.Name,
static kvp => (string?)kvp.Value.Name);

// To provide better user-facing-strings, keep track of whether or not all the members we'd be
// removing are all fields or all properties.
Expand Down Expand Up @@ -227,6 +241,9 @@ public static void AnalyzeNamedTypeStart(
if (primaryConstructorDeclaration.Parent is not TypeDeclarationSyntax)
return;

if (primaryConstructor.Parameters.Any(static p => p.RefKind is RefKind.Ref or RefKind.Out))
return;

// Now ensure the constructor body is something that could be converted to a primary constructor (i.e.
// only assignments to instance fields/props on this).
var semanticModel = compilation.GetSemanticModel(primaryConstructorDeclaration.SyntaxTree);
Expand All @@ -238,15 +255,16 @@ public static void AnalyzeNamedTypeStart(
}

var analyzer = new Analyzer(diagnosticAnalyzer, styleOption, primaryConstructor, primaryConstructorDeclaration, candidateMembersToRemove);
context.RegisterSymbolEndAction(analyzer.OnSymbolEnd);

// Look to see if we have trivial `_x = x` or `this.x = x` assignments. If so, then the field/prop
// could be a candidate for removal (as long as we determine that all use sites of the field/prop would
// be able to use the captured primary constructor parameter.
// be able to use the captured primary constructor parameter).

if (candidateMembersToRemove.Count > 0)
context.RegisterOperationAction(analyzer.AnalyzeFieldOrPropertyReference, OperationKind.FieldReference, OperationKind.PropertyReference);

context.RegisterSymbolEndAction(analyzer.OnSymbolEnd);

return;

bool TryFindPrimaryConstructorCandidate(
Expand Down Expand Up @@ -369,7 +387,8 @@ bool IsAssignmentToInstanceMember(
// If we have an assignment of the form `private_member = param`, then that member can be a candidate for removal.
if (member.DeclaredAccessibility == Accessibility.Private &&
!member.GetAttributes().Any() &&
semanticModel.GetSymbolInfo(assignmentExpression.Right, cancellationToken).GetAnySymbol() is IParameterSymbol parameter)
semanticModel.GetSymbolInfo(assignmentExpression.Right, cancellationToken).GetAnySymbol() is IParameterSymbol parameter &&
parameter.Type.Equals(member.GetMemberType()))
{
candidateMembersToRemove[member] = parameter;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.LanguageService;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editing;
Expand Down Expand Up @@ -171,9 +172,16 @@ private static async Task UsePrimaryConstructorAsync(
constructorDeclaration.AttributeLists.Select(
a => a.WithTarget(AttributeTargetSpecifier(Token(SyntaxKind.MethodKeyword))).WithoutTrivia().WithAdditionalAnnotations(Formatter.Annotation)));
var parameterList = RemoveElementIndentation(
typeDeclaration, constructorDeclaration, constructorDeclaration.ParameterList,
// When moving the parameter list from the constructor to the type, we will no longer have nested types
// in scope. So rewrite references to them if that's the case.
var parameterList = UpdateReferencesToNestedTypes(constructorDeclaration.ParameterList);
parameterList = RemoveElementIndentation(
typeDeclaration, constructorDeclaration, parameterList,
static list => list.Parameters);
parameterList = RemoveInModifierIfMemberIsRemoved(parameterList);
return currentTypeDeclaration
.WithLeadingTrivia(finalTrivia)
.WithAttributeLists(finalAttributeLists)
Expand All @@ -187,6 +195,46 @@ private static async Task UsePrimaryConstructorAsync(

return;

ParameterListSyntax RemoveInModifierIfMemberIsRemoved(ParameterListSyntax parameterList)
{
if (!removeMembers)
return parameterList;

return parameterList.ReplaceNodes(
parameterList.Parameters,
(_, current) =>
{
var inKeyword = current.Modifiers.FirstOrDefault(t => t.Kind() == SyntaxKind.InKeyword);
if (inKeyword == default)
return current;
// remove the 'in' modifier if we're removing the field. Captures can't refer to an in-parameter.
if (!properties.Values.Any(v => v == current.Identifier.ValueText))
return current;
return current.WithModifiers(current.Modifiers.Remove(inKeyword)).WithTriviaFrom(current);
});
}

ParameterListSyntax UpdateReferencesToNestedTypes(ParameterListSyntax parameterList)
{
return parameterList.ReplaceNodes(
parameterList.DescendantNodes().OfType<SimpleNameSyntax>(),
(typeSyntax, updatedTypeSyntax) =>
{
// Don't have to update if the type is already qualified.
if (typeSyntax.Parent is QualifiedNameSyntax qualifiedNameSyntax && qualifiedNameSyntax.Right == typeSyntax)
return updatedTypeSyntax;
var typeSymbol = semanticModel.GetTypeInfo(typeSyntax, cancellationToken).Type;
if (typeSymbol is not INamedTypeSymbol { ContainingType: not null } || !namedType.Equals(typeSymbol.ContainingType.OriginalDefinition))
return updatedTypeSyntax;
// reference to a nested type in an unqualified fashion. Have to qualify this.
return QualifiedName(namedType.GenerateNameSyntax(), updatedTypeSyntax);
});
}

static TListSyntax RemoveElementIndentation<TListSyntax>(
TypeDeclarationSyntax typeDeclaration,
ConstructorDeclarationSyntax constructorDeclaration,
Expand Down Expand Up @@ -318,7 +366,8 @@ async ValueTask RemoveMembersAsync()
continue;

removedMembers[member] = nodeToRemove;
await ReplaceReferencesToMemberWithParameterAsync(member, parameterName).ConfigureAwait(false);
await ReplaceReferencesToMemberWithParameterAsync(
member, CSharpSyntaxFacts.Instance.EscapeIdentifier(parameterName)).ConfigureAwait(false);
}

foreach (var group in removedMembers.Values.GroupBy(n => n.SyntaxTree))
Expand All @@ -345,7 +394,7 @@ async ValueTask RemoveMembersAsync()

async ValueTask ReplaceReferencesToMemberWithParameterAsync(ISymbol member, string parameterName)
{
var parameterNameNode = IdentifierName(parameterName);
var parameterNameNode = IdentifierName(ParseToken(parameterName));

// find all the references to member within this project. We can immediately filter down just to the
// documents containing our named type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Linq;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.LanguageService;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.PooledObjects;
Expand Down Expand Up @@ -80,7 +81,7 @@ static IEnumerable<XmlNodeSyntax> ConvertSummaryToParam(IEnumerable<XmlNodeSynta
foreach (var node in content)
{
yield return IsXmlElement(node, s_summaryTagName, out var xmlElement)
? ConvertXmlElementName(xmlElement, s_paramTagName).AddStartTagAttributes(XmlNameAttribute(parameterName))
? ConvertXmlElementName(xmlElement, s_paramTagName).AddStartTagAttributes(XmlNameAttribute(CSharpSyntaxFacts.Instance.EscapeIdentifier(parameterName)))
: node;
}
}
Expand Down
Loading

0 comments on commit 9af8b0d

Please sign in to comment.