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

Don't report modifier errors while parsing. #16339

Merged
merged 24 commits into from
Jul 17, 2017
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d54126e
Don't report modifier errors while parsing.
CyrusNajmabadi May 17, 2017
8d41a5a
Merge remote-tracking branch 'upstream/dev15.6' into parserDiagnostics29
CyrusNajmabadi May 17, 2017
d55ef9e
Merge remote-tracking branch 'upstream/dev15.6' into parserDiagnostics29
CyrusNajmabadi May 18, 2017
e9f056e
Fix bad merge.
CyrusNajmabadi May 18, 2017
2bb447b
restore code.
CyrusNajmabadi May 18, 2017
ae52d22
Merge remote-tracking branch 'upstream/dev15.6' into parserDiagnostics29
CyrusNajmabadi May 18, 2017
8e68d5a
Fix test.
CyrusNajmabadi May 18, 2017
0a2bd33
Merge remote-tracking branch 'upstream/master' into parserDiagnostics29
CyrusNajmabadi Jun 30, 2017
3dd2734
Merge remote-tracking branch 'upstream/master' into parserDiagnostics29
CyrusNajmabadi Jun 30, 2017
7e4a18c
Merge remote-tracking branch 'upstream/master' into parserDiagnostics29
CyrusNajmabadi Jul 17, 2017
5163f08
Remove old resource
CyrusNajmabadi Jul 17, 2017
0c5b027
Alternative way to do partial checking.
CyrusNajmabadi Jul 17, 2017
0a454fe
Alternative way to do partial checking.
CyrusNajmabadi Jul 17, 2017
4706448
Simplify formatting.
CyrusNajmabadi Jul 17, 2017
6b4dd85
Restore formatting.
CyrusNajmabadi Jul 17, 2017
8cf1f5f
Restore formatting.
CyrusNajmabadi Jul 17, 2017
e9d65d3
Restore formatting.
CyrusNajmabadi Jul 17, 2017
bad3584
Restore formatting.
CyrusNajmabadi Jul 17, 2017
6d2d910
Restore formatting.
CyrusNajmabadi Jul 17, 2017
230815c
Give a proper message.
CyrusNajmabadi Jul 17, 2017
cbb41d1
Update error positions.
CyrusNajmabadi Jul 17, 2017
81f70a1
Update error positions.
CyrusNajmabadi Jul 17, 2017
6b469bb
Update error positions.
CyrusNajmabadi Jul 17, 2017
c287f83
Actually report error on bad modifier.
CyrusNajmabadi Jul 17, 2017
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
11 changes: 1 addition & 10 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

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
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
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,13 @@ private SingleNamespaceOrTypeDeclaration VisitTypeDeclaration(TypeDeclarationSyn
var memberNames = GetNonTypeMemberNames(((Syntax.InternalSyntax.TypeDeclarationSyntax)(node.Green)).Members,
ref declFlags);

var modifiers = node.Modifiers.ToDeclarationModifiers(
allowPartial: true, 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 +388,12 @@ public override SingleNamespaceOrTypeDeclaration VisitDelegateDeclaration(Delega

declFlags |= SingleTypeDeclaration.TypeDeclarationFlags.HasAnyNontypeMembers;

var modifiers = node.Modifiers.ToDeclarationModifiers(allowPartial: false, 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 +418,20 @@ public override SingleNamespaceOrTypeDeclaration VisitEnumDeclaration(EnumDeclar

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

var diagnostics = DiagnosticBag.GetInstance();
var modifiers = node.Modifiers.ToDeclarationModifiers(allowPartial: false, 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
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
Copy link
Member

@jcouv jcouv Jul 17, 2017

Choose a reason for hiding this comment

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

ERR_PartialMethodOnlyMethods [](start = 11, length = 28)

Did you remove the corresponding resource string? #Resolved

Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Jul 17, 2017

Choose a reason for hiding this comment

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

#fixed #Resolved

ERR_PartialMethodNotExplicit = 754,
ERR_PartialMethodExtensionDifference = 755,
ERR_PartialMethodOnlyOneLatent = 756,
Expand Down
195 changes: 65 additions & 130 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
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 @@ -2313,7 +2248,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 @@ -6933,8 +6868,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 @@ -6948,7 +6883,7 @@ private bool IsPossibleNewExpression()
return false;
}
}
else if (modifier != SyntaxModifier.None)
else if (modifier != DeclarationModifiers.None)
{
return false;
}
Expand Down
Loading