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

Jump to bottom

Partial properties: diagnostics for mismatch between parts #73250

Merged
merged 19 commits into from
May 7, 2024
Merged

Partial properties: diagnostics for mismatch between parts #73250

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
6718f15
Partial properties: diagnostics for mismatch between parts
RikkiGibson May 1, 2024
7a6e350
update baselines
RikkiGibson May 1, 2024
5ca6918
fix init mismatch diagnostic
RikkiGibson May 1, 2024
c971937
test extended mod difference
RikkiGibson May 1, 2024
5b43d37
diagnose nullable difference
RikkiGibson May 1, 2024
e2d6baf
adjust unsafe tests
RikkiGibson May 1, 2024
f3bbced
Update src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTest…
RikkiGibson May 1, 2024
155e9c5
fix formatting
RikkiGibson May 1, 2024
63af93e
Fix test failures
RikkiGibson May 2, 2024
fbfdb04
Update diagnostic name and test multiple type-related differences at …
RikkiGibson May 2, 2024
c85cb1f
move tests and address other feedback
RikkiGibson May 2, 2024
f8a3d04
add nullable test with nullable warnings disabled
RikkiGibson May 2, 2024
c064500
Fix formatting
RikkiGibson May 2, 2024
511c209
Simplify readonly check in accessors
RikkiGibson May 2, 2024
3bb56aa
Address feedback
RikkiGibson May 2, 2024
265ead8
Test unsafe modifier in partial when compilation disallows unsafe
RikkiGibson May 2, 2024
692624e
Adjust required modifier tests
RikkiGibson May 7, 2024
4e1520f
Diagnostics for partial property not in partial type and partial prop…
RikkiGibson May 7, 2024
01f15e2
fix formatting
RikkiGibson May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 26 additions & 14 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2234,20 +2234,20 @@ 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>
<data name="ERR_PartialMemberUnsafeDifference" xml:space="preserve">
<value>Both partial member declarations must be unsafe or neither may be unsafe</value>
</data>
<data name="ERR_PartialMethodInExpressionTree" xml:space="preserve">
<value>Partial methods with only a defining declaration or removed conditional methods cannot be used in expression trees</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,16 @@ 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>
<data name="WRN_PartialPropertySignatureDifference" xml:space="preserve">
<value>Partial property declarations '{0}' and '{1}' have signature differences.</value>
</data>
<data name="WRN_PartialPropertySignatureDifference_Title" xml:space="preserve">
<value>Partial property declarations have signature differences.</value>
</data>
</root>
17 changes: 10 additions & 7 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,8 +541,8 @@ internal enum ErrorCode
ERR_PartialMethodMustHaveLatent = 759,
ERR_PartialMethodInconsistentConstraints = 761,
ERR_PartialMethodToDelegate = 762,
ERR_PartialMethodStaticDifference = 763,
ERR_PartialMethodUnsafeDifference = 764,
ERR_PartialMemberStaticDifference = 763,
ERR_PartialMemberUnsafeDifference = 764,
ERR_PartialMethodInExpressionTree = 765,
// ERR_PartialMethodMustReturnVoid = 766, Removed as part of 'extended partial methods' feature
ERR_ExplicitImplCollisionOnRefOut = 767,
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,9 @@ internal enum ErrorCode
ERR_PartialPropertyDuplicateImplementation = 9303,
ERR_PartialPropertyMissingAccessor = 9304,
ERR_PartialPropertyUnexpectedAccessor = 9305,
ERR_PartialPropertyInitMismatch = 9306,
ERR_PartialPropertyTypeDifference = 9307,
WRN_PartialPropertySignatureDifference = 9308,

#endregion

Expand Down
19 changes: 12 additions & 7 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,8 @@ internal static int GetWarningLevel(ErrorCode code)
case ErrorCode.WRN_DynamicDispatchToParamsCollectionIndexer:
case ErrorCode.WRN_DynamicDispatchToParamsCollectionConstructor:

case ErrorCode.WRN_PartialPropertySignatureDifference:
333fred marked this conversation as resolved.
Show resolved Hide resolved

return 1;
default:
return 0;
Expand Down Expand Up @@ -1079,8 +1081,8 @@ 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_PartialMethodUnsafeDifference:
case ErrorCode.ERR_PartialMemberStaticDifference:
case ErrorCode.ERR_PartialMemberUnsafeDifference:
case ErrorCode.ERR_PartialMethodInExpressionTree:
case ErrorCode.ERR_ExplicitImplCollisionOnRefOut:
case ErrorCode.ERR_IndirectRecursiveConstructorCall:
Expand Down Expand Up @@ -1744,7 +1746,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 +1985,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 +2055,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 +2074,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 +2447,9 @@ 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:
case ErrorCode.WRN_PartialPropertySignatureDifference:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Generated/ErrorFacts.Generated.cs
View file
Open in desktop

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

3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs
View file
Open in desktop
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
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/PublicModel/MethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,9 @@ IMethodSymbol IMethodSymbol.PartialDefinitionPart
}
}

bool IMethodSymbol.IsPartialDefinition => _underlying.IsPartialDefinition();
// PROTOTYPE(partial-properties): Perhaps this API should be implemented as '_underlying.OriginalDefinition.IsPartialDefinition()' instead.
// However, this would be a behavior change. Callers may have been assuming that if the API returned true, then the receiver is an original definition symbol.
bool IMethodSymbol.IsPartialDefinition => _underlying.IsDefinition && _underlying.IsPartialDefinition();
jcouv marked this conversation as resolved.
Show resolved Hide resolved

INamedTypeSymbol IMethodSymbol.AssociatedAnonymousDelegate
{
Expand Down
18 changes: 11 additions & 7 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs
View file
Open in desktop
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 @@ -509,7 +513,7 @@ private static void PartialMethodChecks(SourceOrdinaryMethodSymbol definition, S

if (definition.IsUnsafe != implementation.IsUnsafe && definition.CompilationAllowsUnsafe()) // Don't cascade.
{
diagnostics.Add(ErrorCode.ERR_PartialMethodUnsafeDifference, implementation.GetFirstLocation());
diagnostics.Add(ErrorCode.ERR_PartialMemberUnsafeDifference, implementation.GetFirstLocation());
}

if (definition.IsParams() != implementation.IsParams())
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