Skip to content

Commit

Permalink
Don't report modifier errors while parsing. (#16339)
Browse files Browse the repository at this point in the history 
* Don't report modifier errors while parsing.

* Fix bad merge.

* restore code.

* Fix test.

* Remove old resource

* Alternative way to do partial checking.

* Alternative way to do partial checking.

* Simplify formatting.

* Restore formatting.

* Restore formatting.

* Restore formatting.

* Restore formatting.

* Restore formatting.

* Give a proper message.

* Update error positions.

* Update error positions.

* Update error positions.

* Actually report error on bad modifier.
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Jul 17, 2017
1 parent 37abaef commit 001294b
Show file tree
Hide file tree
Showing 21 changed files with 509 additions and 228 deletions.
11 changes: 1 addition & 10 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs
View file

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

3 changes: 0 additions & 3 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2063,9 +2063,6 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_PartialMethodCannotHaveOutParameters" xml:space="preserve">
<value>A partial method cannot have out parameters</value>
</data>
<data name="ERR_PartialMethodOnlyMethods" xml:space="preserve">
<value>Only methods, classes, structs, or interfaces may be partial</value>
</data>
<data name="ERR_PartialMethodNotExplicit" xml:space="preserve">
<value>A partial method may not explicitly implement an interface method</value>
</data>
Expand Down
17 changes: 12 additions & 5 deletions src/Compilers/CSharp/Portable/Declarations/DeclarationTreeBuilder.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
Expand All @@ -9,7 +10,6 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
using System;
using CoreInternalSyntax = Microsoft.CodeAnalysis.Syntax.InternalSyntax;

namespace Microsoft.CodeAnalysis.CSharp
Expand Down Expand Up @@ -338,10 +338,12 @@ private SingleNamespaceOrTypeDeclaration VisitTypeDeclaration(TypeDeclarationSyn
var memberNames = GetNonTypeMemberNames(((Syntax.InternalSyntax.TypeDeclarationSyntax)(node.Green)).Members,
ref declFlags);

var modifiers = node.Modifiers.ToDeclarationModifiers(diagnostics: diagnostics);

return new SingleTypeDeclaration(
kind: kind,
name: node.Identifier.ValueText,
modifiers: node.Modifiers.ToDeclarationModifiers(),
modifiers: modifiers,
arity: node.Arity,
declFlags: declFlags,
syntaxReference: _syntaxTree.GetReference(node),
Expand Down Expand Up @@ -385,10 +387,12 @@ public override SingleNamespaceOrTypeDeclaration VisitDelegateDeclaration(Delega

declFlags |= SingleTypeDeclaration.TypeDeclarationFlags.HasAnyNontypeMembers;

var modifiers = node.Modifiers.ToDeclarationModifiers(diagnostics: diagnostics);

return new SingleTypeDeclaration(
kind: DeclarationKind.Delegate,
name: node.Identifier.ValueText,
modifiers: node.Modifiers.ToDeclarationModifiers(),
modifiers: modifiers,
declFlags: declFlags,
arity: node.Arity,
syntaxReference: _syntaxTree.GetReference(node),
Expand All @@ -413,17 +417,20 @@ public override SingleNamespaceOrTypeDeclaration VisitEnumDeclaration(EnumDeclar

string[] memberNames = GetEnumMemberNames(members, ref declFlags);

var diagnostics = DiagnosticBag.GetInstance();
var modifiers = node.Modifiers.ToDeclarationModifiers(diagnostics: diagnostics);

return new SingleTypeDeclaration(
kind: DeclarationKind.Enum,
name: node.Identifier.ValueText,
arity: 0,
modifiers: node.Modifiers.ToDeclarationModifiers(),
modifiers: modifiers,
declFlags: declFlags,
syntaxReference: _syntaxTree.GetReference(node),
nameLocation: new SourceLocation(node.Identifier),
memberNames: memberNames,
children: ImmutableArray<SingleTypeDeclaration>.Empty,
diagnostics: ImmutableArray<Diagnostic>.Empty);
diagnostics: diagnostics.ToReadOnlyAndFree());
}

private static string[] GetEnumMemberNames(SeparatedSyntaxList<EnumMemberDeclarationSyntax> members, ref SingleTypeDeclaration.TypeDeclarationFlags declFlags)
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ internal enum ErrorCode
ERR_PartialMethodInvalidModifier = 750,
ERR_PartialMethodOnlyInPartialClass = 751,
ERR_PartialMethodCannotHaveOutParameters = 752,
ERR_PartialMethodOnlyMethods = 753,
// ERR_PartialMethodOnlyMethods = 753, Removed as it is subsumed by ERR_PartialMisplaced
ERR_PartialMethodNotExplicit = 754,
ERR_PartialMethodExtensionDifference = 755,
ERR_PartialMethodOnlyOneLatent = 756,
Expand Down
195 changes: 65 additions & 130 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1102,74 +1102,57 @@ private AttributeArgumentSyntax ParseAttributeArgument()
nameEquals, nameColon, this.ParseExpressionCore());
}

[Flags]
private enum SyntaxModifier
private static DeclarationModifiers GetModifier(SyntaxToken token)
=> GetModifier(token.Kind, token.ContextualKind);

internal static DeclarationModifiers GetModifier(SyntaxKind kind, SyntaxKind contextualKind)
{
None = 0,
Public = 0x0001,
Internal = 0x0002,
Protected = 0x0004,
Private = 0x0008,
Sealed = 0x0010,
Abstract = 0x0020,
Static = 0x0040,
Virtual = 0x0080,
Extern = 0x0100,
New = 0x0200,
Override = 0x0400,
ReadOnly = 0x0800,
Volatile = 0x1000,
Unsafe = 0x2000,
Partial = 0x4000,
Async = 0x8000,
}

private const SyntaxModifier AccessModifiers = SyntaxModifier.Public | SyntaxModifier.Internal | SyntaxModifier.Protected | SyntaxModifier.Private;

private static SyntaxModifier GetModifier(SyntaxToken token)
{
switch (token.Kind)
switch (kind)
{
case SyntaxKind.PublicKeyword:
return SyntaxModifier.Public;
return DeclarationModifiers.Public;
case SyntaxKind.InternalKeyword:
return SyntaxModifier.Internal;
return DeclarationModifiers.Internal;
case SyntaxKind.ProtectedKeyword:
return SyntaxModifier.Protected;
return DeclarationModifiers.Protected;
case SyntaxKind.PrivateKeyword:
return SyntaxModifier.Private;
return DeclarationModifiers.Private;
case SyntaxKind.SealedKeyword:
return SyntaxModifier.Sealed;
return DeclarationModifiers.Sealed;
case SyntaxKind.AbstractKeyword:
return SyntaxModifier.Abstract;
return DeclarationModifiers.Abstract;
case SyntaxKind.StaticKeyword:
return SyntaxModifier.Static;
return DeclarationModifiers.Static;
case SyntaxKind.VirtualKeyword:
return SyntaxModifier.Virtual;
return DeclarationModifiers.Virtual;
case SyntaxKind.ExternKeyword:
return SyntaxModifier.Extern;
return DeclarationModifiers.Extern;
case SyntaxKind.NewKeyword:
return SyntaxModifier.New;
return DeclarationModifiers.New;
case SyntaxKind.OverrideKeyword:
return SyntaxModifier.Override;
return DeclarationModifiers.Override;
case SyntaxKind.ReadOnlyKeyword:
return SyntaxModifier.ReadOnly;
return DeclarationModifiers.ReadOnly;
case SyntaxKind.VolatileKeyword:
return SyntaxModifier.Volatile;
return DeclarationModifiers.Volatile;
case SyntaxKind.UnsafeKeyword:
return SyntaxModifier.Unsafe;
return DeclarationModifiers.Unsafe;
case SyntaxKind.PartialKeyword:
return DeclarationModifiers.Partial;
case SyntaxKind.AsyncKeyword:
return DeclarationModifiers.Async;
case SyntaxKind.IdentifierToken:
switch (token.ContextualKind)
switch (contextualKind)
{
case SyntaxKind.PartialKeyword:
return SyntaxModifier.Partial;
return DeclarationModifiers.Partial;
case SyntaxKind.AsyncKeyword:
return SyntaxModifier.Async;
return DeclarationModifiers.Async;
}

goto default;
default:
return SyntaxModifier.None;
return DeclarationModifiers.None;
}
}

Expand All @@ -1180,79 +1163,61 @@ private bool IsPossibleModifier()

private static bool IsPossibleModifier(SyntaxToken token)
{
return GetModifier(token) != SyntaxModifier.None;
return GetModifier(token) != DeclarationModifiers.None;
}

private void ParseModifiers(SyntaxListBuilder tokens)
{
SyntaxModifier mods = 0;
bool seenNoDuplicates = true;
bool seenNoAccessibilityDuplicates = true;

while (true)
{
var newMod = GetModifier(this.CurrentToken);
if (newMod == SyntaxModifier.None)
if (newMod == DeclarationModifiers.None)
{
break;
}

SyntaxToken modTok;
switch (newMod)
{
case SyntaxModifier.Partial:
case DeclarationModifiers.Partial:
var nextToken = PeekToken(1);
var isPartialType = this.IsPartialType();
var isPartialMember = this.IsPartialMember();
if (isPartialType || isPartialMember)
{
var nextToken = PeekToken(1);
if (this.IsPartialType())
{
modTok = ConvertToKeyword(this.EatToken());
modTok = CheckFeatureAvailability(modTok, MessageID.IDS_FeaturePartialTypes);
}
else if (this.IsPartialMember())
{
modTok = ConvertToKeyword(this.EatToken());
modTok = CheckFeatureAvailability(modTok, MessageID.IDS_FeaturePartialMethod);
}
else if (nextToken.Kind == SyntaxKind.NamespaceKeyword)
{
goto default;
}
else if (nextToken.Kind == SyntaxKind.EnumKeyword || nextToken.Kind == SyntaxKind.DelegateKeyword)
{
modTok = ConvertToKeyword(this.EatToken());
modTok = this.AddError(modTok, ErrorCode.ERR_PartialMisplaced);
}
else if (!IsPossibleStartOfTypeDeclaration(nextToken.Kind) || GetModifier(nextToken) == SyntaxModifier.None)
{
return;
}
else
{
modTok = ConvertToKeyword(this.EatToken());
modTok = this.AddError(modTok, ErrorCode.ERR_PartialMisplaced);
}

break;
// Standard legal cases.
modTok = ConvertToKeyword(this.EatToken());
modTok = CheckFeatureAvailability(modTok,
isPartialType ? MessageID.IDS_FeaturePartialTypes : MessageID.IDS_FeaturePartialMethod);
}
case SyntaxModifier.Async:
else if (
nextToken.Kind == SyntaxKind.EnumKeyword ||
nextToken.Kind == SyntaxKind.DelegateKeyword ||
nextToken.Kind == SyntaxKind.NamespaceKeyword ||
(IsPossibleStartOfTypeDeclaration(nextToken.Kind) && GetModifier(nextToken) != DeclarationModifiers.None))
{
// Error tolerance cases. Will report an error about these post parsing.
modTok = ConvertToKeyword(this.EatToken());
}
else
{
if (ShouldAsyncBeTreatedAsModifier(parsingStatementNotDeclaration: false))
{
modTok = ConvertToKeyword(this.EatToken());
modTok = CheckFeatureAvailability(modTok, MessageID.IDS_FeatureAsync);
break;
}
return;
}
default:

break;
case DeclarationModifiers.Async:
if (!ShouldAsyncBeTreatedAsModifier(parsingStatementNotDeclaration: false))
{
modTok = this.EatToken();
break;
return;
}
}

ReportDuplicateModifiers(ref modTok, newMod, mods, ref seenNoDuplicates, ref seenNoAccessibilityDuplicates);
mods |= newMod;
modTok = ConvertToKeyword(this.EatToken());
modTok = CheckFeatureAvailability(modTok, MessageID.IDS_FeatureAsync);
break;
default:
modTok = this.EatToken();
break;
}

tokens.Add(modTok);
}
Expand Down Expand Up @@ -1381,37 +1346,7 @@ private bool ShouldAsyncBeTreatedAsModifier(bool parsingStatementNotDeclaration)

private static bool IsNonContextualModifier(SyntaxToken nextToken)
{
return GetModifier(nextToken) != SyntaxModifier.None && !SyntaxFacts.IsContextualKeyword(nextToken.ContextualKind);
}

private void ReportDuplicateModifiers(ref SyntaxToken modTok, SyntaxModifier newMod, SyntaxModifier mods, ref bool seenNoDuplicates, ref bool seenNoAccessibilityDuplicates)
{
if ((mods & newMod) != 0)
{
if (seenNoDuplicates)
{
modTok = this.AddError(modTok, ErrorCode.ERR_DuplicateModifier, SyntaxFacts.GetText(modTok.Kind));
seenNoDuplicates = false;
}
}
else
{
if ((mods & AccessModifiers) != 0 && (newMod & AccessModifiers) != 0)
{
// Can't have two different access modifiers.
// Exception: "internal protected" or "protected internal" is allowed.
if (!(((newMod == SyntaxModifier.Protected) && (mods & SyntaxModifier.Internal) != 0) ||
((newMod == SyntaxModifier.Internal) && (mods & SyntaxModifier.Protected) != 0)))
{
if (seenNoAccessibilityDuplicates)
{
modTok = this.AddError(modTok, ErrorCode.ERR_BadMemberProtection);
}

seenNoAccessibilityDuplicates = false;
}
}
}
return GetModifier(nextToken) != DeclarationModifiers.None && !SyntaxFacts.IsContextualKeyword(nextToken.ContextualKind);
}

private bool IsPartialType()
Expand Down Expand Up @@ -2297,7 +2232,7 @@ private MemberDeclarationSyntax ParseMemberDeclarationOrStatement(SyntaxKind par

// Check for misplaced modifiers. if we see any, then consider this member
// terminated and restart parsing.
if (GetModifier(this.CurrentToken) != SyntaxModifier.None &&
if (GetModifier(this.CurrentToken) != DeclarationModifiers.None &&
this.CurrentToken.ContextualKind != SyntaxKind.PartialKeyword &&
this.CurrentToken.ContextualKind != SyntaxKind.AsyncKeyword &&
IsComplete(type))
Expand Down Expand Up @@ -6918,8 +6853,8 @@ private bool IsPossibleNewExpression()
return false;
}

SyntaxModifier modifier = GetModifier(nextToken);
if (modifier == SyntaxModifier.Partial)
DeclarationModifiers modifier = GetModifier(nextToken);
if (modifier == DeclarationModifiers.Partial)
{
if (SyntaxFacts.IsPredefinedType(PeekToken(2).Kind))
{
Expand All @@ -6933,7 +6868,7 @@ private bool IsPossibleNewExpression()
return false;
}
}
else if (modifier != SyntaxModifier.None)
else if (modifier != DeclarationModifiers.None)
{
return false;
}
Expand Down
Loading

0 comments on commit 001294b

Please sign in to comment.