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

Report a diagnostic on missing body in partial property implementation #74224

Merged
merged 2 commits into from
Jul 2, 2024
Merged

Report a diagnostic on missing body in partial property implementation #74224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6362bd6
Report a diagnostic on missing body in partial property implementation
RikkiGibson Jul 1, 2024
a2b5d4f
formatting
RikkiGibson Jul 1, 2024
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
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ private void CheckModifiers(Location location, bool hasBody, bool isAutoProperty
// '{0}' is a new virtual member in sealed type '{1}'
diagnostics.Add(ErrorCode.ERR_NewVirtualInSealed, location, this, ContainingType);
}
else if (!hasBody && !IsExtern && !IsAbstract && !isAutoPropertyOrExpressionBodied && !IsPartial)
else if (!hasBody && !IsExtern && !IsAbstract && !isAutoPropertyOrExpressionBodied && !IsPartialDefinition)
{
diagnostics.Add(ErrorCode.ERR_ConcreteMissingBody, location, this);
}
Expand Down
31 changes: 29 additions & 2 deletions src/Compilers/CSharp/Test/Symbol/Symbols/PartialPropertiesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1512,10 +1512,10 @@ partial class C
// (6,17): error CS0274: Cannot specify accessibility modifiers for both accessors of the property or indexer 'C.P2'
// partial int P2 { private get; private set; }
Diagnostic(ErrorCode.ERR_DuplicatePropertyAccessMods, "P2").WithArguments("C.P2").WithLocation(6, 17),
// (6,30): error CS0273: The accessibility modifier of the 'C.P2.get' accessor must be more restrictive than the property or indexer 'C.P2'
// (6,30): error CS0273: The accessibility modifier of the 'C.P2.get' accessor must be more restrictive than the property or indexer 'C.P2'
// partial int P2 { private get; private set; }
Diagnostic(ErrorCode.ERR_InvalidPropertyAccessMod, "get").WithArguments("C.P2.get", "C.P2").WithLocation(6, 30),
// (6,43): error CS0273: The accessibility modifier of the 'C.P2.set' accessor must be more restrictive than the property or indexer 'C.P2'
// (6,43): error CS0273: The accessibility modifier of the 'C.P2.set' accessor must be more restrictive than the property or indexer 'C.P2'
// partial int P2 { private get; private set; }
Diagnostic(ErrorCode.ERR_InvalidPropertyAccessMod, "set").WithArguments("C.P2.set", "C.P2").WithLocation(6, 43),
// (11,17): error CS8799: Both partial member declarations must have identical accessibility modifiers.
Expand All @@ -1524,6 +1524,9 @@ partial class C
// (11,30): error CS8799: Both partial member declarations must have identical accessibility modifiers.
// partial int P1 { private get => 1; private set; }
Diagnostic(ErrorCode.ERR_PartialMemberAccessibilityDifference, "get").WithLocation(11, 30),
// (11,48): error CS0501: 'C.P1.set' must declare a body because it is not marked abstract, extern, or partial
// partial int P1 { private get => 1; private set; }
Diagnostic(ErrorCode.ERR_ConcreteMissingBody, "set").WithArguments("C.P1.set").WithLocation(11, 48),
// (11,48): error CS8799: Both partial member declarations must have identical accessibility modifiers.
// partial int P1 { private get => 1; private set; }
Diagnostic(ErrorCode.ERR_PartialMemberAccessibilityDifference, "set").WithLocation(11, 48));
Expand Down Expand Up @@ -4973,5 +4976,29 @@ partial class C
Assert.Equal("SourceFile(Program.cs[52..53))", defSymbol.Locations.Single().ToString());
Assert.Equal("SourceFile(Program.cs[97..98))", implSymbol.Locations.Single().ToString());
}

[Fact]
public void OnlyOneAccessorHasBodyOnImplementation()
{
var source = """
partial class C
{
public partial int Prop1 { get; set; }
public partial int Prop1 { get => 1; set; }
Copy link
Contributor Author

@RikkiGibson RikkiGibson Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Note that we already have coverage for when a partial property has no accessors with bodies. In that case, the language considers it to be a definition part, and we don't want to report errors on the missing bodies. Basically, the adjusted code path is only going to be hit for partial properties which have 1 accessor with a body, 1 accessor without.


public partial int Prop2 { get; set; }
public partial int Prop2 { get; set { } }
}
""";

var comp = CreateCompilation(source);
comp.VerifyEmitDiagnostics(
// (4,42): error CS0501: 'C.Prop1.set' must declare a body because it is not marked abstract, extern, or partial
// public partial int Prop1 { get => 1; set; }
Diagnostic(ErrorCode.ERR_ConcreteMissingBody, "set").WithArguments("C.Prop1.set").WithLocation(4, 42),
// (7,32): error CS0501: 'C.Prop2.get' must declare a body because it is not marked abstract, extern, or partial
// public partial int Prop2 { get; set { } }
Diagnostic(ErrorCode.ERR_ConcreteMissingBody, "get").WithArguments("C.Prop2.get").WithLocation(7, 32));
}
}
}
Loading