Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[semi-auto-props]: Disallow 'nameof(field)' #60384

Merged
merged 5 commits into from
Apr 6, 2022
Merged

[semi-auto-props]: Disallow 'nameof(field)' #60384

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8beed73
[semi-auto-props]: Disallow 'nameof(field)'
Youssef1313 Mar 23, 2022
2153462
Add more tests
Youssef1313 Mar 25, 2022
d196e24
Add suggested test
Youssef1313 Apr 1, 2022
61d2d8e
Address review comment
Youssef1313 Apr 4, 2022
6f9aeea
Merge remote-tracking branch 'upstream/features/semi-auto-props' into…
Youssef1313 Apr 6, 2022
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
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,8 @@ private BoundExpression BindIdentifier(
// Cannot escape out of the current expression, as it's a compiler-synthesized location.
expression = new BoundDiscardExpression(node, LocalScopeDepth, type: null);
}
else if (node.Identifier.ContextualKind() == SyntaxKind.FieldKeyword &&
else if (!IsInsideNameof &&
Copy link
Contributor

@AlekseyTs AlekseyTs Apr 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

!IsInsideNameof &&

I think it would be better to report an error, but not change what is in scope inside nameof. #Closed

node.Identifier.ContextualKind() == SyntaxKind.FieldKeyword &&
// PROTOTYPE(semi-auto-props): Use ContainingMember() to support local functions and lambdas.
ContainingMemberOrLambda is SourcePropertyAccessorSymbol { Property.IsIndexer: false } accessor)
{
Expand Down
202 changes: 183 additions & 19 deletions src/Compilers/CSharp/Test/Semantic/Semantics/PropertyFieldKeywordTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,133 @@ public class MyAttribute : System.Attribute
public MyAttribute(string s) { }
}
");
var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics(
// (10,24): error CS0103: The name 'field' does not exist in the current context
// [My(nameof(field))]
Diagnostic(ErrorCode.ERR_NameNotInContext, "field").WithArguments("field").WithLocation(10, 24)
);
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact]
public void TestNameOfField()
{
var comp = CreateCompilation(@"
public class C
{
public int P
{
get
{
return nameof(field);
}
}
}
");
var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics(
// (8,27): error CS0103: The name 'field' does not exist in the current context
// return nameof(field);
Diagnostic(ErrorCode.ERR_NameNotInContext, "field").WithArguments("field").WithLocation(8, 27)
);
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact]
public void TestNameOfField_NameofIsMethodInvocation()
{
var comp = CreateCompilation(@"
public class C
{
public int P
{
get
{
return nameof(field);
}
}

public int nameof(int x) => 0;
}
");
var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics();
VerifyTypeIL(comp, "C", @"
.class public auto ansi beforefieldinit C
extends [mscorlib]System.Object
{
// Fields
.field private initonly int32 '<P>k__BackingField'
.custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
01 00 00 00
)
// Methods
.method public hidebysig specialname
instance int32 get_P () cil managed
{
// Method begins at RVA 0x2050
// Code size 13 (0xd)
.maxstack 8
IL_0000: ldarg.0
IL_0001: ldarg.0
IL_0002: ldfld int32 C::'<P>k__BackingField'
IL_0007: call instance int32 C::nameof(int32)
IL_000c: ret
} // end of method C::get_P
.method public hidebysig
instance int32 nameof (
int32 x
) cil managed
{
// Method begins at RVA 0x205e
// Code size 2 (0x2)
.maxstack 8
IL_0000: ldc.i4.0
IL_0001: ret
} // end of method C::nameof
.method public hidebysig specialname rtspecialname
instance void .ctor () cil managed
{
// Method begins at RVA 0x2061
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [mscorlib]System.Object::.ctor()
IL_0006: ret
} // end of method C::.ctor
// Properties
.property instance int32 P()
{
.get instance int32 C::get_P()
}
} // end of class C
");
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact]
public void TestNameOfField_NameofIsLocalFunctionInvocation()
333fred marked this conversation as resolved.
Show resolved Hide resolved
{
var comp = CreateCompilation(@"
public class C
{
public int P
{
get
{
return nameof(field);

int nameof(int x) => 0;
}
}
}
");
var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
comp.VerifyDiagnostics();
VerifyTypeIL(comp, "C", @"
.class public auto ansi beforefieldinit C
Expand All @@ -120,43 +247,73 @@ .method public hidebysig specialname
instance int32 get_P () cil managed
{
// Method begins at RVA 0x2050
// Code size 6 (0x6)
// Code size 12 (0xc)
.maxstack 8
IL_0000: call int32 C::'<get_P>g__local|1_0'()
IL_0005: ret
IL_0000: ldarg.0
IL_0001: ldfld int32 C::'<P>k__BackingField'
IL_0006: call int32 C::'<get_P>g__nameof|1_0'(int32)
IL_000b: ret
} // end of method C::get_P
.method public hidebysig specialname rtspecialname
instance void .ctor () cil managed
{
// Method begins at RVA 0x2057
// Method begins at RVA 0x205d
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [mscorlib]System.Object::.ctor()
IL_0006: ret
} // end of method C::.ctor
.method assembly hidebysig static
int32 '<get_P>g__local|1_0' () cil managed
int32 '<get_P>g__nameof|1_0' (
int32 x
) cil managed
{
.custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
01 00 00 00
)
.custom instance void MyAttribute::.ctor(string) = (
01 00 05 66 69 65 6c 64 00 00
)
// Method begins at RVA 0x205f
// Method begins at RVA 0x2065
// Code size 2 (0x2)
.maxstack 8
IL_0000: ldc.i4.0
IL_0001: ret
} // end of method C::'<get_P>g__local|1_0'
} // end of method C::'<get_P>g__nameof|1_0'
// Properties
.property instance int32 P()
{
.get instance int32 C::get_P()
}
} // end of class C
");
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Theory]
[InlineData("private void field() { }")]
[InlineData("private int field { get => 0; }")]
[InlineData("private int field;")]
public void TestNameOfField_FieldIsMember(string member)
{
var comp = CreateCompilation($@"
System.Console.WriteLine(new C().P);

public class C
{{
{member}

public string P
{{
get
{{
return nameof(field);
}}
}}
}}
");
var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
comp.TestOnlyCompilationData = accessorBindingData;
CompileAndVerify(comp, expectedOutput: "field");
Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
}

[Fact(Skip = "PROTOTYPE(semi-auto-props): Assigning in constructor is not yet supported.")]
Expand Down Expand Up @@ -3839,16 +3996,20 @@ public MyAttribute(string s) { }
Assert.Equal("System.Int32 C.<P>k__BackingField", comp.GetTypeByMetadataName("C").GetFieldsToEmit().Single().ToTestDisplayString());

var fieldKeywordSymbolInfo = speculativeModel.GetSymbolInfo(fieldNode);

// Since we're passing fieldNode, the model don't know we're inside nameof, so field is bound to backing field.
// This doesn't match fieldKeywordSymbolInfo where semantic model is aware we're inside nameof and doesn't bind to backing.
var fieldKeywordSymbolInfo2 = model.GetSpeculativeSymbolInfo(attributeSyntax.SpanStart, fieldNode, bindingOption);

Assert.True(fieldKeywordSymbolInfo.IsEmpty);
Assert.Null(fieldKeywordSymbolInfo.Symbol);
if (bindingOption == SpeculativeBindingOption.BindAsTypeOrNamespace)
{
Assert.True(fieldKeywordSymbolInfo2.IsEmpty);
Assert.Null(fieldKeywordSymbolInfo2.Symbol);
Assert.Equal(fieldKeywordSymbolInfo2, fieldKeywordSymbolInfo);
}
else
{
Assert.Equal(fieldKeywordSymbolInfo, fieldKeywordSymbolInfo2);
Assert.Equal(comp.GetTypeByMetadataName("C").GetFieldsToEmit().Single(), fieldKeywordSymbolInfo.Symbol.GetSymbol());
Assert.Equal(comp.GetTypeByMetadataName("C").GetFieldsToEmit().Single(), fieldKeywordSymbolInfo2.Symbol.GetSymbol());
}

var typeInfo = model.GetSpeculativeTypeInfo(attributeSyntax.SpanStart, fieldNode, bindingOption);
Expand Down Expand Up @@ -3901,15 +4062,19 @@ public MyAttribute(string s) { }
model.TryGetSpeculativeSemanticModel(attributeSyntax.SpanStart, newAttributeSyntax, out var speculativeModel);

var fieldKeywordSymbolInfo = speculativeModel.GetSymbolInfo(fieldNode);

// Since we're passing fieldNode, the model don't know we're inside nameof, so field is bound to backing field.
// This doesn't match fieldKeywordSymbolInfo where semantic model is aware we're inside nameof and doesn't bind to backing.
var fieldKeywordSymbolInfo2 = model.GetSpeculativeSymbolInfo(attributeSyntax.SpanStart, fieldNode, bindingOption);
if (bindingOption == SpeculativeBindingOption.BindAsExpression)
Assert.True(fieldKeywordSymbolInfo.IsEmpty);
Assert.Null(fieldKeywordSymbolInfo.Symbol);
if (bindingOption == SpeculativeBindingOption.BindAsTypeOrNamespace)
{
Assert.Equal(fieldKeywordSymbolInfo, fieldKeywordSymbolInfo2);
Assert.Equal(fieldKeywordSymbolInfo2, fieldKeywordSymbolInfo);
}
else
{
Assert.True(fieldKeywordSymbolInfo2.IsEmpty);
Assert.Null(fieldKeywordSymbolInfo2.Symbol);
Assert.Equal(comp.GetTypeByMetadataName("C").GetFieldsToEmit().Single(), fieldKeywordSymbolInfo2.Symbol.GetSymbol());
}

var typeInfo = model.GetSpeculativeTypeInfo(attributeSyntax.SpanStart, fieldNode, bindingOption);
Expand All @@ -3921,7 +4086,6 @@ public MyAttribute(string s) { }
Assert.Null(aliasInfo);

Assert.Equal("System.Int32 C.<P>k__BackingField", comp.GetTypeByMetadataName("C").GetFieldsToEmit().Single().ToTestDisplayString());
Assert.Equal(comp.GetTypeByMetadataName("C").GetFieldsToEmit().Single(), fieldKeywordSymbolInfo.Symbol.GetSymbol());
Assert.Equal(runNullableAnalysis == "always" ? 0 : 1, accessorBindingData.NumberOfPerformedAccessorBinding);
}

Expand Down