From 9df66a1f5b4a7800fc067fdf1d2c0290b373639d Mon Sep 17 00:00:00 2001 From: Charles Stoner Date: Mon, 2 Jul 2018 11:34:32 -0700 Subject: [PATCH] Misc. changes (#28239) --- docs/features/nullable-reference-types.md | 10 ++ .../CSharp/Portable/Binder/BinderFlags.cs | 2 +- .../Portable/Binder/Binder_Constraints.cs | 4 +- .../Portable/Binder/Binder_Invocation.cs | 2 +- .../CSharp/Portable/Binder/Binder_Symbols.cs | 2 +- .../Binder/ContextualAttributeBinder.cs | 2 +- .../Errors/DiagnosticInfoWithSymbols.cs | 21 ---- .../Portable/Symbols/SymbolWithAnnotations.cs | 9 +- .../Semantic/Semantics/StaticNullChecking.cs | 98 ++++++++++++++----- 9 files changed, 99 insertions(+), 51 deletions(-) diff --git a/docs/features/nullable-reference-types.md b/docs/features/nullable-reference-types.md index 3d4d108d62c..8e2e034cf83 100644 --- a/docs/features/nullable-reference-types.md +++ b/docs/features/nullable-reference-types.md @@ -179,5 +179,15 @@ A warning is reported for inconsistent top-level nullability of constraint types static void F4 where T : class, Stream? { } // warning static void F5 where T : Stream?, IDisposable { } // warning ``` +An error is reported for duplicate constraints. _Should the error be reported for duplicates that differ by top-level or nested nullability?_ +```c# +class C where T : class +{ + static void F1() where U : T?, T? { } // error: duplicate constraint + static void F2() where U : I, I { } // error: duplicate constraint + static void F3() where U : T, T? { } // error? + static void F4() where U : I, I { } // error? +} +``` ## Compiler switch _Describe behavior when feature is disabled._ diff --git a/src/Compilers/CSharp/Portable/Binder/BinderFlags.cs b/src/Compilers/CSharp/Portable/Binder/BinderFlags.cs index 15cc9156085..c031a20bca1 100644 --- a/src/Compilers/CSharp/Portable/Binder/BinderFlags.cs +++ b/src/Compilers/CSharp/Portable/Binder/BinderFlags.cs @@ -101,7 +101,7 @@ internal enum BinderFlags : uint /// /// This is a , or has as its parent. /// - InContectualAttributeBinder = 1 << 29, + InContextualAttributeBinder = 1 << 29, // Groups diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs index cf0fd4d5b29..3eb67cfcf0a 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs @@ -209,10 +209,12 @@ internal partial class Binder return false; } + // PROTOTYPE(NullableReferenceTypes): Report ERR_DuplicateBound for + // duplicates that differ by top-level or nested nullability as well? if (constraintTypes.Contains(c => type.Equals(c, TypeCompareKind.AllAspects))) { // "Duplicate constraint '{0}' for type parameter '{1}'" - Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type.TypeSymbol, typeParameterName); + Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type, typeParameterName); return false; } diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs index ee0473e0180..d013635227d 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs @@ -1119,7 +1119,7 @@ private static void CheckRestrictedTypeReceiver(BoundExpression expression, Comp private bool IsBindingModuleLevelAttribute() { - if ((this.Flags & BinderFlags.InContectualAttributeBinder) == 0) + if ((this.Flags & BinderFlags.InContextualAttributeBinder) == 0) { return false; } diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs index 975d186ca2a..869336fb87d 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs @@ -2103,7 +2103,7 @@ private AssemblySymbol GetForwardedToAssembly(string fullName, int arity, Diagno // might require us to examine types forwarded by this assembly, thus binding assembly level // attributes again. And the cycle continues. // So, we won't do the analysis in this case, at the expense of better diagnostics. - if ((this.Flags & BinderFlags.InContectualAttributeBinder) != 0) + if ((this.Flags & BinderFlags.InContextualAttributeBinder) != 0) { var current = this; diff --git a/src/Compilers/CSharp/Portable/Binder/ContextualAttributeBinder.cs b/src/Compilers/CSharp/Portable/Binder/ContextualAttributeBinder.cs index 8fc8a24edad..cdc69716702 100644 --- a/src/Compilers/CSharp/Portable/Binder/ContextualAttributeBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/ContextualAttributeBinder.cs @@ -16,7 +16,7 @@ internal sealed class ContextualAttributeBinder : Binder /// Next binder in the chain (enclosing). /// Symbol to which the attribute was applied (e.g. a parameter). public ContextualAttributeBinder(Binder enclosing, Symbol symbol) - : base(enclosing, enclosing.Flags | BinderFlags.InContectualAttributeBinder) + : base(enclosing, enclosing.Flags | BinderFlags.InContextualAttributeBinder) { _attributeTarget = symbol; _attributedMember = GetAttributedMember(symbol); diff --git a/src/Compilers/CSharp/Portable/Errors/DiagnosticInfoWithSymbols.cs b/src/Compilers/CSharp/Portable/Errors/DiagnosticInfoWithSymbols.cs index bfaa7c67cb1..795e7d6587a 100644 --- a/src/Compilers/CSharp/Portable/Errors/DiagnosticInfoWithSymbols.cs +++ b/src/Compilers/CSharp/Portable/Errors/DiagnosticInfoWithSymbols.cs @@ -1,8 +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 Microsoft.CodeAnalysis.CSharp.Symbols; using System.Collections.Immutable; -using System.Diagnostics; namespace Microsoft.CodeAnalysis.CSharp { @@ -14,32 +12,13 @@ internal class DiagnosticInfoWithSymbols : DiagnosticInfo internal DiagnosticInfoWithSymbols(ErrorCode errorCode, object[] arguments, ImmutableArray symbols) : base(CSharp.MessageProvider.Instance, (int)errorCode, arguments) { -#if DEBUG - AssertArguments(arguments); -#endif this.Symbols = symbols; } internal DiagnosticInfoWithSymbols(bool isWarningAsError, ErrorCode errorCode, object[] arguments, ImmutableArray symbols) : base(CSharp.MessageProvider.Instance, isWarningAsError, (int)errorCode, arguments) { -#if DEBUG - AssertArguments(arguments); -#endif this.Symbols = symbols; } - -#if DEBUG - private static void AssertArguments(object[] arguments) - { - if (arguments != null) - { - foreach (var argument in arguments) - { - Debug.Assert(!(argument is SymbolWithAnnotations)); - } - } - } -#endif } } diff --git a/src/Compilers/CSharp/Portable/Symbols/SymbolWithAnnotations.cs b/src/Compilers/CSharp/Portable/Symbols/SymbolWithAnnotations.cs index f4d9e93b4c2..b2c208d8eca 100644 --- a/src/Compilers/CSharp/Portable/Symbols/SymbolWithAnnotations.cs +++ b/src/Compilers/CSharp/Portable/Symbols/SymbolWithAnnotations.cs @@ -171,7 +171,7 @@ public NamespaceSymbolWithAnnotations(NamespaceSymbol namespaceSymbol) /// A simple class that combines a single type symbol with annotations /// [DebuggerDisplay("{GetDebuggerDisplay(), nq}")] - internal abstract class TypeSymbolWithAnnotations : NamespaceOrTypeSymbolWithAnnotations + internal abstract class TypeSymbolWithAnnotations : NamespaceOrTypeSymbolWithAnnotations, IFormattable { internal static readonly SymbolDisplayFormat DebuggerDisplayFormat = new SymbolDisplayFormat( typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces, @@ -342,6 +342,13 @@ public TypeSymbolWithAnnotations AsNullableReferenceTypeIfInferLocalNullability( public abstract override string ToDisplayString(SymbolDisplayFormat format = null); internal string GetDebuggerDisplay() => ToDisplayString(DebuggerDisplayFormat); + // PROTOTYPE(NullableReferenceTypes): Remove IFormattable implementation + // if instances should not be used as Diagnostic arguments. + string IFormattable.ToString(string format, IFormatProvider formatProvider) + { + return ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat); + } + public bool Equals(TypeSymbolWithAnnotations other, TypeCompareKind comparison) { if (ReferenceEquals(this, other)) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs index c5cca7d1264..082a1485b98 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs @@ -34611,12 +34611,10 @@ class C where V : V?, V? { } where T3 : class where U3 : T3, T3?;"; var comp = CreateCompilation(source, parseOptions: TestOptions.Regular8); - // PROTOTYPE(NullableReferenceTypes): Report `Duplicate constraint 'V?' ...` rather than `... 'V' ...`. - // (Requires allowing TypeSymbolWithAnnotations as Diagnostic argument.) comp.VerifyDiagnostics( - // (3,26): error CS0405: Duplicate constraint 'V' for type parameter 'V' + // (3,26): error CS0405: Duplicate constraint 'V?' for type parameter 'V' // class C where V : V?, V? { } - Diagnostic(ErrorCode.ERR_DuplicateBound, "V?").WithArguments("V", "V").WithLocation(3, 26), + Diagnostic(ErrorCode.ERR_DuplicateBound, "V?").WithArguments("V?", "V").WithLocation(3, 26), // (3,9): error CS0454: Circular constraint dependency involving 'V' and 'V' // class C where V : V?, V? { } Diagnostic(ErrorCode.ERR_CircularConstraint, "V").WithArguments("V", "V").WithLocation(3, 9), @@ -34855,6 +34853,40 @@ class C Diagnostic(ErrorCode.ERR_ConstructedDynamicTypeAsBound, "I").WithArguments("I").WithLocation(5, 35)); } + [Fact] + public void DuplicateConstraints() + { + var source = +@"interface I where T : class { } +class C where T : class +{ + static void F1() where U : T, T { } + static void F2() where U : T, T? { } + static void F3() where U : T?, T { } + static void F4() where U : T?, T? { } + static void F5() where U : I, I { } + static void F6() where U : I, I { } + static void F7() where U : I, I { } + static void F8() where U : I, I { } +}"; + var comp = CreateCompilation(source, parseOptions: TestOptions.Regular8); + // PROTOTYPE(NullableReferenceTypes): Report ERR_DuplicateBound for + // duplicates that differ by top-level or nested nullability as well? + comp.VerifyDiagnostics( + // (4,38): error CS0405: Duplicate constraint 'T' for type parameter 'U' + // static void F1() where U : T, T { } + Diagnostic(ErrorCode.ERR_DuplicateBound, "T").WithArguments("T", "U").WithLocation(4, 38), + // (7,39): error CS0405: Duplicate constraint 'T?' for type parameter 'U' + // static void F4() where U : T?, T? { } + Diagnostic(ErrorCode.ERR_DuplicateBound, "T?").WithArguments("T?", "U").WithLocation(7, 39), + // (8,41): error CS0405: Duplicate constraint 'I' for type parameter 'U' + // static void F5() where U : I, I { } + Diagnostic(ErrorCode.ERR_DuplicateBound, "I").WithArguments("I", "U").WithLocation(8, 41), + // (11,42): error CS0405: Duplicate constraint 'I' for type parameter 'U' + // static void F8() where U : I, I { } + Diagnostic(ErrorCode.ERR_DuplicateBound, "I").WithArguments("I", "U").WithLocation(11, 42)); + } + [Fact] public void PartialClassConstraintMismatch() { @@ -35217,12 +35249,10 @@ static void F(object x, object? y) public void UnannotatedConstraint_01() { var source0 = -@"public class A -{ -} -public class B where T : A -{ -}"; +@"public class A1 { } +public class A2 { } +public class B1 where T : A1 { } +public class B2 where T : A2 { }"; var comp0 = CreateCompilation(source0, parseOptions: TestOptions.Regular7); comp0.VerifyDiagnostics(); var ref0 = comp0.EmitToImageReference(); @@ -35232,25 +35262,30 @@ public class B where T : A { static void Main() { - new B(); - new B(); + new B1(); + new B1(); + new B2>(); + new B2>(); } }"; var comp = CreateCompilation(source, references: new[] { ref0 }, parseOptions: TestOptions.Regular8); comp.VerifyDiagnostics(); + var typeParameters = comp.GetMember("B1").TypeParameters; + Assert.Equal("A1", typeParameters[0].ConstraintTypesNoUseSiteDiagnostics[0].ToTestDisplayString(true)); + typeParameters = comp.GetMember("B2").TypeParameters; + Assert.Equal("A2", typeParameters[0].ConstraintTypesNoUseSiteDiagnostics[0].ToTestDisplayString(true)); } [Fact] public void UnannotatedConstraint_02() { var source0 = -@"public class A -{ -} -public class B where T : A -{ -}"; - var comp0 = CreateCompilation(source0, parseOptions: TestOptions.Regular7); +@"using System.Runtime.CompilerServices; +public class A1 { } +public class A2 { } +[NonNullTypes(false)] public class B1 where T : A1 where U : A1? { } +[NonNullTypes(false)] public class B2 where T : A2 where U : A2 { }"; + var comp0 = CreateCompilation(new[] { source0, NonNullTypesAttributesDefinition }, parseOptions: TestOptions.Regular8); comp0.VerifyDiagnostics(); var ref0 = comp0.EmitToImageReference(); @@ -35259,12 +35294,21 @@ public class B where T : A { static void Main() { - new B>(); - new B>(); + new B1(); + new B1(); + new B2, A2>(); + new B2, A2>(); } }"; var comp = CreateCompilation(source, references: new[] { ref0 }, parseOptions: TestOptions.Regular8); comp.VerifyDiagnostics(); + // PROTOTYPE(NullableReferenceTypes): Should be ~ rather than !. + var typeParameters = comp.GetMember("B1").TypeParameters; + Assert.Equal("A1!", typeParameters[0].ConstraintTypesNoUseSiteDiagnostics[0].ToTestDisplayString(true)); + Assert.Equal("A1?", typeParameters[1].ConstraintTypesNoUseSiteDiagnostics[0].ToTestDisplayString(true)); + typeParameters = comp.GetMember("B2").TypeParameters; + Assert.Equal("A2!", typeParameters[0].ConstraintTypesNoUseSiteDiagnostics[0].ToTestDisplayString(true)); + Assert.Equal("A2!", typeParameters[1].ConstraintTypesNoUseSiteDiagnostics[0].ToTestDisplayString(true)); } [Fact] @@ -35288,14 +35332,20 @@ class C { static void Main() { - A.F(new B1()); - A.F(new B2()); + A.F(null); // warning + A.F(null); // warning A.F(null); A.F(null); } }"; var comp = CreateCompilation(source, parseOptions: TestOptions.Regular8, references: new[] { ref0 }); - comp.VerifyDiagnostics(); + comp.VerifyDiagnostics( + // (7,17): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // A.F(null); // warning + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(7, 17), + // (8,17): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // A.F(null); // warning + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(8, 17)); } [Fact] -- GitLab