Skip to content

Commit

Permalink
Report diagnostic for field and value identifiers in property accesso…
Browse files Browse the repository at this point in the history
…rs where the meaning would change as contextual keywords
  • Loading branch information
cston committed May 18, 2024
1 parent 3dfab18 commit 80b4999
Show file tree
Hide file tree
Showing 26 changed files with 1,157 additions and 62 deletions.
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
118 changes: 117 additions & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1782,10 +1782,20 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
assertBindIdentifierTargets(inMethodBinder, identifierMap, methodBody, diagnostics);
#endif

var compilation = bodyBinder.Compilation;

if (method.MethodKind is MethodKind.PropertyGet or MethodKind.PropertySet)
{
var requiredVersion = MessageID.IDS_FeatureFieldAndValueKeywords.RequiredVersion();
if (requiredVersion > compilation.LanguageVersion)
{
ReportFieldOrValueContextualKeywordConflicts(method, methodBody, diagnostics);
}
}

BoundNode methodBodyForSemanticModel = methodBody;
NullableWalker.SnapshotManager? snapshotManager = null;
ImmutableDictionary<Symbol, Symbol>? remappedSymbols = null;
var compilation = bodyBinder.Compilation;

nullableInitialState = getInitializerState(methodBody);

Expand Down Expand Up @@ -2149,6 +2159,112 @@ static void assertBindIdentifierTargets(InMethodBinder? inMethodBinder, Concurre
#endif
}

/// <summary>
/// Report a diagnostic for any 'field' or 'value' identifier in the bound tree where the
/// meaning will change when the identifier is considered a contextual keyword.
/// </summary>
private static void ReportFieldOrValueContextualKeywordConflicts(MethodSymbol method, BoundNode node, BindingDiagnosticBag diagnostics)
{
Debug.Assert(method.MethodKind is MethodKind.PropertyGet or MethodKind.PropertySet);
Debug.Assert(method.AssociatedSymbol is PropertySymbol);

PooledHashSet<IdentifierNameSyntax>? valueIdentifiers = null;

foreach (var syntax in node.Syntax.DescendantNodesAndSelf())
{
// PROTOTYPE: Handle all syntax fields with <Kind Name="IdentifierToken"/> from syntax.xml.
switch (syntax)
{
case IdentifierNameSyntax identifierName:
if (isFieldOrValueInKeywordContext(method, identifierName.Identifier, out bool isValue))
{
if (isValue)
{
// Report conflicts with "value" later, after collecting all references and
// dropping any that refer to the implicit parameter.
valueIdentifiers ??= PooledHashSet<IdentifierNameSyntax>.GetInstance();
valueIdentifiers.Add(identifierName);
}
else
{
reportConflict(identifierName, identifierName.Identifier, diagnostics);
}
}
break;
case VariableDeclaratorSyntax variableDeclarator:
reportConflictIfAny(method, variableDeclarator, variableDeclarator.Identifier, diagnostics);
break;
case ParameterSyntax parameter:
reportConflictIfAny(method, parameter, parameter.Identifier, diagnostics);
break;
}
}

if (valueIdentifiers is { })
{
// Remove references to the implicit "value" parameter.
var checker = new ValueIdentifierChecker(valueIdentifiers);
checker.Visit(node);
foreach (var identifierName in valueIdentifiers)
{
reportConflict(identifierName, identifierName.Identifier, diagnostics);
}
valueIdentifiers.Free();
}

static bool isFieldOrValueInKeywordContext(MethodSymbol method, SyntaxToken identifierToken, out bool isValue)
{
switch (identifierToken.Text)
{
case "field":
isValue = false;
return method.AssociatedSymbol is PropertySymbol { IsIndexer: false };
case "value":
isValue = true;
return method.MethodKind == MethodKind.PropertySet;
default:
isValue = false;
return false;
}
}

static void reportConflictIfAny(MethodSymbol method, SyntaxNode syntax, SyntaxToken identifierToken, BindingDiagnosticBag diagnostics)
{
if (isFieldOrValueInKeywordContext(method, identifierToken, out _))
{
reportConflict(syntax, identifierToken, diagnostics);
}
}

static void reportConflict(SyntaxNode syntax, SyntaxToken identifierToken, BindingDiagnosticBag diagnostics)
{
var requiredVersion = MessageID.IDS_FeatureFieldAndValueKeywords.RequiredVersion();
diagnostics.Add(ErrorCode.INF_IdentifierConflictWithContextualKeyword, syntax, identifierToken.Text, requiredVersion.ToDisplayString());
}
}

private sealed class ValueIdentifierChecker : BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator
{
private readonly HashSet<IdentifierNameSyntax> _valueIdentifiers;

public ValueIdentifierChecker(HashSet<IdentifierNameSyntax> valueIdentifiers)
{
_valueIdentifiers = valueIdentifiers;
}

public override BoundNode? VisitParameter(BoundParameter node)
{
Debug.Assert(node.Syntax is IdentifierNameSyntax);

if (node.ParameterSymbol is SynthesizedPropertyAccessorValueParameterSymbol { Name: "value" })
{
_valueIdentifiers.Remove((IdentifierNameSyntax)node.Syntax);
}

return base.VisitParameter(node);
}
}

#if DEBUG
private static bool IsEmptyRewritePossible(BoundNode node)
{
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2324,6 +2324,7 @@ internal enum ErrorCode
ERR_RefStructDoesNotSupportDefaultInterfaceImplementationForMember = 9245,
ERR_BadNonVirtualInterfaceMemberAccessOnAllowsRefLike = 9246,
ERR_BadAllowByRefLikeEnumerator = 9247,
INF_IdentifierConflictWithContextualKeyword = 9248,

// Note: you will need to do the following after adding errors:
// 1) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2450,6 +2450,7 @@ or ErrorCode.ERR_NotRefStructConstraintNotSatisfied
or ErrorCode.ERR_RefStructDoesNotSupportDefaultInterfaceImplementationForMember
or ErrorCode.ERR_BadNonVirtualInterfaceMemberAccessOnAllowsRefLike
or ErrorCode.ERR_BadAllowByRefLikeEnumerator
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 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 @@ -286,6 +286,7 @@ internal enum MessageID
IDS_FeatureRefUnsafeInIteratorAsync = MessageBase + 12843,

IDS_FeatureRefStructInterfaces = MessageBase + 12844,
IDS_FeatureFieldAndValueKeywords = MessageBase + 12845,
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -472,6 +473,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
case MessageID.IDS_FeatureParamsCollections:
case MessageID.IDS_FeatureRefUnsafeInIteratorAsync:
case MessageID.IDS_FeatureRefStructInterfaces:
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.

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.

15 changes: 15 additions & 0 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.

15 changes: 15 additions & 0 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.

15 changes: 15 additions & 0 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.

Loading

0 comments on commit 80b4999

Please sign in to comment.