Skip to content

Commit

Permalink
Partial properties: diagnostics for mismatch between parts
Browse files Browse the repository at this point in the history
  • Loading branch information
RikkiGibson committed May 1, 2024
1 parent c3f13e3 commit 6718f15
Show file tree
Hide file tree
Showing 31 changed files with 1,053 additions and 362 deletions.
30 changes: 18 additions & 12 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2234,17 +2234,17 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_PartialMethodMustHaveLatent" xml:space="preserve">
<value>No defining declaration found for implementing declaration of partial method '{0}'</value>
</data>
<data name="ERR_PartialMethodInconsistentTupleNames" xml:space="preserve">
<value>Both partial method declarations, '{0}' and '{1}', must use the same tuple element names.</value>
<data name="ERR_PartialMemberInconsistentTupleNames" xml:space="preserve">
<value>Both partial member declarations, '{0}' and '{1}', must use the same tuple element names.</value>
</data>
<data name="ERR_PartialMethodInconsistentConstraints" xml:space="preserve">
<value>Partial method declarations of '{0}' have inconsistent constraints for type parameter '{1}'</value>
</data>
<data name="ERR_PartialMethodToDelegate" xml:space="preserve">
<value>Cannot create delegate from method '{0}' because it is a partial method without an implementing declaration</value>
</data>
<data name="ERR_PartialMethodStaticDifference" xml:space="preserve">
<value>Both partial method declarations must be static or neither may be static</value>
<data name="ERR_PartialMemberStaticDifference" xml:space="preserve">
<value>Both partial member declarations must be static or neither may be static</value>
</data>
<data name="ERR_PartialMethodUnsafeDifference" xml:space="preserve">
<value>Both partial method declarations must be unsafe or neither may be unsafe</value>
Expand Down Expand Up @@ -5752,8 +5752,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_FieldLikeEventCantBeReadOnly" xml:space="preserve">
<value>Field-like event '{0}' cannot be 'readonly'.</value>
</data>
<data name="ERR_PartialMethodReadOnlyDifference" xml:space="preserve">
<value>Both partial method declarations must be readonly or neither may be readonly</value>
<data name="ERR_PartialMemberReadOnlyDifference" xml:space="preserve">
<value>Both partial member declarations must be readonly or neither may be readonly</value>
</data>
<data name="ERR_ReadOnlyModMissingAccessor" xml:space="preserve">
<value>'{0}': 'readonly' can only be used on accessors if the property or indexer has both a get and a set accessor</value>
Expand Down Expand Up @@ -6536,17 +6536,17 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_PartialMethodWithExtendedModMustHaveAccessMods" xml:space="preserve">
<value>Partial method '{0}' must have accessibility modifiers because it has a 'virtual', 'override', 'sealed', 'new', or 'extern' modifier.</value>
</data>
<data name="ERR_PartialMethodAccessibilityDifference" xml:space="preserve">
<value>Both partial method declarations must have identical accessibility modifiers.</value>
<data name="ERR_PartialMemberAccessibilityDifference" xml:space="preserve">
<value>Both partial member declarations must have identical accessibility modifiers.</value>
</data>
<data name="ERR_PartialMethodExtendedModDifference" xml:space="preserve">
<value>Both partial method declarations must have identical combinations of 'virtual', 'override', 'sealed', and 'new' modifiers.</value>
<data name="ERR_PartialMemberExtendedModDifference" xml:space="preserve">
<value>Both partial member declarations must have identical combinations of 'virtual', 'override', 'sealed', and 'new' modifiers.</value>
</data>
<data name="ERR_PartialMethodReturnTypeDifference" xml:space="preserve">
<value>Both partial method declarations must have the same return type.</value>
</data>
<data name="ERR_PartialMethodRefReturnDifference" xml:space="preserve">
<value>Partial method declarations must have matching ref return values.</value>
<data name="ERR_PartialMemberRefReturnDifference" xml:space="preserve">
<value>Partial member declarations must have matching ref return values.</value>
</data>
<data name="WRN_PartialMethodTypeDifference" xml:space="preserve">
<value>Partial method declarations '{0}' and '{1}' have signature differences.</value>
Expand Down Expand Up @@ -7947,4 +7947,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_PartialPropertyUnexpectedAccessor" xml:space="preserve">
<value>Property accessor '{0}' does not implement any accessor declared on the definition part</value>
</data>
<data name="ERR_PartialPropertyInitMismatch" xml:space="preserve">
<value>Property accessor '{0}' must be '{1}' to match the definition part</value>
</data>
<data name="ERR_PartialPropertyTypeDifference" xml:space="preserve">
<value>Both partial property declarations must have the same type.</value>
</data>
</root>
14 changes: 8 additions & 6 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ internal enum ErrorCode
ERR_PartialMethodMustHaveLatent = 759,
ERR_PartialMethodInconsistentConstraints = 761,
ERR_PartialMethodToDelegate = 762,
ERR_PartialMethodStaticDifference = 763,
ERR_PartialMemberStaticDifference = 763,
ERR_PartialMethodUnsafeDifference = 764,
ERR_PartialMethodInExpressionTree = 765,
// ERR_PartialMethodMustReturnVoid = 766, Removed as part of 'extended partial methods' feature
Expand Down Expand Up @@ -1392,7 +1392,7 @@ internal enum ErrorCode
ERR_CantChangeTupleNamesOnOverride = 8139,
ERR_DuplicateInterfaceWithTupleNamesInBaseList = 8140,
ERR_ImplBadTupleNames = 8141,
ERR_PartialMethodInconsistentTupleNames = 8142,
ERR_PartialMemberInconsistentTupleNames = 8142,
ERR_ExpressionTreeContainsTupleLiteral = 8143,
ERR_ExpressionTreeContainsTupleConversion = 8144,
#endregion tuple diagnostics introduced in C# 7
Expand Down Expand Up @@ -1709,7 +1709,7 @@ internal enum ErrorCode
ERR_InvalidPropertyReadOnlyMods = 8660,
ERR_DuplicatePropertyReadOnlyMods = 8661,
ERR_FieldLikeEventCantBeReadOnly = 8662,
ERR_PartialMethodReadOnlyDifference = 8663,
ERR_PartialMemberReadOnlyDifference = 8663,
ERR_ReadOnlyModMissingAccessor = 8664,
ERR_OverrideRefConstraintNotSatisfied = 8665,
ERR_OverrideValConstraintNotSatisfied = 8666,
Expand Down Expand Up @@ -1809,8 +1809,8 @@ internal enum ErrorCode
ERR_PartialMethodWithNonVoidReturnMustHaveAccessMods = 8796,
ERR_PartialMethodWithOutParamMustHaveAccessMods = 8797,
ERR_PartialMethodWithExtendedModMustHaveAccessMods = 8798,
ERR_PartialMethodAccessibilityDifference = 8799,
ERR_PartialMethodExtendedModDifference = 8800,
ERR_PartialMemberAccessibilityDifference = 8799,
ERR_PartialMemberExtendedModDifference = 8800,

ERR_SimpleProgramLocalIsReferencedOutsideOfTopLevelStatement = 8801,
ERR_SimpleProgramMultipleUnitsWithTopLevelStatements = 8802,
Expand All @@ -1832,7 +1832,7 @@ internal enum ErrorCode
ERR_ModuleInitializerMethodAndContainingTypesMustNotBeGeneric = 8816,

ERR_PartialMethodReturnTypeDifference = 8817,
ERR_PartialMethodRefReturnDifference = 8818,
ERR_PartialMemberRefReturnDifference = 8818,
WRN_NullabilityMismatchInReturnTypeOnPartial = 8819,

ERR_StaticAnonymousFunctionCannotCaptureVariable = 8820,
Expand Down Expand Up @@ -2317,6 +2317,8 @@ internal enum ErrorCode
ERR_PartialPropertyDuplicateImplementation = 9303,
ERR_PartialPropertyMissingAccessor = 9304,
ERR_PartialPropertyUnexpectedAccessor = 9305,
ERR_PartialPropertyInitMismatch = 9306,
ERR_PartialPropertyTypeDifference = 9307,

#endregion

Expand Down
14 changes: 8 additions & 6 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_PartialMethodMustHaveLatent:
case ErrorCode.ERR_PartialMethodInconsistentConstraints:
case ErrorCode.ERR_PartialMethodToDelegate:
case ErrorCode.ERR_PartialMethodStaticDifference:
case ErrorCode.ERR_PartialMemberStaticDifference:
case ErrorCode.ERR_PartialMethodUnsafeDifference:
case ErrorCode.ERR_PartialMethodInExpressionTree:
case ErrorCode.ERR_ExplicitImplCollisionOnRefOut:
Expand Down Expand Up @@ -1744,7 +1744,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_CantChangeTupleNamesOnOverride:
case ErrorCode.ERR_DuplicateInterfaceWithTupleNamesInBaseList:
case ErrorCode.ERR_ImplBadTupleNames:
case ErrorCode.ERR_PartialMethodInconsistentTupleNames:
case ErrorCode.ERR_PartialMemberInconsistentTupleNames:
case ErrorCode.ERR_ExpressionTreeContainsTupleLiteral:
case ErrorCode.ERR_ExpressionTreeContainsTupleConversion:
case ErrorCode.ERR_AutoPropertyCannotBeRefReturning:
Expand Down Expand Up @@ -1983,7 +1983,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_InvalidPropertyReadOnlyMods:
case ErrorCode.ERR_DuplicatePropertyReadOnlyMods:
case ErrorCode.ERR_FieldLikeEventCantBeReadOnly:
case ErrorCode.ERR_PartialMethodReadOnlyDifference:
case ErrorCode.ERR_PartialMemberReadOnlyDifference:
case ErrorCode.ERR_ReadOnlyModMissingAccessor:
case ErrorCode.ERR_OverrideRefConstraintNotSatisfied:
case ErrorCode.ERR_OverrideValConstraintNotSatisfied:
Expand Down Expand Up @@ -2053,8 +2053,8 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_PartialMethodWithNonVoidReturnMustHaveAccessMods:
case ErrorCode.ERR_PartialMethodWithOutParamMustHaveAccessMods:
case ErrorCode.ERR_PartialMethodWithExtendedModMustHaveAccessMods:
case ErrorCode.ERR_PartialMethodAccessibilityDifference:
case ErrorCode.ERR_PartialMethodExtendedModDifference:
case ErrorCode.ERR_PartialMemberAccessibilityDifference:
case ErrorCode.ERR_PartialMemberExtendedModDifference:
case ErrorCode.ERR_SimpleProgramLocalIsReferencedOutsideOfTopLevelStatement:
case ErrorCode.ERR_SimpleProgramMultipleUnitsWithTopLevelStatements:
case ErrorCode.ERR_TopLevelStatementAfterNamespaceOrType:
Expand All @@ -2072,7 +2072,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_ModuleInitializerMethodMustBeStaticParameterlessVoid:
case ErrorCode.ERR_ModuleInitializerMethodAndContainingTypesMustNotBeGeneric:
case ErrorCode.ERR_PartialMethodReturnTypeDifference:
case ErrorCode.ERR_PartialMethodRefReturnDifference:
case ErrorCode.ERR_PartialMemberRefReturnDifference:
case ErrorCode.WRN_NullabilityMismatchInReturnTypeOnPartial:
case ErrorCode.ERR_StaticAnonymousFunctionCannotCaptureVariable:
case ErrorCode.ERR_StaticAnonymousFunctionCannotCaptureThis:
Expand Down Expand Up @@ -2445,6 +2445,8 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_PartialPropertyDuplicateImplementation:
case ErrorCode.ERR_PartialPropertyMissingAccessor:
case ErrorCode.ERR_PartialPropertyUnexpectedAccessor:
case ErrorCode.ERR_PartialPropertyInitMismatch:
case ErrorCode.ERR_PartialPropertyTypeDifference:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,7 @@ internal static bool IsExplicitInterfaceImplementation(this Symbol member)

internal static bool IsPartialMember(this Symbol member)
{
Debug.Assert(member.IsDefinition);
return member
is SourceOrdinaryMethodSymbol { IsPartial: true }
or SourcePropertySymbol { IsPartial: true }
Expand All @@ -555,6 +556,7 @@ internal static bool IsPartialMember(this Symbol member)

internal static bool IsPartialImplementation(this Symbol member)
{
Debug.Assert(member.IsDefinition);
return member
is SourceOrdinaryMethodSymbol { IsPartialImplementation: true }
or SourcePropertySymbol { IsPartialImplementation: true }
Expand All @@ -563,6 +565,7 @@ internal static bool IsPartialImplementation(this Symbol member)

internal static bool IsPartialDefinition(this Symbol member)
{
Debug.Assert(member.IsDefinition);
return member
is SourceOrdinaryMethodSymbol { IsPartialDefinition: true }
or SourcePropertySymbol { IsPartialDefinition: true }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,10 @@ protected sealed override void PartialMethodChecks(BindingDiagnosticBag diagnost
/// parts of a partial method. Diagnostics are reported on the
/// implementing part, matching Dev10 behavior.
/// </summary>
/// <remarks>
/// This method is analogous to <see cref="SourcePropertySymbol.PartialPropertyChecks" />.
/// Whenever new checks are added to this method, the other method should also have those checks added, if applicable.
/// </remarks>
private static void PartialMethodChecks(SourceOrdinaryMethodSymbol definition, SourceOrdinaryMethodSymbol implementation, BindingDiagnosticBag diagnostics)
{
Debug.Assert(!ReferenceEquals(definition, implementation));
Expand All @@ -484,22 +488,22 @@ private static void PartialMethodChecks(SourceOrdinaryMethodSymbol definition, S
else if (MemberSignatureComparer.ConsideringTupleNamesCreatesDifference(definition, implementation))
{
hasTypeDifferences = true;
diagnostics.Add(ErrorCode.ERR_PartialMethodInconsistentTupleNames, implementation.GetFirstLocation(), definition, implementation);
diagnostics.Add(ErrorCode.ERR_PartialMemberInconsistentTupleNames, implementation.GetFirstLocation(), definition, implementation);
}

if (definition.RefKind != implementation.RefKind)
{
diagnostics.Add(ErrorCode.ERR_PartialMethodRefReturnDifference, implementation.GetFirstLocation());
diagnostics.Add(ErrorCode.ERR_PartialMemberRefReturnDifference, implementation.GetFirstLocation());
}

if (definition.IsStatic != implementation.IsStatic)
{
diagnostics.Add(ErrorCode.ERR_PartialMethodStaticDifference, implementation.GetFirstLocation());
diagnostics.Add(ErrorCode.ERR_PartialMemberStaticDifference, implementation.GetFirstLocation());
}

if (definition.IsDeclaredReadOnly != implementation.IsDeclaredReadOnly)
{
diagnostics.Add(ErrorCode.ERR_PartialMethodReadOnlyDifference, implementation.GetFirstLocation());
diagnostics.Add(ErrorCode.ERR_PartialMemberReadOnlyDifference, implementation.GetFirstLocation());
}

if (definition.IsExtensionMethod != implementation.IsExtensionMethod)
Expand All @@ -520,15 +524,15 @@ private static void PartialMethodChecks(SourceOrdinaryMethodSymbol definition, S
if (definition.HasExplicitAccessModifier != implementation.HasExplicitAccessModifier
|| definition.DeclaredAccessibility != implementation.DeclaredAccessibility)
{
diagnostics.Add(ErrorCode.ERR_PartialMethodAccessibilityDifference, implementation.GetFirstLocation());
diagnostics.Add(ErrorCode.ERR_PartialMemberAccessibilityDifference, implementation.GetFirstLocation());
}

if (definition.IsVirtual != implementation.IsVirtual
|| definition.IsOverride != implementation.IsOverride
|| definition.IsSealed != implementation.IsSealed
|| definition.IsNew != implementation.IsNew)
{
diagnostics.Add(ErrorCode.ERR_PartialMethodExtendedModDifference, implementation.GetFirstLocation());
diagnostics.Add(ErrorCode.ERR_PartialMemberExtendedModDifference, implementation.GetFirstLocation());
}

PartialMethodConstraintsChecks(definition, implementation, diagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,18 @@ internal sealed override ImmutableArray<string> NotNullWhenFalseMembers
bool isNullableAnalysisEnabled,
BindingDiagnosticBag diagnostics) :
base(containingType, syntax.GetReference(), location, isIterator: false,
MakeModifiersAndFlags(property, propertyModifiers, isNullableAnalysisEnabled))
MakeModifiersAndFlags(
containingType,
property,
propertyModifiers,
location,
hasBlockBody: false,
hasExpressionBody: true,
modifiers: [],
methodKind: MethodKind.PropertyGet,
isNullableAnalysisEnabled,
diagnostics,
out var modifierErrors))
{
_property = property;
_isAutoPropertyAccessor = false;
Expand All @@ -158,21 +169,6 @@ internal sealed override ImmutableArray<string> NotNullWhenFalseMembers
this.CheckModifiers(location, hasBody: true, isAutoPropertyOrExpressionBodied: true, diagnostics: diagnostics);
}

private static (DeclarationModifiers, Flags) MakeModifiersAndFlags(SourcePropertySymbol property, DeclarationModifiers propertyModifiers, bool isNullableAnalysisEnabled)
{
// The modifiers for the accessor are the same as the modifiers for the property,
// minus the indexer and readonly bit
var declarationModifiers = GetAccessorModifiers(propertyModifiers);

// ReturnsVoid property is overridden in this class so
// returnsVoid argument to MakeFlags is ignored.
Flags flags = MakeFlags(MethodKind.PropertyGet, property.RefKind, declarationModifiers, returnsVoid: false, returnsVoidIsSet: false,
isExpressionBodied: true, isExtensionMethod: false, isNullableAnalysisEnabled: isNullableAnalysisEnabled,
isVarArg: false, isExplicitInterfaceImplementation: property.IsExplicitInterfaceImplementation, hasThisInitializer: false);

return (declarationModifiers, flags);
}

#nullable enable
protected SourcePropertyAccessorSymbol(
NamedTypeSymbol containingType,
Expand Down Expand Up @@ -821,5 +817,27 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
internal bool IsPartialImplementation => _property is SourcePropertySymbol { IsPartialImplementation: true };

public sealed override bool IsExtern => PartialImplementationPart is { } implementation ? implementation.IsExtern : base.IsExtern;

internal void PartialAccessorChecks(SourcePropertyAccessorSymbol implementationAccessor, BindingDiagnosticBag diagnostics)
{
Debug.Assert(IsPartialDefinition);

if (LocalAccessibility != implementationAccessor.LocalAccessibility)
{
diagnostics.Add(ErrorCode.ERR_PartialMemberAccessibilityDifference, implementationAccessor.GetFirstLocation());
}

if (_property.HasReadOnlyModifier == implementationAccessor._property.HasReadOnlyModifier
&& IsDeclaredReadOnly != implementationAccessor.IsDeclaredReadOnly)
{
diagnostics.Add(ErrorCode.ERR_PartialMemberReadOnlyDifference, implementationAccessor.GetFirstLocation());
}

if (_usesInit != implementationAccessor._usesInit)
{
var accessorName = _usesInit ? "init" : "get";
diagnostics.Add(ErrorCode.ERR_PartialPropertyInitMismatch, implementationAccessor.GetFirstLocation(), implementationAccessor, accessorName);
}
}
}
}

0 comments on commit 6718f15

Please sign in to comment.