Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,11 @@ private BoundStatement BindUsingDeclarationStatementParts(LocalDeclarationStatem

private BoundStatement BindDeclarationStatementParts(LocalDeclarationStatementSyntax node, BindingDiagnosticBag diagnostics)
{
// Check for duplicate modifiers in local declarations.
// The actual modifier (const) is determined by node.IsConst below.
if (diagnostics.DiagnosticBag is not null)
ModifierUtils.CheckForDuplicateModifiers(node.Modifiers, diagnostics.DiagnosticBag);

var typeSyntax = node.Declaration.Type;
bool isConst = node.IsConst;

Expand Down
12 changes: 5 additions & 7 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10247,7 +10247,7 @@ private StatementSyntax ParseLocalDeclarationStatement(SyntaxList<AttributeListS
}

var mods = _pool.Allocate();
this.ParseDeclarationModifiers(mods, isUsingDeclaration: usingKeyword is not null);
this.ParseLocalDeclarationStatementModifiers(mods, isUsingDeclaration: usingKeyword is not null);

var variables = _pool.AllocateSeparated<VariableDeclaratorSyntax>();
try
Expand Down Expand Up @@ -10541,7 +10541,7 @@ private bool IsEndOfDeclarationClause()
}
}

private void ParseDeclarationModifiers(SyntaxListBuilder list, bool isUsingDeclaration)
private void ParseLocalDeclarationStatementModifiers(SyntaxListBuilder list, bool isUsingDeclaration)
{
SyntaxKind k;
while (IsDeclarationModifier(k = this.CurrentToken.ContextualKind) || IsAdditionalLocalFunctionModifier(k))
Expand Down Expand Up @@ -10570,11 +10570,9 @@ private void ParseDeclarationModifiers(SyntaxListBuilder list, bool isUsingDecla
{
mod = this.AddError(mod, ErrorCode.ERR_BadMemberFlag, mod.Text);
}
else if (list.Any(mod.RawKind))
{
// check for duplicates, can only be const
mod = this.AddError(mod, ErrorCode.ERR_TypeExpected);
}

// Note: Duplicate modifiers are not reported here during parsing.
// They will be reported during binding (see Binder_Statements.BindDeclarationStatementParts).

list.Add(mod);
}
Expand Down
70 changes: 44 additions & 26 deletions src/Compilers/CSharp/Portable/Symbols/Source/ModifierUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -408,41 +408,59 @@ private static DeclarationModifiers ToDeclarationModifier(SyntaxKind kind)
}
}

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

for (int i = 0; i < modifiers.Count; i++)
{
SyntaxToken modifier = modifiers[i];
DeclarationModifiers one = ToDeclarationModifier(modifier.ContextualKind());
private static DeclarationModifiers GetDeclarationModifiersAndCheckForDuplicateModifiers(
SyntaxTokenList modifiers,
DiagnosticBag diagnostics)
{
var allModifiers = DeclarationModifiers.None;

var seenNoDuplicates = true;
foreach (var modifierToken in modifiers)
{
var thisModifier = ToDeclarationModifier(modifierToken.ContextualKind());
ReportDuplicateModifiers(
modifier, one, result,
modifierToken,
thisModifier,
allModifiers,
ref seenNoDuplicates,
diagnostics);

if (one == DeclarationModifiers.Partial)
allModifiers |= thisModifier;
}

return allModifiers;
}

public static DeclarationModifiers ToDeclarationModifiers(
this SyntaxTokenList modifiers, bool isForTypeDeclaration, DiagnosticBag diagnostics, bool isOrdinaryMethod = false)
{
var result = GetDeclarationModifiersAndCheckForDuplicateModifiers(modifiers, diagnostics);
if ((result & DeclarationModifiers.Partial) == DeclarationModifiers.Partial)
{
var i = modifiers.IndexOf(SyntaxKind.PartialKeyword);
var modifier = modifiers[i];

var messageId = isForTypeDeclaration ? MessageID.IDS_FeaturePartialTypes : MessageID.IDS_FeaturePartialMethod;
messageId.CheckFeatureAvailability(diagnostics, modifier);

// `partial` must always be the last modifier according to the language. However, there was a bug
// where we allowed `partial async` at the end of modifiers on methods. We keep this behavior for
// backcompat.
var isLast = i == modifiers.Count - 1;
var isPartialAsyncMethod = isOrdinaryMethod && i == modifiers.Count - 2 && modifiers[i + 1].ContextualKind() is SyntaxKind.AsyncKeyword;
if (!isLast && !isPartialAsyncMethod)
{
var messageId = isForTypeDeclaration ? MessageID.IDS_FeaturePartialTypes : MessageID.IDS_FeaturePartialMethod;
messageId.CheckFeatureAvailability(diagnostics, modifier);

// `partial` must always be the last modifier according to the language. However, there was a bug
// where we allowed `partial async` at the end of modifiers on methods. We keep this behavior for
// backcompat.
var isLast = i == modifiers.Count - 1;
var isPartialAsyncMethod = isOrdinaryMethod && i == modifiers.Count - 2 && modifiers[i + 1].ContextualKind() is SyntaxKind.AsyncKeyword;
if (!isLast && !isPartialAsyncMethod)
{
diagnostics.Add(
ErrorCode.ERR_PartialMisplaced,
modifier.GetLocation());
}
diagnostics.Add(
ErrorCode.ERR_PartialMisplaced,
modifier.GetLocation());
}

result |= one;
}

switch (result & DeclarationModifiers.AccessibilityMask)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,15 +425,6 @@ public static void Main()
}
";
ParserErrorMessageTests.ParseAndValidate(test,
// (7,15): error CS1031: Type expected
// const const double d = 0;
Diagnostic(ErrorCode.ERR_TypeExpected, "const").WithLocation(7, 15),
// (8,15): error CS1031: Type expected
// const const const long l = 0;
Diagnostic(ErrorCode.ERR_TypeExpected, "const").WithLocation(8, 15),
// (8,21): error CS1031: Type expected
// const const const long l = 0;
Diagnostic(ErrorCode.ERR_TypeExpected, "const").WithLocation(8, 21),
// (9,15): error CS0106: The modifier 'readonly' is not valid for this item
// const readonly readonly readonly const double r = 0;
Diagnostic(ErrorCode.ERR_BadMemberFlag, "readonly").WithArguments("readonly").WithLocation(9, 15),
Expand All @@ -442,13 +433,86 @@ public static void Main()
Diagnostic(ErrorCode.ERR_BadMemberFlag, "readonly").WithArguments("readonly").WithLocation(9, 24),
// (9,33): error CS0106: The modifier 'readonly' is not valid for this item
// const readonly readonly readonly const double r = 0;
Diagnostic(ErrorCode.ERR_BadMemberFlag, "readonly").WithArguments("readonly").WithLocation(9, 33),
// (9,42): error CS1031: Type expected
// const readonly readonly readonly const double r = 0;
Diagnostic(ErrorCode.ERR_TypeExpected, "const").WithLocation(9, 42)
Diagnostic(ErrorCode.ERR_BadMemberFlag, "readonly").WithArguments("readonly").WithLocation(9, 33)
);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/32106")]
public void DuplicateModifiers_NotReportedDuringParsing()
{
// Duplicate modifiers for local declarations and local functions should not produce
// parsing errors. They will be reported during binding.
var localDeclaration = """
class C
{
void M()
{
const const int a = 0;
}
}
""";
ParserErrorMessageTests.ParseAndValidate(localDeclaration);

var localFunction = """
class C
{
void M()
{
static static void F() {}
}
}
""";
ParserErrorMessageTests.ParseAndValidate(localFunction);

var typeDeclaration = """public public class C {}""";
ParserErrorMessageTests.ParseAndValidate(typeDeclaration);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/32106")]
public void DuplicateModifiers_StillFailDuringBinding()
{
// Verify that duplicate modifiers still produce errors during binding/compilation
var localDeclaration = """
class C
{
void M()
{
const const int a = 0;
}
}
""";
CreateCompilation(localDeclaration).VerifyDiagnostics(
// (5,15): error CS1004: Duplicate 'const' modifier
// const const int a = 0;
Diagnostic(ErrorCode.ERR_DuplicateModifier, "const").WithArguments("const").WithLocation(5, 15),
// (5,25): warning CS0219: The variable 'a' is assigned but its value is never used
// const const int a = 0;
Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "a").WithArguments("a").WithLocation(5, 25));

var localFunction = """
class C
{
void M()
{
static static void F() {}
}
}
""";
CreateCompilation(localFunction).VerifyDiagnostics(
// (5,16): error CS1004: Duplicate 'static' modifier
// static static void F() {}
Diagnostic(ErrorCode.ERR_DuplicateModifier, "static").WithArguments("static").WithLocation(5, 16),
// (5,28): warning CS8321: The local function 'F' is declared but never used
// static static void F() {}
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "F").WithArguments("F").WithLocation(5, 28));

var typeDeclaration = """public public class C {}""";
CreateCompilation(typeDeclaration).VerifyDiagnostics(
// (1,8): error CS1004: Duplicate 'public' modifier
// public public class C {}
Diagnostic(ErrorCode.ERR_DuplicateModifier, "public").WithArguments("public").WithLocation(1, 8));
}

[WorkItem(553293, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/553293")]
[Fact]
public void CS1021ERR_IntOverflow()
Expand Down
Loading