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

Produce errors for 'partial file record' #62686

Merged
merged 5 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
6 changes: 2 additions & 4 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,10 +1212,8 @@ private void ParseModifiers(SyntaxListBuilder tokens, bool forAccessors, bool fo
nextToken.Kind == SyntaxKind.DelegateKeyword ||
(IsPossibleStartOfTypeDeclaration(nextToken.Kind) && GetModifier(nextToken) != DeclarationModifiers.None))
{
// Misplaced partial
// TODO(https://github.com/dotnet/roslyn/issues/22439):
// We should consider moving this check into binding, but avoid holding on to trees
modTok = AddError(ConvertToKeyword(this.EatToken()), ErrorCode.ERR_PartialMisplaced);
// Error reported in ModifierUtils.
modTok = ConvertToKeyword(this.EatToken());
}
else
{
Expand Down
26 changes: 18 additions & 8 deletions src/Compilers/CSharp/Portable/Symbols/Source/ModifierUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
internal static class ModifierUtils
{
internal static DeclarationModifiers MakeAndCheckNontypeMemberModifiers(
bool isForTypeDeclaration,
bool isOrdinaryMethod,
bool isForInterfaceMember,
SyntaxTokenList modifiers,
DeclarationModifiers defaultAccess,
Expand All @@ -21,8 +21,8 @@ internal static class ModifierUtils
BindingDiagnosticBag diagnostics,
out bool modifierErrors)
{
var result = modifiers.ToDeclarationModifiers(diagnostics.DiagnosticBag ?? new DiagnosticBag());
result = CheckModifiers(isForTypeDeclaration, isForInterfaceMember, result, allowedModifiers, errorLocation, diagnostics, modifiers, out modifierErrors);
var result = modifiers.ToDeclarationModifiers(diagnostics.DiagnosticBag ?? new DiagnosticBag(), isOrdinaryMethod: isOrdinaryMethod);
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
result = CheckModifiers(isForTypeDeclaration: false, isForInterfaceMember, result, allowedModifiers, errorLocation, diagnostics, modifiers, out modifierErrors);

if ((result & DeclarationModifiers.AccessibilityMask) == 0)
{
Expand Down Expand Up @@ -382,21 +382,32 @@ private static DeclarationModifiers ToDeclarationModifier(SyntaxKind kind)
}

public static DeclarationModifiers ToDeclarationModifiers(
this SyntaxTokenList modifiers, DiagnosticBag diagnostics)
this SyntaxTokenList modifiers, DiagnosticBag diagnostics, bool isOrdinaryMethod = false)
{
var result = DeclarationModifiers.None;
bool seenNoDuplicates = true;
bool seenNoAccessibilityDuplicates = true;

foreach (var modifier in modifiers)
for (int i = 0; i < modifiers.Count; i++)
{
SyntaxToken modifier = modifiers[i];
DeclarationModifiers one = ToDeclarationModifier(modifier.ContextualKind());

ReportDuplicateModifiers(
modifier, one, result,
ref seenNoDuplicates, ref seenNoAccessibilityDuplicates,
ref seenNoDuplicates,
diagnostics);

if (one == DeclarationModifiers.Partial && i < modifiers.Count - 1)
{
// There was a bug where we allowed `partial async` at the end of modifiers on methods. We keep this behavior for backcompat.
if (!(isOrdinaryMethod && i == modifiers.Count - 2 && ToDeclarationModifier(modifiers[i + 1].ContextualKind()) == DeclarationModifiers.Async))
{
diagnostics.Add(
ErrorCode.ERR_PartialMisplaced,
modifier.GetLocation());
}
}

result |= one;
}

Expand All @@ -423,7 +434,6 @@ private static DeclarationModifiers ToDeclarationModifier(SyntaxKind kind)
DeclarationModifiers modifierKind,
DeclarationModifiers allModifiers,
ref bool seenNoDuplicates,
ref bool seenNoAccessibilityDuplicates,
DiagnosticBag diagnostics)
{
if ((allModifiers & modifierKind) != 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, MethodKind
DeclarationModifiers.Unsafe;

bool isInterface = ContainingType.IsInterface;
var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isForTypeDeclaration: false, isForInterfaceMember: isInterface, modifiers, defaultAccess, allowedModifiers, location, diagnostics, out modifierErrors);
var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isOrdinaryMethod: false, isForInterfaceMember: isInterface, modifiers, defaultAccess, allowedModifiers, location, diagnostics, out modifierErrors);

this.CheckUnsafeModifier(mods, diagnostics);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, Location l
{
// Check that the set of modifiers is allowed
const DeclarationModifiers allowedModifiers = DeclarationModifiers.Extern | DeclarationModifiers.Unsafe;
var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isForTypeDeclaration: false, isForInterfaceMember: ContainingType.IsInterface, modifiers, DeclarationModifiers.None, allowedModifiers, location, diagnostics, out modifierErrors);
var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isOrdinaryMethod: false, isForInterfaceMember: ContainingType.IsInterface, modifiers, DeclarationModifiers.None, allowedModifiers, location, diagnostics, out modifierErrors);

this.CheckUnsafeModifier(mods, diagnostics);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ private void CheckAccessibility(Location location, BindingDiagnosticBag diagnost
allowedModifiers |= DeclarationModifiers.Extern;
}

var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isForTypeDeclaration: false, isForInterfaceMember: isInterface,
var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isOrdinaryMethod: false, isForInterfaceMember: isInterface,
modifiers, defaultAccess, allowedModifiers, location, diagnostics, out modifierErrors);

ModifierUtils.CheckFeatureAvailabilityForStaticAbstractMembersInInterfacesIfNeeded(mods, explicitInterfaceImplementation, location, diagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ internal static DeclarationModifiers MakeModifiers(NamedTypeSymbol containingTyp

var errorLocation = new SourceLocation(firstIdentifier);
DeclarationModifiers result = ModifierUtils.MakeAndCheckNontypeMemberModifiers(
isForTypeDeclaration: false, isForInterfaceMember: isInterface,
isOrdinaryMethod: false, isForInterfaceMember: isInterface,
modifiers, defaultAccess, allowedModifiers, errorLocation, diagnostics, out modifierErrors);

if ((result & DeclarationModifiers.Abstract) != 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ internal override bool IsExpressionBodied
protected override DeclarationModifiers MakeDeclarationModifiers(DeclarationModifiers allowedModifiers, BindingDiagnosticBag diagnostics)
{
var syntax = GetSyntax();
return ModifierUtils.MakeAndCheckNontypeMemberModifiers(isForTypeDeclaration: false, isForInterfaceMember: ContainingType.IsInterface,
return ModifierUtils.MakeAndCheckNontypeMemberModifiers(isOrdinaryMethod: true, isForInterfaceMember: ContainingType.IsInterface,
syntax.Modifiers, defaultAccess: DeclarationModifiers.None, allowedModifiers, Locations[0], diagnostics, out _);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ internal sealed override bool IsDeclaredReadOnly
defaultInterfaceImplementationModifiers = DeclarationModifiers.AccessibilityMask;
}

var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isForTypeDeclaration: false, isForInterfaceMember: isInterface,
var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isOrdinaryMethod: false, isForInterfaceMember: isInterface,
modifiers, defaultAccess, allowedModifiers, location, diagnostics, out modifierErrors);

ModifierUtils.ReportDefaultInterfaceImplementationModifiers(hasBody, mods,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ private static AccessorDeclarationSyntax GetSetAccessorDeclaration(BasePropertyD

allowedModifiers |= DeclarationModifiers.Extern;

var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isForTypeDeclaration: false, isForInterfaceMember: isInterface,
var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(isOrdinaryMethod: false, isForInterfaceMember: isInterface,
modifiers, defaultAccess, allowedModifiers, location, diagnostics, out modifierErrors);

ModifierUtils.CheckFeatureAvailabilityForStaticAbstractMembersInInterfacesIfNeeded(mods, isExplicitInterfaceImplementation, location, diagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ protected static DeclarationModifiers MakeDeclarationModifiers(MethodKind method
}

var result = ModifierUtils.MakeAndCheckNontypeMemberModifiers(
isForTypeDeclaration: false, isForInterfaceMember: inInterface,
isOrdinaryMethod: false, isForInterfaceMember: inInterface,
syntax.Modifiers, defaultAccess, allowedModifiers, location, diagnostics, modifierErrors: out _);

if (inInterface)
Expand Down
75 changes: 75 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6782,5 +6782,80 @@ static void Main()
Assert.Equal(RefKind.In, lambdas[1].Parameters[0].RefKind);
Assert.Equal(RefKind.Out, lambdas[2].Parameters[0].RefKind);
}

[Fact]
public void StaticPartialLambda()
Comment on lines +6786 to +6787
Copy link
Member Author

Choose a reason for hiding this comment

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

If in future the parser allowed partial, we should update this code path:

ModifierUtils.ToDeclarationModifiers(syntax.Modifiers, diagnostics.DiagnosticBag ?? new DiagnosticBag());
to report ERR_PartialMisplaced.

This test makes sure this will not be forgotten.

{
CreateCompilation("""
class C
{
void M()
{
System.Action x = static partial () => { };
}
}
""").VerifyDiagnostics(
// (5,27): error CS8934: Cannot convert lambda expression to type 'Action' because the return type does not match the delegate return type
// System.Action x = static partial () => { };
Diagnostic(ErrorCode.ERR_CantConvAnonMethReturnType, "static partial () => { }").WithArguments("lambda expression", "System.Action").WithLocation(5, 27),
// (5,34): error CS0246: The type or namespace name 'partial' could not be found (are you missing a using directive or an assembly reference?)
// System.Action x = static partial () => { };
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "partial").WithArguments("partial").WithLocation(5, 34));
}

[Fact]
public void PartialStaticLambda()
{
CreateCompilation("""
class C
{
void M()
{
System.Action x = partial static () => { };
}
}
""").VerifyDiagnostics(
// (5,27): error CS0103: The name 'partial' does not exist in the current context
// System.Action x = partial static () => { };
Diagnostic(ErrorCode.ERR_NameNotInContext, "partial").WithArguments("partial").WithLocation(5, 27),
// (5,35): error CS1002: ; expected
// System.Action x = partial static () => { };
Diagnostic(ErrorCode.ERR_SemicolonExpected, "static").WithLocation(5, 35),
// (5,35): error CS0106: The modifier 'static' is not valid for this item
// System.Action x = partial static () => { };
Diagnostic(ErrorCode.ERR_BadMemberFlag, "static").WithArguments("static").WithLocation(5, 35),
// (5,43): error CS8124: Tuple must contain at least two elements.
// System.Action x = partial static () => { };
Diagnostic(ErrorCode.ERR_TupleTooFewElements, ")").WithLocation(5, 43),
// (5,45): error CS1001: Identifier expected
// System.Action x = partial static () => { };
Diagnostic(ErrorCode.ERR_IdentifierExpected, "=>").WithLocation(5, 45),
// (5,45): error CS1003: Syntax error, ',' expected
// System.Action x = partial static () => { };
Diagnostic(ErrorCode.ERR_SyntaxError, "=>").WithArguments(",").WithLocation(5, 45),
// (5,48): error CS1002: ; expected
// System.Action x = partial static () => { };
Diagnostic(ErrorCode.ERR_SemicolonExpected, "{").WithLocation(5, 48));
}

[Fact]
public void PartialLambda()
{
CreateCompilation("""
class C
{
void M()
{
System.Action x = partial () => { };
}
}
""").VerifyDiagnostics(
// (5,27): error CS0246: The type or namespace name 'partial' could not be found (are you missing a using directive or an assembly reference?)
// System.Action x = partial () => { };
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "partial").WithArguments("partial").WithLocation(5, 27),
// (5,27): error CS8934: Cannot convert lambda expression to type 'Action' because the return type does not match the delegate return type
// System.Action x = partial () => { };
Diagnostic(ErrorCode.ERR_CantConvAnonMethReturnType, "partial () => { }").WithArguments("lambda expression", "System.Action").WithLocation(5, 27));
}
}
}
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/Symbols/EnumTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -399,5 +399,17 @@ static void Main()
Diagnostic(ErrorCode.ERR_NoTypeDef, "F").WithArguments("A", "UseSiteError_sourceA, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null").WithLocation(5, 20)
);
}

[Fact]
public void PartialPublicEnum()
{
CreateCompilation("partial public enum E { }").VerifyDiagnostics(
// (1,1): error CS0267: The 'partial' modifier can only appear immediately before 'class', 'record', 'struct', 'interface', or a method return type.
// partial public enum E { }
Diagnostic(ErrorCode.ERR_PartialMisplaced, "partial").WithLocation(1, 1),
// (1,21): error CS0267: The 'partial' modifier can only appear immediately before 'class', 'record', 'struct', 'interface', or a method return type.
// partial public enum E { }
Diagnostic(ErrorCode.ERR_PartialMisplaced, "E").WithLocation(1, 21));
Comment on lines +406 to +412
Copy link
Member Author

Choose a reason for hiding this comment

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

The duplicate diagnostic is an existing behavior. But whether you want me to fix that in this PR or in a follow-up, I'm okay with that.

}
}
}
87 changes: 87 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/Symbols/LocalFunctionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,5 +102,92 @@ void M()
var staticLocal = semanticModel.GetDeclaredSymbol(localsSyntax[0]).GetSymbol<MethodSymbol>();
Assert.False(staticLocal.RequiresInstanceReceiver);
}

[Fact]
public void PartialStaticLocalFunction()
{
CreateCompilation("""
public class C
{
public void M()
{
partial static void local() { }
}
}
""").VerifyDiagnostics(
// (5,9): error CS0103: The name 'partial' does not exist in the current context
// partial static void local() { }
Diagnostic(ErrorCode.ERR_NameNotInContext, "partial").WithArguments("partial").WithLocation(5, 9),
// (5,17): error CS1002: ; expected
// partial static void local() { }
Diagnostic(ErrorCode.ERR_SemicolonExpected, "static").WithLocation(5, 17),
// (5,29): warning CS8321: The local function 'local' is declared but never used
// partial static void local() { }
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "local").WithArguments("local").WithLocation(5, 29));
}

[Fact]
public void StaticPartialLocalFunction()
Comment on lines +129 to +130
Copy link
Member Author

Choose a reason for hiding this comment

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

This test serves the same purpose as the one I commented on for lambdas, but for this code path:

_declarationModifiers =
DeclarationModifiers.Private |
syntax.Modifiers.ToDeclarationModifiers(diagnostics: _declarationDiagnostics.DiagnosticBag);

{
CreateCompilation("""
public class C
{
public void M()
{
static partial void local() { }
}
}
""").VerifyDiagnostics(
// (5,9): error CS0106: The modifier 'static' is not valid for this item
// static partial void local() { }
Diagnostic(ErrorCode.ERR_BadMemberFlag, "static").WithArguments("static").WithLocation(5, 9),
// (5,16): error CS1031: Type expected
// static partial void local() { }
Diagnostic(ErrorCode.ERR_TypeExpected, "partial").WithLocation(5, 16),
// (5,16): error CS1525: Invalid expression term 'partial'
// static partial void local() { }
Diagnostic(ErrorCode.ERR_InvalidExprTerm, "partial").WithArguments("partial").WithLocation(5, 16),
// (5,16): error CS1002: ; expected
// static partial void local() { }
Diagnostic(ErrorCode.ERR_SemicolonExpected, "partial").WithLocation(5, 16),
// (5,16): error CS1513: } expected
// static partial void local() { }
Diagnostic(ErrorCode.ERR_RbraceExpected, "partial").WithLocation(5, 16),
// (5,29): error CS0759: No defining declaration found for implementing declaration of partial method 'C.local()'
// static partial void local() { }
Diagnostic(ErrorCode.ERR_PartialMethodMustHaveLatent, "local").WithArguments("C.local()").WithLocation(5, 29),
// (5,29): error CS0751: A partial method must be declared within a partial type
// static partial void local() { }
Diagnostic(ErrorCode.ERR_PartialMethodOnlyInPartialClass, "local").WithLocation(5, 29),
// (7,1): error CS1022: Type or namespace definition, or end-of-file expected
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(7, 1));
}

[Fact]
public void PartialLocalFunction()
{
CreateCompilation("""
public class C
{
public void M()
{
partial void local() { }
}
}
""").VerifyDiagnostics(
// (4,6): error CS1513: } expected
// {
Diagnostic(ErrorCode.ERR_RbraceExpected, "").WithLocation(4, 6),
// (5,22): error CS0759: No defining declaration found for implementing declaration of partial method 'C.local()'
// partial void local() { }
Diagnostic(ErrorCode.ERR_PartialMethodMustHaveLatent, "local").WithArguments("C.local()").WithLocation(5, 22),
// (5,22): error CS0751: A partial method must be declared within a partial type
// partial void local() { }
Diagnostic(ErrorCode.ERR_PartialMethodOnlyInPartialClass, "local").WithLocation(5, 22),
// (7,1): error CS1022: Type or namespace definition, or end-of-file expected
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(7, 1));
}
}
}
Loading