Skip to content

Commit

Permalink
Address PR feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
333fred committed Mar 15, 2022
1 parent 740c4ed commit 2ca360a
Show file tree
Hide file tree
Showing 17 changed files with 94 additions and 49 deletions.
42 changes: 25 additions & 17 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4986,13 +4986,15 @@ private static void CheckRequiredMembersInObjectInitializer(
return;
}

var requiredMembers = constructor.ContainingType.AllRequiredMembers.ToBuilder();
var requiredMembers = constructor.ContainingType.AllRequiredMembers;

if (requiredMembers.Count == 0)
{
return;
}

var requiredMembersBuilder = requiredMembers.ToBuilder();

if (initializers.IsDefaultOrEmpty)
{
reportMembers();
Expand All @@ -5001,24 +5003,28 @@ private static void CheckRequiredMembersInObjectInitializer(

foreach (var initializer in initializers)
{
var (memberSymbol, initializerExpression) = initializer is BoundAssignmentOperator assignmentOperator
? (assignmentOperator.Left switch
{
// Regular initializers
BoundObjectInitializerMember member => member.MemberSymbol,
// Attribute initializers
BoundPropertyAccess propertyAccess => propertyAccess.PropertySymbol,
BoundFieldAccess fieldAccess => fieldAccess.FieldSymbol,
_ => null
}, assignmentOperator.Right)
: default;
if (initializer is not BoundAssignmentOperator assignmentOperator)
{
continue;
}

var memberSymbol = assignmentOperator.Left switch
{
// Regular initializers
BoundObjectInitializerMember member => member.MemberSymbol,
// Attribute initializers
BoundPropertyAccess propertyAccess => propertyAccess.PropertySymbol,
BoundFieldAccess fieldAccess => fieldAccess.FieldSymbol,
// Error cases
_ => null
};

if (memberSymbol is null)
{
continue;
}

if (!requiredMembers.TryGetValue(memberSymbol.Name, out var requiredMember))
if (!requiredMembersBuilder.TryGetValue(memberSymbol.Name, out var requiredMember))
{
continue;
}
Expand All @@ -5028,12 +5034,12 @@ private static void CheckRequiredMembersInObjectInitializer(
continue;
}

requiredMembers.Remove(memberSymbol.Name);
requiredMembersBuilder.Remove(memberSymbol.Name);

if (initializerExpression is BoundObjectInitializerExpressionBase)
if (assignmentOperator.Right is BoundObjectInitializerExpressionBase initializerExpression)
{
// Required member '{0}' must be assigned a value, it cannot use a nested member or collection initializer.
diagnostics.Add(ErrorCode.ERR_RequiredMembersMustBeAssignment, initializerExpression.Syntax.Location, requiredMember);
diagnostics.Add(ErrorCode.ERR_RequiredMembersMustBeAssignedValue, initializerExpression.Syntax.Location, requiredMember);
}
}

Expand All @@ -5049,7 +5055,7 @@ void reportMembers()
_ => creationSyntax.Location
};

foreach (var (_, member) in requiredMembers)
foreach (var (_, member) in requiredMembersBuilder)
{
// Required member '{0}' must be set in the object initializer or attribute constructor.
diagnostics.Add(ErrorCode.ERR_RequiredMemberMustBeSet, location, member);
Expand Down Expand Up @@ -5847,6 +5853,8 @@ internal bool TryPerformConstructorOverloadResolution(
diagnostics.AddDependencies(useSiteInfo);
foreach (var diagnostic in useSiteInfo.Diagnostics)
{
// We don't want to report this error here because we'll report ERR_RequiredMembersBaseTypeInvalid. That error is suppressable by the
// user using the `SetsRequiredMembers` attribute on the constructor, so reporting this error would prevent that from working.
if ((ErrorCode)diagnostic.Code == ErrorCode.ERR_RequiredMembersInvalid)
{
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -7010,7 +7010,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_RequiredMemberMustBeSet" xml:space="preserve">
<value>Required member '{0}' must be set in the object initializer or attribute constructor.</value>
</data>
<data name="ERR_RequiredMembersMustBeAssignment" xml:space="preserve">
<data name="ERR_RequiredMembersMustBeAssignedValue" xml:space="preserve">
<value>Required member '{0}' must be assigned a value, it cannot use a nested member or collection initializer.</value>
</data>
<data name="ERR_RequiredMembersInvalid" xml:space="preserve">
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2059,7 +2059,7 @@ internal enum ErrorCode
ERR_ExplicitRequiredMember = 9504,
ERR_RequiredMemberMustBeSettable = 9505,
ERR_RequiredMemberMustBeSet = 9506,
ERR_RequiredMembersMustBeAssignment = 9507,
ERR_RequiredMembersMustBeAssignedValue = 9507,
ERR_RequiredMembersInvalid = 9508,
ERR_RequiredMembersBaseTypeInvalid = 9509,
}
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 41 additions & 4 deletions src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1388,13 +1388,33 @@ public class D
comp.VerifyDiagnostics(
// (2,24): error CS9507: Required member 'C.D1' must be assigned a value, it cannot use a nested member or collection initializer.
// var c = new C() { D1 = { NestedProp = 1 }, D2 = { NestedProp = 2 } };
Diagnostic(ErrorCode.ERR_RequiredMembersMustBeAssignment, "{ NestedProp = 1 }").WithArguments("C.D1").WithLocation(2, 24),
Diagnostic(ErrorCode.ERR_RequiredMembersMustBeAssignedValue, "{ NestedProp = 1 }").WithArguments("C.D1").WithLocation(2, 24),
// (2,49): error CS9507: Required member 'C.D2' must be assigned a value, it cannot use a nested member or collection initializer.
// var c = new C() { D1 = { NestedProp = 1 }, D2 = { NestedProp = 2 } };
Diagnostic(ErrorCode.ERR_RequiredMembersMustBeAssignment, "{ NestedProp = 2 }").WithArguments("C.D2").WithLocation(2, 49)
Diagnostic(ErrorCode.ERR_RequiredMembersMustBeAssignedValue, "{ NestedProp = 2 }").WithArguments("C.D2").WithLocation(2, 49)
);
}

[Fact]
public void EnforcedRequiredMembers_NoInheritance_NestedObjectCreationAllowed()
{
var c = @"
var c = new C() { D1 = new() { NestedProp = 1 }, D2 = new() { NestedProp = 2 } };

public class C
{
public required D D1 { get; set; }
public required D D2;
}
public class D
{
public int NestedProp { get; set; }
}
";
var comp = CreateCompilationWithRequiredMembers(c);
CompileAndVerify(comp).VerifyDiagnostics();
}

[Fact]
public void EnforcedRequiredMembers_NoInheritance_DisallowedNestedCollectionInitializer()
{
Expand All @@ -1412,13 +1432,30 @@ public class C
comp.VerifyDiagnostics(
// (3,24): error CS9507: Required member 'C.L1' must be assigned a value, it cannot use a nested member or collection initializer.
// var c = new C() { L1 = { 1, 2, 3 }, L2 = { 4, 5, 6 } };
Diagnostic(ErrorCode.ERR_RequiredMembersMustBeAssignment, "{ 1, 2, 3 }").WithArguments("C.L1").WithLocation(3, 24),
Diagnostic(ErrorCode.ERR_RequiredMembersMustBeAssignedValue, "{ 1, 2, 3 }").WithArguments("C.L1").WithLocation(3, 24),
// (3,42): error CS9507: Required member 'C.L2' must be assigned a value, it cannot use a nested member or collection initializer.
// var c = new C() { L1 = { 1, 2, 3 }, L2 = { 4, 5, 6 } };
Diagnostic(ErrorCode.ERR_RequiredMembersMustBeAssignment, "{ 4, 5, 6 }").WithArguments("C.L2").WithLocation(3, 42)
Diagnostic(ErrorCode.ERR_RequiredMembersMustBeAssignedValue, "{ 4, 5, 6 }").WithArguments("C.L2").WithLocation(3, 42)
);
}

[Fact]
public void EnforcedRequiredMembers_NoInheritance_NestedObjectCreationWithCollectionInitializerAllowed()
{
var c = @"
using System.Collections.Generic;
var c = new C() { L1 = new() { 1, 2, 3 }, L2 = new() { 4, 5, 6 } };

public class C
{
public required List<int> L1 { get; set; }
public required List<int> L2;
}
";
var comp = CreateCompilationWithRequiredMembers(c);
CompileAndVerify(comp).VerifyDiagnostics();
}

[Theory]
[CombinatorialData]
public void EnforcedRequiredMembers_Inheritance_NoneSet(bool useMetadataReference)
Expand Down

0 comments on commit 2ca360a

Please sign in to comment.