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.

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

Report diagnostic for field and value identifiers in property accessors where the meaning would change as contextual keywords #73570

Merged
merged 16 commits into from
Jun 5, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ internal RangeVariableSymbol AddRangeVariable(Binder binder, SyntaxToken identif
var result = new RangeVariableSymbol(name, binder.ContainingMemberOrLambda, identifier.GetLocation());
bool error = false;

Debug.Assert(identifier.Parent is { });
binder.ReportFieldOrValueContextualKeywordConflictIfAny(identifier.Parent, identifier, diagnostics);

foreach (var existingRangeVariable in allRangeVariables.Keys)
{
if (existingRangeVariable.Name == name)
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,8 @@ private BoundExpression BindDeconstructionVariable(
{
SourceLocalSymbol localSymbol = LookupLocal(designation.Identifier);

ReportFieldOrValueContextualKeywordConflictIfAny(designation, designation.Identifier, diagnostics);

// is this a local?
if ((object)localSymbol != null)
{
Expand Down
33 changes: 33 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1549,6 +1549,11 @@ private BoundExpression BindIdentifier(
var members = ArrayBuilder<Symbol>.GetInstance();
Symbol symbol = GetSymbolOrMethodOrPropertyGroup(lookupResult, node, name, node.Arity, members, diagnostics, out isError, qualifierOpt: null); // reports diagnostics in result.

if (symbol is not SynthesizedAccessorValueParameterSymbol { Name: "value" })
{
ReportFieldOrValueContextualKeywordConflictIfAny(node, node.Identifier, diagnostics);
}

if ((object)symbol == null)
{
Debug.Assert(members.Count > 0);
Expand Down Expand Up @@ -1731,6 +1736,30 @@ void reportPrimaryConstructorParameterShadowing(SimpleNameSyntax node, Symbol sy
}
}

#nullable enable
/// <summary>
/// Report a diagnostic for a 'field' or 'value' identifier that the meaning will
/// change when the identifier is considered a contextual keyword.
/// </summary>
internal void ReportFieldOrValueContextualKeywordConflictIfAny(SyntaxNode syntax, SyntaxToken identifier, BindingDiagnosticBag diagnostics)
{
string name = identifier.Text;
switch (name)
{
case "field" when ContainingMember() is MethodSymbol { MethodKind: MethodKind.PropertyGet or MethodKind.PropertySet, AssociatedSymbol: PropertySymbol { IsIndexer: false } }:
case "value" when ContainingMember() is MethodSymbol { MethodKind: MethodKind.PropertySet or MethodKind.EventAdd or MethodKind.EventRemove }:
{
var requiredVersion = MessageID.IDS_FeatureFieldAndValueKeywords.RequiredVersion();
if (Compilation.LanguageVersion < requiredVersion)
AlekseyTs marked this conversation as resolved.
Show resolved Hide resolved
{
diagnostics.Add(ErrorCode.INF_IdentifierConflictWithContextualKeyword, syntax, name, requiredVersion.ToDisplayString());
}
}
break;
}
}
#nullable disable

private void LookupIdentifier(LookupResult lookupResult, SimpleNameSyntax node, bool invoked, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
LookupOptions options = LookupOptions.AllMethodsOnArityZero;
Expand Down Expand Up @@ -3090,6 +3119,8 @@ private BoundExpression BindOutVariableDeclarationArgument(
var designation = (SingleVariableDesignationSyntax)declarationExpression.Designation;
TypeSyntax typeSyntax = declarationExpression.Type;

ReportFieldOrValueContextualKeywordConflictIfAny(designation, designation.Identifier, diagnostics);

// Is this a local?
SourceLocalSymbol localSymbol = this.LookupLocal(designation.Identifier);
if ((object)localSymbol != null)
Expand Down Expand Up @@ -7524,6 +7555,8 @@ private BoundExpression BindMemberAccessWithBoundLeft(

boundLeft = MakeMemberAccessValue(boundLeft, diagnostics);

ReportFieldOrValueContextualKeywordConflictIfAny(right, right.Identifier, diagnostics);

TypeSymbol leftType = boundLeft.Type;

if ((object)leftType != null && leftType.IsDynamic())
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ private UnboundLambda AnalyzeAnonymousFunction(
// x => ...
hasSignature = true;
var simple = (SimpleLambdaExpressionSyntax)syntax;
ReportFieldOrValueContextualKeywordConflictIfAny(simple.Parameter, simple.Parameter.Identifier, diagnostics);
AlekseyTs marked this conversation as resolved.
Show resolved Hide resolved
namesBuilder.Add(simple.Parameter.Identifier.ValueText);
break;
case SyntaxKind.ParenthesizedLambdaExpression:
Expand Down Expand Up @@ -192,6 +193,8 @@ private UnboundLambda AnalyzeAnonymousFunction(
}
}

ReportFieldOrValueContextualKeywordConflictIfAny(p, p.Identifier, diagnostics);

namesBuilder.Add(p.Identifier.ValueText);
typesBuilder.Add(type);
refKindsBuilder.Add(refKind);
Expand Down
12 changes: 10 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,8 @@ private BoundLabeledStatement BindLabeled(LabeledStatementSyntax node, BindingDi
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
var binder = this.LookupSymbolsWithFallback(result, node.Identifier.ValueText, arity: 0, useSiteInfo: ref useSiteInfo, options: LookupOptions.LabelsOnly);

ReportFieldOrValueContextualKeywordConflictIfAny(node, node.Identifier, diagnostics);

// result.Symbols can be empty in some malformed code, e.g. when a labeled statement is used an embedded statement in an if or foreach statement
// In this case we create new label symbol on the fly, and an error is reported by parser
var symbol = result.Symbols.Count > 0 && result.IsMultiViable ?
Expand Down Expand Up @@ -559,6 +561,8 @@ private BoundStatement BindLocalFunctionStatement(LocalFunctionStatementSyntax n
{
MessageID.IDS_FeatureLocalFunctions.CheckFeatureAvailability(diagnostics, node.Identifier);

ReportFieldOrValueContextualKeywordConflictIfAny(node, node.Identifier, diagnostics);

// already defined symbol in containing block
var localSymbol = this.LookupLocalFunction(node.Identifier);

Expand Down Expand Up @@ -968,7 +972,7 @@ protected BoundLocalDeclaration BindVariableDeclaration(
{
Debug.Assert(declarator != null);

return BindVariableDeclaration(LocateDeclaredVariableSymbol(declarator, typeSyntax, kind),
return BindVariableDeclaration(LocateDeclaredVariableSymbol(declarator, typeSyntax, kind, diagnostics),
kind,
isVar,
declarator,
Expand Down Expand Up @@ -1202,8 +1206,10 @@ internal ImmutableArray<BoundExpression> BindDeclaratorArguments(VariableDeclara
return arguments;
}

private SourceLocalSymbol LocateDeclaredVariableSymbol(VariableDeclaratorSyntax declarator, TypeSyntax typeSyntax, LocalDeclarationKind outerKind)
private SourceLocalSymbol LocateDeclaredVariableSymbol(VariableDeclaratorSyntax declarator, TypeSyntax typeSyntax, LocalDeclarationKind outerKind, BindingDiagnosticBag diagnostics)
{
ReportFieldOrValueContextualKeywordConflictIfAny(declarator, declarator.Identifier, diagnostics);

LocalDeclarationKind kind = outerKind == LocalDeclarationKind.UsingVariable ? LocalDeclarationKind.UsingVariable : LocalDeclarationKind.RegularVariable;
return LocateDeclaredVariableSymbol(declarator.Identifier, typeSyntax, declarator.Initializer, kind);
}
Expand Down Expand Up @@ -3158,6 +3164,8 @@ private BoundCatchBlock BindCatchBlock(CatchClauseSyntax node, ArrayBuilder<Boun
var declaration = node.Declaration;
if (declaration != null)
{
ReportFieldOrValueContextualKeywordConflictIfAny(declaration, declaration.Identifier, diagnostics);

// Note: The type is being bound twice: here and in LocalSymbol.Type. Currently,
// LocalSymbol.Type ignores diagnostics so it seems cleaner to bind the type here
// as well. However, if LocalSymbol.Type is changed to report diagnostics, we'll
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,7 @@ private TypeSymbol BindTupleType(TupleTypeSyntax syntax, BindingDiagnosticBag di
hasExplicitNames = true;
CheckTupleMemberName(name, i, nameToken, diagnostics, uniqueFieldNames);
locations.Add(nameToken.GetLocation());
ReportFieldOrValueContextualKeywordConflictIfAny(argumentSyntax, nameToken, diagnostics);
AlekseyTs marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand Down Expand Up @@ -871,6 +872,8 @@ protected NamespaceOrTypeOrAliasSymbolWithAnnotations BindNonGenericSimpleNamesp
var result = LookupResult.GetInstance();
LookupOptions options = GetSimpleNameLookupOptions(node, node.Identifier.IsVerbatimIdentifier());

ReportFieldOrValueContextualKeywordConflictIfAny(node, node.Identifier, diagnostics);

CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
this.LookupSymbolsSimpleName(result, qualifierOpt, identifierValueText, 0, basesBeingResolved, options, diagnose: true, useSiteInfo: ref useSiteInfo);
diagnostics.Add(node, useSiteInfo);
Expand Down Expand Up @@ -1210,6 +1213,8 @@ private TypeWithAnnotations BindGenericSimpleNamespaceOrTypeOrAliasSymbol(
diagnostics, basesBeingResolved, qualifierOpt, node, plainName, node.Arity, options);
NamedTypeSymbol resultType;

ReportFieldOrValueContextualKeywordConflictIfAny(node, node.Identifier, diagnostics);

if (isUnboundTypeExpr)
{
if (!IsUnboundTypeAllowed(node))
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,11 @@ private BoundForEachStatement BindForEachPartsWorker(BindingDiagnosticBag diagno
CheckFeatureAvailability(_syntax.AwaitKeyword, MessageID.IDS_FeatureAsyncStreams, diagnostics);
}

if (_syntax is ForEachStatementSyntax forEachStatement)
{
ReportFieldOrValueContextualKeywordConflictIfAny(forEachStatement, forEachStatement.Identifier, diagnostics);
Copy link
Member

Choose a reason for hiding this comment

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

nit: have you considered placing this check in the switch below that already differentiates regular foreach from deconstruction foreach? (line 277)

}

// Use the right binder to avoid seeing iteration variable
BoundExpression collectionExpr = originalBinder.GetBinder(_syntax.Expression).BindRValueWithoutTargetType(_syntax.Expression, diagnostics);

Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6865,6 +6865,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="INF_TooManyBoundLambdas_Title" xml:space="preserve">
<value>Compiling requires binding the lambda expression many times. Consider declaring the lambda expression with explicit parameter types, or if the containing method call is generic, consider using explicit type arguments.</value>
</data>
<data name="INF_IdentifierConflictWithContextualKeyword" xml:space="preserve">
<value>'{0}' is a contextual keyword, with a specific meaning, starting in language version {1}. Use '@{0}' to avoid a breaking change when compiling with language version {1} or later.</value>
</data>
<data name="INF_IdentifierConflictWithContextualKeyword_Title" xml:space="preserve">
<value>Identifier is a contextual keyword, with a specific meaning, in a later language version.</value>
</data>
<data name="ERR_EqualityContractRequiresGetter" xml:space="preserve">
<value>Record equality contract property '{0}' must have a get accessor.</value>
</data>
Expand Down Expand Up @@ -7920,6 +7926,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeatureRefUnsafeInIteratorAsync" xml:space="preserve">
<value>ref and unsafe in async and iterator methods</value>
</data>
<data name="IDS_FeatureFieldAndValueKeywords" xml:space="preserve">
<value>field and value keywords</value>
</data>
<data name="ERR_RefLocalAcrossAwait" xml:space="preserve">
<value>A 'ref' local cannot be preserved across 'await' or 'yield' boundary.</value>
</data>
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2336,6 +2336,8 @@ internal enum ErrorCode
WRN_PartialPropertySignatureDifference = 9256,
ERR_PartialPropertyRequiredDifference = 9257,

INF_IdentifierConflictWithContextualKeyword = 9258,

// Note: you will need to do the following after adding errors:
// 1) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)

Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2461,6 +2461,7 @@ or ErrorCode.ERR_PartialPropertyInitMismatch
or ErrorCode.ERR_PartialPropertyTypeDifference
or ErrorCode.WRN_PartialPropertySignatureDifference
or ErrorCode.ERR_PartialPropertyRequiredDifference
or ErrorCode.INF_IdentifierConflictWithContextualKeyword
=> false,
};
#pragma warning restore CS8524 // The switch expression does not handle some values of its input type (it is not exhaustive) involving an unnamed enum value.
Expand All @@ -2482,7 +2483,7 @@ internal static bool PreventsSuccessfulDelegateConversion(ErrorCode code)
return false;
}

if (IsWarning(code))
if (IsWarning(code) || IsInfo(code) || IsHidden(code))
{
return false;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ internal enum MessageID
IDS_FeatureRefStructInterfaces = MessageBase + 12844,

IDS_FeaturePartialProperties = MessageBase + 12845,
IDS_FeatureFieldAndValueKeywords = MessageBase + 12846,
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -475,6 +476,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
case MessageID.IDS_FeatureRefUnsafeInIteratorAsync:
case MessageID.IDS_FeatureRefStructInterfaces:
case MessageID.IDS_FeaturePartialProperties:
case MessageID.IDS_FeatureFieldAndValueKeywords:
return LanguageVersion.Preview;

// C# 12.0 features.
Expand Down

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

Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public LocalFunctionSymbol(
ScopeBinder = binder;

binder = binder.SetOrClearUnsafeRegionIfNecessary(syntax.Modifiers);
_binder = binder;

if (syntax.TypeParameterList != null)
{
Expand All @@ -85,8 +86,6 @@ public LocalFunctionSymbol(
_declarationDiagnostics.AddRange(diagnostics.DiagnosticBag);
_declarationDependencies.AddAll(diagnostics.DependenciesBag);
diagnostics.Free();

_binder = binder;
}

/// <summary>
Expand Down Expand Up @@ -202,6 +201,11 @@ private void ComputeParameters()
addRefReadOnlyModifier: false,
diagnostics: diagnostics).Cast<SourceParameterSymbol, ParameterSymbol>();

foreach (var parameter in this.Syntax.ParameterList.Parameters)
{
WithTypeParametersBinder.ReportFieldOrValueContextualKeywordConflictIfAny(parameter, parameter.Identifier, diagnostics);
AlekseyTs marked this conversation as resolved.
Show resolved Hide resolved
}

// Note: we don't need to warn on annotations used in #nullable disable context for local functions, as this is handled in binding already

var isVararg = arglistToken.Kind() == SyntaxKind.ArgListKeyword;
Expand Down Expand Up @@ -433,6 +437,7 @@ private ImmutableArray<SourceMethodTypeParameterSymbol> MakeTypeParameters(Bindi
}

SourceMemberContainerTypeSymbol.ReportReservedTypeName(identifier.Text, this.DeclaringCompilation, diagnostics.DiagnosticBag, location);
_binder.ReportFieldOrValueContextualKeywordConflictIfAny(parameter, identifier, diagnostics);

var tpEnclosing = ContainingSymbol.FindEnclosingTypeParameter(name);
if ((object?)tpEnclosing != null)
Expand Down
15 changes: 15 additions & 0 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.

Loading
Loading