From 8bfceb297b7ee4c05cd844d7f82bd29e0af631ec Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 24 Jan 2019 14:43:15 -0800 Subject: [PATCH 01/19] Add syntax tests for readonly members --- .../Syntax/Parsing/DeclarationParsingTests.cs | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs index d29c2237fdd93..ea39a3a4aa269 100644 --- a/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs @@ -2729,6 +2729,134 @@ public void TestClassMethodWithPartial() Assert.Equal(SyntaxKind.None, ms.SemicolonToken.Kind()); } + [Fact] + public void TestStructMethodWithReadonly() + { + var text = "struct a { readonly void M() { } }"; + var file = this.ParseFile(text); + + Assert.NotNull(file); + Assert.Equal(1, file.Members.Count); + Assert.Equal(text, file.ToString()); + Assert.Equal(0, file.Errors().Length); + + Assert.Equal(SyntaxKind.StructDeclaration, file.Members[0].Kind()); + var structDecl = (TypeDeclarationSyntax)file.Members[0]; + Assert.Equal(0, structDecl.AttributeLists.Count); + Assert.Equal(0, structDecl.Modifiers.Count); + Assert.NotNull(structDecl.Keyword); + Assert.Equal(SyntaxKind.StructKeyword, structDecl.Keyword.Kind()); + Assert.NotNull(structDecl.Identifier); + Assert.Equal("a", structDecl.Identifier.ToString()); + Assert.Null(structDecl.BaseList); + Assert.Equal(0, structDecl.ConstraintClauses.Count); + Assert.NotNull(structDecl.OpenBraceToken); + Assert.NotNull(structDecl.CloseBraceToken); + + Assert.Equal(1, structDecl.Members.Count); + + Assert.Equal(SyntaxKind.MethodDeclaration, structDecl.Members[0].Kind()); + var ms = (MethodDeclarationSyntax)structDecl.Members[0]; + Assert.Equal(0, ms.AttributeLists.Count); + Assert.Equal(1, ms.Modifiers.Count); + Assert.Equal(SyntaxKind.ReadOnlyKeyword, ms.Modifiers[0].Kind()); + Assert.NotNull(ms.ReturnType); + Assert.Equal("void", ms.ReturnType.ToString()); + Assert.NotNull(ms.Identifier); + Assert.Equal("M", ms.Identifier.ToString()); + Assert.NotNull(ms.ParameterList.OpenParenToken); + Assert.False(ms.ParameterList.OpenParenToken.IsMissing); + Assert.Equal(0, ms.ParameterList.Parameters.Count); + Assert.NotNull(ms.ParameterList.CloseParenToken); + Assert.False(ms.ParameterList.CloseParenToken.IsMissing); + Assert.Equal(0, ms.ConstraintClauses.Count); + Assert.NotNull(ms.Body); + Assert.NotEqual(SyntaxKind.None, ms.Body.OpenBraceToken.Kind()); + Assert.NotEqual(SyntaxKind.None, ms.Body.CloseBraceToken.Kind()); + Assert.Equal(SyntaxKind.None, ms.SemicolonToken.Kind()); + } + + [Fact] + public void TestStructExpressionPropertyWithReadonly() + { + var text = "struct a { readonly int M => 42; }"; + var file = this.ParseFile(text); + + Assert.NotNull(file); + Assert.Equal(1, file.Members.Count); + Assert.Equal(text, file.ToString()); + Assert.Equal(0, file.Errors().Length); + + Assert.Equal(SyntaxKind.StructDeclaration, file.Members[0].Kind()); + var structDecl = (TypeDeclarationSyntax)file.Members[0]; + Assert.Equal(0, structDecl.AttributeLists.Count); + Assert.Equal(0, structDecl.Modifiers.Count); + Assert.NotNull(structDecl.Keyword); + Assert.Equal(SyntaxKind.StructKeyword, structDecl.Keyword.Kind()); + Assert.NotNull(structDecl.Identifier); + Assert.Equal("a", structDecl.Identifier.ToString()); + Assert.Null(structDecl.BaseList); + Assert.Equal(0, structDecl.ConstraintClauses.Count); + Assert.NotNull(structDecl.OpenBraceToken); + Assert.NotNull(structDecl.CloseBraceToken); + + Assert.Equal(1, structDecl.Members.Count); + + Assert.Equal(SyntaxKind.PropertyDeclaration, structDecl.Members[0].Kind()); + var propertySyntax = (PropertyDeclarationSyntax)structDecl.Members[0]; + Assert.Equal(0, propertySyntax.AttributeLists.Count); + Assert.Equal(1, propertySyntax.Modifiers.Count); + Assert.Equal(SyntaxKind.ReadOnlyKeyword, propertySyntax.Modifiers[0].Kind()); + Assert.NotNull(propertySyntax.Type); + Assert.Equal("int", propertySyntax.Type.ToString()); + Assert.NotNull(propertySyntax.Identifier); + Assert.Equal("M", propertySyntax.Identifier.ToString()); + Assert.NotNull(propertySyntax.ExpressionBody); + Assert.NotEqual(SyntaxKind.None, propertySyntax.ExpressionBody.ArrowToken.Kind()); + Assert.NotNull(propertySyntax.ExpressionBody.Expression); + Assert.Equal(SyntaxKind.SemicolonToken, propertySyntax.SemicolonToken.Kind()); + } + + [Fact] + public void TestStructGetterPropertyWithReadonly() + { + var text = "struct a { int P { readonly get { return 42; } } }"; + var file = this.ParseFile(text); + + Assert.NotNull(file); + Assert.Equal(1, file.Members.Count); + Assert.Equal(text, file.ToString()); + Assert.Equal(0, file.Errors().Length); + + Assert.Equal(SyntaxKind.StructDeclaration, file.Members[0].Kind()); + var structDecl = (TypeDeclarationSyntax)file.Members[0]; + Assert.Equal(0, structDecl.AttributeLists.Count); + Assert.Equal(0, structDecl.Modifiers.Count); + Assert.NotNull(structDecl.Keyword); + Assert.Equal(SyntaxKind.StructKeyword, structDecl.Keyword.Kind()); + Assert.NotNull(structDecl.Identifier); + Assert.Equal("a", structDecl.Identifier.ToString()); + Assert.Null(structDecl.BaseList); + Assert.Equal(0, structDecl.ConstraintClauses.Count); + Assert.NotNull(structDecl.OpenBraceToken); + Assert.NotNull(structDecl.CloseBraceToken); + + Assert.Equal(1, structDecl.Members.Count); + + Assert.Equal(SyntaxKind.PropertyDeclaration, structDecl.Members[0].Kind()); + var propertySyntax = (PropertyDeclarationSyntax)structDecl.Members[0]; + Assert.Equal(0, propertySyntax.AttributeLists.Count); + Assert.Equal(0, propertySyntax.Modifiers.Count); + Assert.NotNull(propertySyntax.Type); + Assert.Equal("int", propertySyntax.Type.ToString()); + Assert.NotNull(propertySyntax.Identifier); + Assert.Equal("P", propertySyntax.Identifier.ToString()); + var accessors = propertySyntax.AccessorList.Accessors; + Assert.Equal(1, accessors.Count); + Assert.Equal(1, accessors[0].Modifiers.Count); + Assert.Equal(SyntaxKind.ReadOnlyKeyword, accessors[0].Modifiers[0].Kind()); + } + [Fact] public void TestClassMethodWithParameter() { From 3310634c2d792b8ab8767166055cab52ddf6a08d Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 24 Jan 2019 15:35:03 -0800 Subject: [PATCH 02/19] Add simple test for readonly struct method --- .../Semantic/Semantics/ReadOnlyStructsTests.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 301eb2f364551..3e9581c5d9f6f 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -249,5 +249,22 @@ public static void Main() Diagnostic(ErrorCode.ERR_AssgReadonlyStatic2, "s.field").WithArguments("Program.s").WithLocation(8, 9) ); } + + [Fact] + public void ReadOnlyStructMethod() + { + var csharp = @" +public struct S +{ + public int i; + public readonly int M() + { + return i; + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } } } From 3eabddad4795dc23925ecffddfa46a2a9d306161 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 24 Jan 2019 15:35:23 -0800 Subject: [PATCH 03/19] Allow readonly modifier on methods in structs --- .../Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs index 8be06acf4ac08..098decb928098 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs @@ -772,6 +772,11 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, MethodKind allowedModifiers |= DeclarationModifiers.Extern | DeclarationModifiers.Async; } + if (ContainingType.IsStructType()) + { + allowedModifiers |= DeclarationModifiers.ReadOnly; + } + var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(modifiers, defaultAccess, allowedModifiers, location, diagnostics, out modifierErrors); this.CheckUnsafeModifier(mods, diagnostics); From 245aaa82f0636f25ba9ae505494abde072932606 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 28 Jan 2019 13:13:40 -0800 Subject: [PATCH 04/19] Add simple readonly class method test --- .../Semantics/ReadOnlyStructsTests.cs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 3e9581c5d9f6f..21ba140eee027 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -265,6 +265,27 @@ public readonly int M() "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics(); + comp.GetMember("S").GetMember("M"); // TODO: MethodSymbol.IsReadOnly + } + + [Fact] + public void ReadOnlyClassMethod() + { + var csharp = @" +public class C +{ + public int i; + public readonly int M() + { + return i; + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (5,25): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly int M() + Diagnostic(ErrorCode.ERR_BadMemberFlag, "M").WithArguments("readonly").WithLocation(5, 25)); } } } From 92f16a11033e2351d9863359f37f493fd8f298f3 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 28 Jan 2019 13:32:37 -0800 Subject: [PATCH 05/19] Add SourceMemberMethodSymbol.IsReadOnly --- .../Portable/Symbols/Source/SourceMemberMethodSymbol.cs | 8 ++++++++ .../Test/Semantic/Semantics/ReadOnlyStructsTests.cs | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs index a110cbc67f8ef..07f84a46584dc 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs @@ -509,6 +509,14 @@ public sealed override bool IsAsync } } + internal bool IsReadOnly + { + get + { + return (this.DeclarationModifiers & DeclarationModifiers.ReadOnly) != 0; + } + } + internal sealed override Cci.CallingConvention CallingConvention { get diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 21ba140eee027..4b3944b6cdb79 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -265,7 +265,10 @@ public readonly int M() "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics(); - comp.GetMember("S").GetMember("M"); // TODO: MethodSymbol.IsReadOnly + + var method = (SourceMemberMethodSymbol)comp.GetMember("S").GetMember("M"); + // PROTOTYPE: add `public abstract bool IsReadOnly` to MethodSymbol and implement in subtypes? + Assert.True(method.IsReadOnly); } [Fact] From efb9e972b355f05df2bc619046e07af811ceec0a Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 13 Feb 2019 14:40:29 -0800 Subject: [PATCH 06/19] Test readonly static method --- .../Source/SourceOrdinaryMethodSymbol.cs | 5 +++++ .../Portable/Symbols/TypeParameterSymbol.cs | 2 +- .../Semantics/ReadOnlyStructsTests.cs | 20 +++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs index 098decb928098..99ff44e0f8789 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs @@ -938,6 +938,11 @@ private void CheckModifiers(bool hasBody, Location location, DiagnosticBag diagn // The modifier '{0}' is not valid for this item diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.VirtualKeyword)); } + else if (IsStatic && IsReadOnly) + { + // The modifier '{0}' is not valid for this item + diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword)); + } else if (IsAbstract && !ContainingType.IsAbstract && (ContainingType.TypeKind == TypeKind.Class || ContainingType.TypeKind == TypeKind.Submission)) { // '{0}' is abstract but it is contained in non-abstract class '{1}' diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/TypeParameterSymbol.cs index ac97c9072e25e..fc1aaa064529b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/TypeParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/TypeParameterSymbol.cs @@ -584,7 +584,7 @@ internal bool GetIsValueType(ConsList inProgress) } public sealed override bool IsValueType => GetIsValueType(ConsList.Empty); - + internal sealed override ManagedKind ManagedKind { get diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 4b3944b6cdb79..b9b9e705b8c87 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -290,5 +290,25 @@ public readonly int M() // public readonly int M() Diagnostic(ErrorCode.ERR_BadMemberFlag, "M").WithArguments("readonly").WithLocation(5, 25)); } + + [Fact] + public void ReadOnlyStructStaticMethod() + { + var csharp = @" +public struct S +{ + public static int i; + public static readonly int M() + { + return i; + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (5,32): error CS0106: The modifier 'readonly' is not valid for this item + // public static readonly int M() + Diagnostic(ErrorCode.ERR_BadMemberFlag, "M").WithArguments("readonly").WithLocation(5, 32)); + } } } From 6329c7054020ed1dd02cd9453ec5af86ca5aee1d Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 19 Feb 2019 15:44:31 -0800 Subject: [PATCH 07/19] Disallow readonly accessors on static properties --- .../Source/SourcePropertyAccessorSymbol.cs | 12 ++- .../Semantics/ReadOnlyStructsTests.cs | 75 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs index ff4f0ea15b1ab..124d88d368596 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs @@ -410,7 +410,12 @@ private DeclarationModifiers MakeModifiers(AccessorDeclarationSyntax syntax, Loc const DeclarationModifiers defaultAccess = DeclarationModifiers.None; // Check that the set of modifiers is allowed - const DeclarationModifiers allowedModifiers = DeclarationModifiers.AccessibilityMask; + DeclarationModifiers allowedModifiers = DeclarationModifiers.AccessibilityMask; + if (this.ContainingType.IsStructType()) + { + allowedModifiers |= DeclarationModifiers.ReadOnly; + } + var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(syntax.Modifiers, defaultAccess, allowedModifiers, location, diagnostics, out modifierErrors); // For interface, check there are no accessibility modifiers. @@ -451,6 +456,11 @@ private void CheckModifiers(Location location, bool hasBody, bool isAutoProperty { diagnostics.Add(AccessCheck.GetProtectedMemberInSealedTypeError(ContainingType), location, this); } + else if (IsStatic && IsReadOnly) + { + // The modifier '{0}' is not valid for this item + diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword)); + } } /// diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index b9b9e705b8c87..8ff8c9d7652ff 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -310,5 +310,80 @@ public static readonly int M() // public static readonly int M() Diagnostic(ErrorCode.ERR_BadMemberFlag, "M").WithArguments("readonly").WithLocation(5, 32)); } + + [Fact] + public void ReadOnlyStructProperty() + { + var csharp = @" +public struct S +{ + public int i; + public int P + { + readonly get + { + return i; + } + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } + + [Fact] + public void ReadOnlyStructStaticProperty() + { + var csharp = @" +public struct S +{ + public static int i; + public static int P + { + readonly get + { + return i; + } + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (7,18): error CS0106: The modifier 'readonly' is not valid for this item + // readonly get + Diagnostic(ErrorCode.ERR_BadMemberFlag, "get").WithArguments("readonly").WithLocation(7, 18)); + } + + [Fact] + public void ReadOnlyStructStaticExpressionProperty() + { + var csharp = @" +public struct S +{ + public static int i; + public static readonly int P => i; +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (5,32): error CS0106: The modifier 'readonly' is not valid for this item + // public static readonly int P => i; + Diagnostic(ErrorCode.ERR_BadMemberFlag, "P").WithArguments("readonly").WithLocation(5, 32)); + } + + [Fact] + public void ReadOnlyAutoProperty() + { + var csharp = @" +public struct S +{ + public int P1 { readonly get; } + public int P2 { readonly get; set; } + public int P3 { readonly get; readonly set; } // PROTOTYPE: readonly set on an auto-property should give an error +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } } } From e0fd8a26e52a4ceb1708771d7ce582d9568f9eab Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 19 Feb 2019 17:33:41 -0800 Subject: [PATCH 08/19] Check usages of readonly keyword in property (not accessor) declarations --- .../Symbols/Source/SourcePropertySymbol.cs | 16 +++++ .../Semantics/ReadOnlyStructsTests.cs | 58 ++++++++++++++++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index 7b4c1837f07cb..f2169f1457c99 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -114,6 +114,12 @@ private SourcePropertySymbol( ? propertySyntax.ExpressionBody : ((IndexerDeclarationSyntax)syntax).ExpressionBody; bool hasExpressionBody = arrowExpression != null; + + if (!hasExpressionBody && HasReadOnlyModifier) + { + diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword)); + } + bool hasInitializer = !isIndexer && propertySyntax.Initializer != null; bool notRegularProperty = (!IsAbstract && !IsExtern && !isIndexer && hasAccessorList); @@ -627,6 +633,11 @@ internal bool IsNew get { return (_modifiers & DeclarationModifiers.New) != 0; } } + private bool HasReadOnlyModifier + { + get { return (_modifiers & DeclarationModifiers.ReadOnly) != 0; } + } + public override MethodSymbol GetMethod { get { return _getMethod; } @@ -813,6 +824,11 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool isExp } } + if (ContainingType.IsStructType()) + { + allowedModifiers |= DeclarationModifiers.ReadOnly; + } + if (!isInterface) { allowedModifiers |= diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 8ff8c9d7652ff..c31b1856cac8c 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -331,6 +331,29 @@ readonly get comp.VerifyDiagnostics(); } + [Fact] + public void ReadOnlyClassProperty() + { + var csharp = @" +public class C +{ + public int i; + public int P + { + readonly get + { + return i; + } + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (7,18): error CS0106: The modifier 'readonly' is not valid for this item + // readonly get + Diagnostic(ErrorCode.ERR_BadMemberFlag, "get").WithArguments("readonly").WithLocation(7, 18)); + } + [Fact] public void ReadOnlyStructStaticProperty() { @@ -366,9 +389,40 @@ public struct S "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics( - // (5,32): error CS0106: The modifier 'readonly' is not valid for this item + // (5,37): error CS0106: The modifier 'readonly' is not valid for this item // public static readonly int P => i; - Diagnostic(ErrorCode.ERR_BadMemberFlag, "P").WithArguments("readonly").WithLocation(5, 32)); + Diagnostic(ErrorCode.ERR_BadMemberFlag, "i").WithArguments("readonly").WithLocation(5, 37)); + } + + [Fact] + public void ReadOnlyStructExpressionProperty() + { + var csharp = @" +public struct S +{ + public int i; + public readonly int P => i; +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } + + [Fact] + public void ReadOnlyStructBlockProperty() + { + var csharp = @" +public struct S +{ + public int i; + public readonly int P { get { return i; } } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (5,25): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly int P { get { return i; } } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "P").WithArguments("readonly").WithLocation(5, 25)); } [Fact] From 4997b245e80599b692c2281fc4bdac77ec3d77f2 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 22 Feb 2019 15:16:13 -0800 Subject: [PATCH 09/19] Add ref-returning readonly method test --- .../Semantics/ReadOnlyStructsTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index c31b1856cac8c..df424c947c140 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -397,6 +397,7 @@ public struct S [Fact] public void ReadOnlyStructExpressionProperty() { + // TODO(rigibson): this syntax needs to change to `public int P readonly => i;` var csharp = @" public struct S { @@ -435,6 +436,32 @@ public struct S public int P2 { readonly get; set; } public int P3 { readonly get; readonly set; } // PROTOTYPE: readonly set on an auto-property should give an error } +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } + + [Fact] + public void RefReturningReadOnlyMethod() + { + // PROTOTYPE: would be good to add some more mutation here + // as well as expected diagnostics once that part of the feature is ready. + var csharp = @" +public struct S +{ + private static int f1; + public readonly ref int M1() => ref f1; + + private static readonly int f2; + public readonly ref readonly int M2() => ref f1; + + private static readonly int f3; + public ref readonly int M3() + { + f1++; + return ref f3; + } +} "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics(); From c35df0ee3ad9f11b0cc96d9b5469fb6e7a63a0ec Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Sun, 24 Feb 2019 12:03:41 -0800 Subject: [PATCH 10/19] Add readonly ref readonly parsing test --- .../Syntax/Parsing/DeclarationParsingTests.cs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs index ea39a3a4aa269..a575e293afd32 100644 --- a/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs @@ -2776,6 +2776,56 @@ public void TestStructMethodWithReadonly() Assert.Equal(SyntaxKind.None, ms.SemicolonToken.Kind()); } + [Fact] + public void TestReadOnlyRefReturning() + { + var text = "struct a { readonly ref readonly int M() { } }"; + var file = this.ParseFile(text); + + Assert.NotNull(file); + Assert.Equal(1, file.Members.Count); + Assert.Equal(text, file.ToString()); + Assert.Equal(0, file.Errors().Length); + + Assert.Equal(SyntaxKind.StructDeclaration, file.Members[0].Kind()); + var structDecl = (TypeDeclarationSyntax)file.Members[0]; + Assert.Equal(0, structDecl.AttributeLists.Count); + Assert.Equal(0, structDecl.Modifiers.Count); + Assert.NotNull(structDecl.Keyword); + Assert.Equal(SyntaxKind.StructKeyword, structDecl.Keyword.Kind()); + Assert.NotNull(structDecl.Identifier); + Assert.Equal("a", structDecl.Identifier.ToString()); + Assert.Null(structDecl.BaseList); + Assert.Equal(0, structDecl.ConstraintClauses.Count); + Assert.NotNull(structDecl.OpenBraceToken); + Assert.NotNull(structDecl.CloseBraceToken); + + Assert.Equal(1, structDecl.Members.Count); + + Assert.Equal(SyntaxKind.MethodDeclaration, structDecl.Members[0].Kind()); + var ms = (MethodDeclarationSyntax)structDecl.Members[0]; + Assert.Equal(0, ms.AttributeLists.Count); + Assert.Equal(1, ms.Modifiers.Count); + Assert.Equal(SyntaxKind.ReadOnlyKeyword, ms.Modifiers[0].Kind()); + Assert.Equal(SyntaxKind.RefType, ms.ReturnType.Kind()); + var rt = (RefTypeSyntax)ms.ReturnType; + Assert.Equal(SyntaxKind.RefKeyword, rt.RefKeyword.Kind()); + Assert.Equal(SyntaxKind.ReadOnlyKeyword, rt.ReadOnlyKeyword.Kind()); + Assert.Equal("int", rt.Type.ToString()); + Assert.NotNull(ms.Identifier); + Assert.Equal("M", ms.Identifier.ToString()); + Assert.NotNull(ms.ParameterList.OpenParenToken); + Assert.False(ms.ParameterList.OpenParenToken.IsMissing); + Assert.Equal(0, ms.ParameterList.Parameters.Count); + Assert.NotNull(ms.ParameterList.CloseParenToken); + Assert.False(ms.ParameterList.CloseParenToken.IsMissing); + Assert.Equal(0, ms.ConstraintClauses.Count); + Assert.NotNull(ms.Body); + Assert.NotEqual(SyntaxKind.None, ms.Body.OpenBraceToken.Kind()); + Assert.NotEqual(SyntaxKind.None, ms.Body.CloseBraceToken.Kind()); + Assert.Equal(SyntaxKind.None, ms.SemicolonToken.Kind()); + } + [Fact] public void TestStructExpressionPropertyWithReadonly() { From c3daaae95b15b25084083dddc3ec16850386a40f Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 26 Feb 2019 13:23:48 -0800 Subject: [PATCH 11/19] Allow readonly modifier on property decls in general. Add more tests. --- .../Symbols/Source/SourcePropertySymbol.cs | 10 --- .../Semantics/ReadOnlyStructsTests.cs | 62 ++++++++++++++++--- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index f2169f1457c99..033e0bfda6d28 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -115,11 +115,6 @@ private SourcePropertySymbol( : ((IndexerDeclarationSyntax)syntax).ExpressionBody; bool hasExpressionBody = arrowExpression != null; - if (!hasExpressionBody && HasReadOnlyModifier) - { - diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword)); - } - bool hasInitializer = !isIndexer && propertySyntax.Initializer != null; bool notRegularProperty = (!IsAbstract && !IsExtern && !isIndexer && hasAccessorList); @@ -633,11 +628,6 @@ internal bool IsNew get { return (_modifiers & DeclarationModifiers.New) != 0; } } - private bool HasReadOnlyModifier - { - get { return (_modifiers & DeclarationModifiers.ReadOnly) != 0; } - } - public override MethodSymbol GetMethod { get { return _getMethod; } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index df424c947c140..d5d6bc0b60dbb 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -420,10 +420,7 @@ public struct S } "; var comp = CreateCompilation(csharp); - comp.VerifyDiagnostics( - // (5,25): error CS0106: The modifier 'readonly' is not valid for this item - // public readonly int P { get { return i; } } - Diagnostic(ErrorCode.ERR_BadMemberFlag, "P").WithArguments("readonly").WithLocation(5, 25)); + comp.VerifyDiagnostics(); } [Fact] @@ -433,8 +430,10 @@ public void ReadOnlyAutoProperty() public struct S { public int P1 { readonly get; } - public int P2 { readonly get; set; } - public int P3 { readonly get; readonly set; } // PROTOTYPE: readonly set on an auto-property should give an error + public readonly int P2 { get; } + public int P3 { readonly get; set; } + public int P4 { readonly get; readonly set; } // PROTOTYPE: readonly set on an auto-property should give an error + public readonly int P5 { get; set; } // PROTOTYPE: readonly set on an auto-property should give an error } "; var comp = CreateCompilation(csharp); @@ -453,7 +452,7 @@ public struct S public readonly ref int M1() => ref f1; private static readonly int f2; - public readonly ref readonly int M2() => ref f1; + public readonly ref readonly int M2() => ref f2; private static readonly int f3; public ref readonly int M3() @@ -466,5 +465,54 @@ public ref readonly int M3() var comp = CreateCompilation(csharp); comp.VerifyDiagnostics(); } + + [Fact] + public void ReadOnlyConstructor() + { + var csharp = @" +public struct S +{ + public readonly S(int i) { } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (4,21): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly S(int i) { } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "S").WithArguments("readonly").WithLocation(4, 21)); + } + + [Fact] + public void ReadOnlyOperator() + { + var csharp = @" +public struct S +{ + public static readonly S operator +(S lhs, S rhs) => lhs; + public static readonly explicit operator int(S s) => 42; +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (4,39): error CS0106: The modifier 'readonly' is not valid for this item + // public static readonly S operator +(S lhs, S rhs) => lhs; + Diagnostic(ErrorCode.ERR_BadMemberFlag, "+").WithArguments("readonly").WithLocation(4, 39), + // (5,46): error CS0106: The modifier 'readonly' is not valid for this item + // public static readonly explicit operator int(S s) => 42; + Diagnostic(ErrorCode.ERR_BadMemberFlag, "int").WithArguments("readonly").WithLocation(5, 46)); + } + + [Fact] + public void ReadOnlyDelegate() + { + var csharp = @" +public readonly delegate int Del(); +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (2,30): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly delegate int Del(); + Diagnostic(ErrorCode.ERR_BadMemberFlag, "Del").WithArguments("readonly").WithLocation(2, 30)); + } } } From 00fab4c134e70d27a33b9ac0c5c289a044b0787f Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 26 Feb 2019 13:30:29 -0800 Subject: [PATCH 12/19] Add indexer tests --- .../Semantics/ReadOnlyStructsTests.cs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index d5d6bc0b60dbb..61fcde55d85db 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -514,5 +514,52 @@ public void ReadOnlyDelegate() // public readonly delegate int Del(); Diagnostic(ErrorCode.ERR_BadMemberFlag, "Del").WithArguments("readonly").WithLocation(2, 30)); } + + [Fact] + public void ReadOnlyIndexer() + { + var csharp = @" +public struct S1 +{ + public readonly int this[int i] + { + get => 42; + set {} + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } + + [Fact] + public void ReadOnlyExpressionIndexer() + { + var csharp = @" +public struct S1 +{ + public readonly int this[int i] => 42; +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } + + [Fact] + public void ReadOnlyGetExpressionIndexer() + { + var csharp = @" +public struct S1 +{ + public int this[int i] + { + readonly get => 42; + readonly set {} + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } } } From 4b322e7f148cc61845ec31579831e86321749acd Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 26 Feb 2019 16:12:55 -0800 Subject: [PATCH 13/19] Readonly events on structs --- .../Symbols/Source/SourceEventSymbol.cs | 15 ++++ .../Symbols/Source/SourcePropertySymbol.cs | 1 - .../Semantics/ReadOnlyStructsTests.cs | 85 +++++++++++++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs index ca678e2e5e62e..b2feefc7d96cf 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs @@ -368,6 +368,11 @@ public sealed override bool IsVirtual get { return (_modifiers & DeclarationModifiers.Virtual) != 0; } } + internal bool IsReadOnly + { + get { return (_modifiers & DeclarationModifiers.ReadOnly) != 0; } + } + public sealed override Accessibility DeclaredAccessibility { get { return ModifierUtils.EffectiveAccessibility(_modifiers); } @@ -435,6 +440,11 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool expli } } + if (this.ContainingType.IsStructType()) + { + allowedModifiers |= DeclarationModifiers.ReadOnly; + } + if (!isInterface) { allowedModifiers |= DeclarationModifiers.Extern; @@ -468,6 +478,11 @@ protected void CheckModifiersAndType(DiagnosticBag diagnostics) // A static member '{0}' cannot be marked as override, virtual, or abstract diagnostics.Add(ErrorCode.ERR_StaticNotVirtual, location, this); } + else if (IsStatic && IsReadOnly) + { + // The modifier '{0}' is not valid for this item + diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword)); + } else if (IsOverride && (IsNew || IsVirtual)) { // A member '{0}' marked as override cannot be marked as new or virtual diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index 033e0bfda6d28..9ec47071bac68 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -114,7 +114,6 @@ private SourcePropertySymbol( ? propertySyntax.ExpressionBody : ((IndexerDeclarationSyntax)syntax).ExpressionBody; bool hasExpressionBody = arrowExpression != null; - bool hasInitializer = !isIndexer && propertySyntax.Initializer != null; bool notRegularProperty = (!IsAbstract && !IsExtern && !isIndexer && hasAccessorList); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 61fcde55d85db..11193c621329a 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -561,5 +561,90 @@ readonly set {} var comp = CreateCompilation(csharp); comp.VerifyDiagnostics(); } + + [Fact] + public void ReadOnlyFieldLikeEvent() + { + var csharp = @" +using System; + +public struct S1 +{ + public readonly event Action OnFoo; + public void M() { OnFoo?.Invoke(new EventArgs()); } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (6,45): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly event Action OnFoo; + Diagnostic(ErrorCode.ERR_BadMemberFlag, "OnFoo").WithArguments("readonly").WithLocation(6, 45)); + } + + [Fact] + public void ReadOnlyEventExplicitAddRemove() + { + var csharp = @" +using System; + +public struct S1 +{ + public readonly event Action OnFoo + { + add {} + remove {} + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics(); + } + + [Fact] + public void ReadOnlyStaticEvent() + { + var csharp = @" +using System; + +public struct S1 +{ + public static readonly event Action OnFoo + { + add {} + remove {} + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (6,52): error CS0106: The modifier 'readonly' is not valid for this item + // public static readonly event Action OnFoo + Diagnostic(ErrorCode.ERR_BadMemberFlag, "OnFoo").WithArguments("readonly").WithLocation(6, 52)); + } + + [Fact] + public void ReadOnlyEventReadOnlyAccessors() + { + var csharp = @" +using System; + +public struct S1 +{ + public event Action OnFoo + { + readonly add {} + readonly remove {} + } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (8,9): error CS1609: Modifiers cannot be placed on event accessor declarations + // readonly add {} + Diagnostic(ErrorCode.ERR_NoModifiersOnAccessor, "readonly").WithLocation(8, 9), + // (9,9): error CS1609: Modifiers cannot be placed on event accessor declarations + // readonly remove {} + Diagnostic(ErrorCode.ERR_NoModifiersOnAccessor, "readonly").WithLocation(9, 9)); + } } } From ee3228a8a0f34cdae965dcfca84eeb7f33072ded Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 26 Feb 2019 16:17:26 -0800 Subject: [PATCH 14/19] Remove comment --- .../CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 11193c621329a..e6ddaf774b00b 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -397,7 +397,6 @@ public struct S [Fact] public void ReadOnlyStructExpressionProperty() { - // TODO(rigibson): this syntax needs to change to `public int P readonly => i;` var csharp = @" public struct S { From 7da2c022bad3d4d07a329b069abc565013c584e6 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Tue, 26 Feb 2019 16:20:53 -0800 Subject: [PATCH 15/19] Disallow readonly on events with associated fields --- .../CSharp/Portable/Symbols/Source/SourceEventSymbol.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs index b2feefc7d96cf..66554b26a3248 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs @@ -478,7 +478,7 @@ protected void CheckModifiersAndType(DiagnosticBag diagnostics) // A static member '{0}' cannot be marked as override, virtual, or abstract diagnostics.Add(ErrorCode.ERR_StaticNotVirtual, location, this); } - else if (IsStatic && IsReadOnly) + else if (IsReadOnly && (IsStatic || HasAssociatedField)) { // The modifier '{0}' is not valid for this item diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword)); From 3f737211b4d82fac117dbef84e0afd910ac280af Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 27 Feb 2019 12:49:49 -0800 Subject: [PATCH 16/19] Test static readonly auto properties and destructors --- .../Semantics/ReadOnlyStructsTests.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index e6ddaf774b00b..f2bca342dd968 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -439,6 +439,29 @@ public struct S comp.VerifyDiagnostics(); } + [Fact] + public void ReadOnlyStaticAutoProperty() + { + var csharp = @" +public struct S +{ + public static readonly int P1 { get; set; } + public static int P2 { readonly get; } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (4,37): error CS0106: The modifier 'readonly' is not valid for this item + // public static readonly int P1 { get; set; } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "get").WithArguments("readonly").WithLocation(4, 37), + // (4,42): error CS0106: The modifier 'readonly' is not valid for this item + // public static readonly int P1 { get; set; } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "set").WithArguments("readonly").WithLocation(4, 42), + // (5,37): error CS0106: The modifier 'readonly' is not valid for this item + // public static int P2 { readonly get; } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "get").WithArguments("readonly").WithLocation(5, 37)); + } + [Fact] public void RefReturningReadOnlyMethod() { @@ -481,6 +504,25 @@ public readonly S(int i) { } Diagnostic(ErrorCode.ERR_BadMemberFlag, "S").WithArguments("readonly").WithLocation(4, 21)); } + [Fact] + public void ReadOnlyDestructor() + { + var csharp = @" +public struct S +{ + readonly ~S() { } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (4,15): error CS0106: The modifier 'readonly' is not valid for this item + // readonly ~S() { } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "S").WithArguments("readonly").WithLocation(4, 15), + // (4,15): error CS0575: Only class types can contain destructors + // readonly ~S() { } + Diagnostic(ErrorCode.ERR_OnlyClassesCanContainDestructors, "S").WithArguments("S.~S()").WithLocation(4, 15)); + } + [Fact] public void ReadOnlyOperator() { From 893a8d48adb29d4fb379c2b4f492ab09be6127c7 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 27 Feb 2019 15:35:09 -0800 Subject: [PATCH 17/19] Disallow redundant readonly on accessor. Simplify diagnostics for static readonly property. --- .../Source/SourcePropertyAccessorSymbol.cs | 4 +-- .../Symbols/Source/SourcePropertySymbol.cs | 7 +++++ .../Semantics/ReadOnlyStructsTests.cs | 27 ++++++++++++++----- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs index 124d88d368596..18f65e6d2e385 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs @@ -411,7 +411,7 @@ private DeclarationModifiers MakeModifiers(AccessorDeclarationSyntax syntax, Loc // Check that the set of modifiers is allowed DeclarationModifiers allowedModifiers = DeclarationModifiers.AccessibilityMask; - if (this.ContainingType.IsStructType()) + if (this.ContainingType.IsStructType() && !_property.HasReadOnlyModifier) { allowedModifiers |= DeclarationModifiers.ReadOnly; } @@ -456,7 +456,7 @@ private void CheckModifiers(Location location, bool hasBody, bool isAutoProperty { diagnostics.Add(AccessCheck.GetProtectedMemberInSealedTypeError(ContainingType), location, this); } - else if (IsStatic && IsReadOnly) + else if (IsStatic && IsReadOnly && !_property.HasReadOnlyModifier) { // The modifier '{0}' is not valid for this item diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword)); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs index 9ec47071bac68..a1d6619d0315a 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs @@ -627,6 +627,8 @@ internal bool IsNew get { return (_modifiers & DeclarationModifiers.New) != 0; } } + internal bool HasReadOnlyModifier => (_modifiers & DeclarationModifiers.ReadOnly) != 0; + public override MethodSymbol GetMethod { get { return _getMethod; } @@ -895,6 +897,11 @@ private void CheckModifiers(Location location, bool isIndexer, DiagnosticBag dia // A static member '{0}' cannot be marked as override, virtual, or abstract diagnostics.Add(ErrorCode.ERR_StaticNotVirtual, location, this); } + else if (IsStatic && HasReadOnlyModifier) + { + // The modifier '{0}' is not valid for this item + diagnostics.Add(ErrorCode.ERR_BadMemberFlag, location, SyntaxFacts.GetText(SyntaxKind.ReadOnlyKeyword)); + } else if (IsOverride && (IsNew || IsVirtual)) { // A member '{0}' marked as override cannot be marked as new or virtual diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index f2bca342dd968..6847e112f6f79 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -389,9 +389,9 @@ public struct S "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics( - // (5,37): error CS0106: The modifier 'readonly' is not valid for this item + // (5,32): error CS0106: The modifier 'readonly' is not valid for this item // public static readonly int P => i; - Diagnostic(ErrorCode.ERR_BadMemberFlag, "i").WithArguments("readonly").WithLocation(5, 37)); + Diagnostic(ErrorCode.ERR_BadMemberFlag, "P").WithArguments("readonly").WithLocation(5, 32)); } [Fact] @@ -439,6 +439,22 @@ public struct S comp.VerifyDiagnostics(); } + [Fact] + public void ReadOnlyProperty_RedundantReadOnlyAccessor() + { + var csharp = @" +public struct S +{ + public readonly int P { readonly get; } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (4,38): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly int P { readonly get; } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "get").WithArguments("readonly").WithLocation(4, 38)); + } + [Fact] public void ReadOnlyStaticAutoProperty() { @@ -451,12 +467,9 @@ public struct S "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics( - // (4,37): error CS0106: The modifier 'readonly' is not valid for this item - // public static readonly int P1 { get; set; } - Diagnostic(ErrorCode.ERR_BadMemberFlag, "get").WithArguments("readonly").WithLocation(4, 37), - // (4,42): error CS0106: The modifier 'readonly' is not valid for this item + // (4,32): error CS0106: The modifier 'readonly' is not valid for this item // public static readonly int P1 { get; set; } - Diagnostic(ErrorCode.ERR_BadMemberFlag, "set").WithArguments("readonly").WithLocation(4, 42), + Diagnostic(ErrorCode.ERR_BadMemberFlag, "P1").WithArguments("readonly").WithLocation(4, 32), // (5,37): error CS0106: The modifier 'readonly' is not valid for this item // public static int P2 { readonly get; } Diagnostic(ErrorCode.ERR_BadMemberFlag, "get").WithArguments("readonly").WithLocation(5, 37)); From d1a1cca80e4c6e1d259a2b131c055ca8a7d8671a Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 28 Feb 2019 10:56:10 -0800 Subject: [PATCH 18/19] Add tests based on feedback --- .../Semantics/ReadOnlyStructsTests.cs | 135 ++++++++++++------ 1 file changed, 92 insertions(+), 43 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 6847e112f6f79..98482445a52c4 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -272,23 +272,91 @@ public readonly int M() } [Fact] - public void ReadOnlyClassMethod() + public void ReadOnlyClass() { var csharp = @" -public class C +using System; + +public readonly class C { - public int i; - public readonly int M() - { - return i; - } + public readonly int M() => 42; + public readonly int P { get; set; } + public readonly int this[int i] => i; + public readonly event Action E { add {} remove {} } +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (4,23): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly class C + Diagnostic(ErrorCode.ERR_BadMemberFlag, "C").WithArguments("readonly").WithLocation(4, 23), + // (6,25): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly int M() => 42; + Diagnostic(ErrorCode.ERR_BadMemberFlag, "M").WithArguments("readonly").WithLocation(6, 25), + // (7,25): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly int P { get; set; } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "P").WithArguments("readonly").WithLocation(7, 25), + // (8,25): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly int this[int i] => i; + Diagnostic(ErrorCode.ERR_BadMemberFlag, "this").WithArguments("readonly").WithLocation(8, 25), + // (9,45): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly event Action E { add {} remove {} } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "E").WithArguments("readonly").WithLocation(9, 45)); + } + + [Fact] + public void ReadOnlyInterface() + { + var csharp = @" +using System; + +public readonly interface I +{ + readonly int M(); + readonly int P { get; set; } + readonly int this[int i] { get; } + readonly event Action E; } "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics( - // (5,25): error CS0106: The modifier 'readonly' is not valid for this item - // public readonly int M() - Diagnostic(ErrorCode.ERR_BadMemberFlag, "M").WithArguments("readonly").WithLocation(5, 25)); + // (4,27): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly interface I + Diagnostic(ErrorCode.ERR_BadMemberFlag, "I").WithArguments("readonly").WithLocation(4, 27), + // (6,18): error CS0106: The modifier 'readonly' is not valid for this item + // readonly int M(); + Diagnostic(ErrorCode.ERR_BadMemberFlag, "M").WithArguments("readonly").WithLocation(6, 18), + // (7,18): error CS0106: The modifier 'readonly' is not valid for this item + // readonly int P { get; set; } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "P").WithArguments("readonly").WithLocation(7, 18), + // (8,18): error CS0106: The modifier 'readonly' is not valid for this item + // readonly int this[int i] { get; } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "this").WithArguments("readonly").WithLocation(8, 18), + // (9,38): error CS0106: The modifier 'readonly' is not valid for this item + // readonly event Action E; + Diagnostic(ErrorCode.ERR_BadMemberFlag, "E").WithArguments("readonly").WithLocation(9, 38)); + } + + [Fact] + public void ReadOnlyEnum() + { + var csharp = @" +public readonly enum E +{ + readonly A, readonly B +} +"; + var comp = CreateCompilation(csharp); + comp.VerifyDiagnostics( + // (2,22): error CS0106: The modifier 'readonly' is not valid for this item + // public readonly enum E + Diagnostic(ErrorCode.ERR_BadMemberFlag, "E").WithArguments("readonly").WithLocation(2, 22), + // (3,2): error CS1041: Identifier expected; 'readonly' is a keyword + // { + Diagnostic(ErrorCode.ERR_IdentifierExpectedKW, "").WithArguments("", "readonly").WithLocation(3, 2), + // (4,17): error CS1041: Identifier expected; 'readonly' is a keyword + // readonly A, readonly B; + Diagnostic(ErrorCode.ERR_IdentifierExpectedKW, "readonly").WithArguments("", "readonly").WithLocation(4, 17)); } [Fact] @@ -331,29 +399,6 @@ readonly get comp.VerifyDiagnostics(); } - [Fact] - public void ReadOnlyClassProperty() - { - var csharp = @" -public class C -{ - public int i; - public int P - { - readonly get - { - return i; - } - } -} -"; - var comp = CreateCompilation(csharp); - comp.VerifyDiagnostics( - // (7,18): error CS0106: The modifier 'readonly' is not valid for this item - // readonly get - Diagnostic(ErrorCode.ERR_BadMemberFlag, "get").WithArguments("readonly").WithLocation(7, 18)); - } - [Fact] public void ReadOnlyStructStaticProperty() { @@ -507,14 +552,18 @@ public void ReadOnlyConstructor() var csharp = @" public struct S { + static readonly S() { } public readonly S(int i) { } } "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics( // (4,21): error CS0106: The modifier 'readonly' is not valid for this item + // static readonly S() { } + Diagnostic(ErrorCode.ERR_BadMemberFlag, "S").WithArguments("readonly").WithLocation(4, 21), + // (5,21): error CS0106: The modifier 'readonly' is not valid for this item // public readonly S(int i) { } - Diagnostic(ErrorCode.ERR_BadMemberFlag, "S").WithArguments("readonly").WithLocation(4, 21)); + Diagnostic(ErrorCode.ERR_BadMemberFlag, "S").WithArguments("readonly").WithLocation(5, 21)); } [Fact] @@ -624,15 +673,15 @@ public void ReadOnlyFieldLikeEvent() public struct S1 { - public readonly event Action OnFoo; - public void M() { OnFoo?.Invoke(new EventArgs()); } + public readonly event Action E; + public void M() { E?.Invoke(new EventArgs()); } } "; var comp = CreateCompilation(csharp); comp.VerifyDiagnostics( // (6,45): error CS0106: The modifier 'readonly' is not valid for this item - // public readonly event Action OnFoo; - Diagnostic(ErrorCode.ERR_BadMemberFlag, "OnFoo").WithArguments("readonly").WithLocation(6, 45)); + // public readonly event Action E; + Diagnostic(ErrorCode.ERR_BadMemberFlag, "E").WithArguments("readonly").WithLocation(6, 45)); } [Fact] @@ -643,7 +692,7 @@ public void ReadOnlyEventExplicitAddRemove() public struct S1 { - public readonly event Action OnFoo + public readonly event Action E { add {} remove {} @@ -662,7 +711,7 @@ public void ReadOnlyStaticEvent() public struct S1 { - public static readonly event Action OnFoo + public static readonly event Action E { add {} remove {} @@ -672,8 +721,8 @@ public static readonly event Action OnFoo var comp = CreateCompilation(csharp); comp.VerifyDiagnostics( // (6,52): error CS0106: The modifier 'readonly' is not valid for this item - // public static readonly event Action OnFoo - Diagnostic(ErrorCode.ERR_BadMemberFlag, "OnFoo").WithArguments("readonly").WithLocation(6, 52)); + // public static readonly event Action E + Diagnostic(ErrorCode.ERR_BadMemberFlag, "E").WithArguments("readonly").WithLocation(6, 52)); } [Fact] @@ -684,7 +733,7 @@ public void ReadOnlyEventReadOnlyAccessors() public struct S1 { - public event Action OnFoo + public event Action E { readonly add {} readonly remove {} From 69b1942f80622da271c0ca6a17efc32968ef5080 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 1 Mar 2019 10:58:32 -0800 Subject: [PATCH 19/19] Note that feature checks and corresponding tests are needed --- .../CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs index 98482445a52c4..43a5e0360b496 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ReadOnlyStructsTests.cs @@ -250,6 +250,8 @@ public static void Main() ); } + // PROTOTYPE: readonly members features should require C# 8.0 or greater + [Fact] public void ReadOnlyStructMethod() {