Report nullability for attribute arguments: (#33816)

* Report nullability for attribute arguments:
- Add an entry point to nullable walker for attribute analysis
- Implement analyze attribute in the walker
- Update MethodThisParameter to not throw
- Add extra fields to BoundAttribute that are needed for nullable analysis
- Add tests



* PR Feedback:
- Analyse attributes even when we don't bind a constructor

- Add tests for when we don't bind constructor but still want to analyze what we have

* PR Feedback:
- fix typos
- make no matching constructors use implicit constructors

* Replace tabs with spaces

* Fix failing test

src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs

@@ -147,21 +147,33 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a
             AnalyzedAttributeArguments analyzedArguments = attributeArgumentBinder.BindAttributeArguments(argumentListOpt, attributeTypeForBinding, diagnostics);
 
             HashSet<DiagnosticInfo> useSiteDiagnostics = null;
+            ImmutableArray<int> argsToParamsOpt = default;
+            bool expanded = false;
+            MethodSymbol attributeConstructor = null;
 
             // Bind attributeType's constructor based on the bound constructor arguments
-            MethodSymbol attributeConstructor = attributeTypeForBinding.IsErrorType() ?
-                null :
-                BindAttributeConstructor(node, attributeTypeForBinding, analyzedArguments.ConstructorArguments, diagnostics, ref resultKind, suppressErrors: attributeType.IsErrorType(), useSiteDiagnostics: ref useSiteDiagnostics);
+            if (!attributeTypeForBinding.IsErrorType())
+            {
+                attributeConstructor = BindAttributeConstructor(node,
+                                                                attributeTypeForBinding,
+                                                                analyzedArguments.ConstructorArguments,
+                                                                diagnostics,
+                                                                ref resultKind,
+                                                                suppressErrors: attributeType.IsErrorType(),
+                                                                ref argsToParamsOpt,
+                                                                ref expanded,
+                                                                ref useSiteDiagnostics);
+            }
             diagnostics.Add(node, useSiteDiagnostics);
 
-            if ((object)attributeConstructor != null)
+            if (!(attributeConstructor is null))
             {
                 ReportDiagnosticsIfObsolete(diagnostics, attributeConstructor, node, hasBaseReceiver: false);
-            }
 
-            if (attributeConstructor?.Parameters.Any(p => p.RefKind == RefKind.In) == true)
-            {
-                Error(diagnostics, ErrorCode.ERR_AttributeCtorInParameter, node, attributeConstructor.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat));
+                if (attributeConstructor.Parameters.Any(p => p.RefKind == RefKind.In))
+                {
+                    Error(diagnostics, ErrorCode.ERR_AttributeCtorInParameter, node, attributeConstructor.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat));
+                }
             }
 
             var constructorArguments = analyzedArguments.ConstructorArguments;
@@ -170,10 +182,9 @@ private BoundAttribute BindAttributeCore(AttributeSyntax node, NamedTypeSymbol a
             ImmutableArray<BoundExpression> boundNamedArguments = analyzedArguments.NamedArguments;
             constructorArguments.Free();
 
-            return new BoundAttribute(node, attributeConstructor, boundConstructorArguments, boundConstructorArgumentNamesOpt,
+            return new BoundAttribute(node, attributeConstructor, boundConstructorArguments, boundConstructorArgumentNamesOpt, argsToParamsOpt, expanded,
                 boundNamedArguments, resultKind, attributeType, hasErrors: resultKind != LookupResultKind.Viable);
         }
-
         private CSharpAttributeData GetAttribute(BoundAttribute boundAttribute, DiagnosticBag diagnostics)
         {
             var attributeType = (NamedTypeSymbol)boundAttribute.Type;
@@ -181,6 +192,8 @@ private CSharpAttributeData GetAttribute(BoundAttribute boundAttribute, Diagnost
 
             Debug.Assert((object)attributeType != null);
 
+            NullableWalker.AnalyzeIfNeeded(Compilation, boundAttribute, diagnostics);
+
             bool hasErrors = boundAttribute.HasAnyErrors;
 
             if (attributeType.IsErrorType() || attributeType.IsAbstract || (object)attributeConstructor == null)
@@ -484,13 +497,15 @@ private TypeSymbol BindNamedAttributeArgumentType(AttributeArgumentSyntax namedA
             return namedArgumentType;
         }
 
-        protected virtual MethodSymbol BindAttributeConstructor(
+        protected MethodSymbol BindAttributeConstructor(
             AttributeSyntax node,
             NamedTypeSymbol attributeType,
             AnalyzedArguments boundConstructorArguments,
             DiagnosticBag diagnostics,
             ref LookupResultKind resultKind,
             bool suppressErrors,
+            ref ImmutableArray<int> argsToParamsOpt,
+            ref bool expanded,
             ref HashSet<DiagnosticInfo> useSiteDiagnostics)
         {
             MemberResolutionResult<MethodSymbol> memberResolutionResult;
@@ -511,7 +526,8 @@ private TypeSymbol BindNamedAttributeArgumentType(AttributeArgumentSyntax namedA
                         LookupResultKind.Inaccessible :
                         LookupResultKind.OverloadResolutionFailure);
             }
-
+            argsToParamsOpt = memberResolutionResult.Result.ArgsToParamsOpt;
+            expanded = memberResolutionResult.Result.Kind == MemberResolutionKind.ApplicableInExpandedForm;
             return memberResolutionResult.Member;
         }
 

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -1438,6 +1438,8 @@
     <Field Name="Constructor" Type="MethodSymbol" Null="allow"/>
     <Field Name="ConstructorArguments" Type="ImmutableArray&lt;BoundExpression&gt;"/>
     <Field Name="ConstructorArgumentNamesOpt" Type="ImmutableArray&lt;string&gt;" Null="allow"/>
+    <Field Name="ConstructorArgumentsToParamsOpt" Type="ImmutableArray&lt;int&gt;" Null="allow"/>
+    <Field Name="ConstructorExpanded" Type="bool" />
     <Field Name="NamedArguments" Type ="ImmutableArray&lt;BoundExpression&gt;"/>
     <Field Name="ResultKind" PropertyOverrides="true" Type="LookupResultKind"/>
   </Node>

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

@@ -423,8 +423,9 @@ protected ParameterSymbol MethodThisParameter
         {
             get
             {
-                var method = _symbol as MethodSymbol;
-                return (object)method == null ? null : method.ThisParameter;
+                ParameterSymbol thisParameter = null;
+                (_symbol as MethodSymbol)?.TryGetThisParameter(out thisParameter);
+                return thisParameter;
             }
         }
 

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -265,7 +265,7 @@ protected override ImmutableArray<PendingBranch> Scan(ref bool badRegion)
             this.State = TopState();                   // entry point is reachable
             this.regionPlace = RegionPlace.Before;
             EnterParameters();                               // with parameters assigned
-            if ((object)methodThisParameter != null)
+            if (!(methodThisParameter is null))
             {
                 EnterParameter(methodThisParameter, methodThisParameter.Type);
             }
@@ -288,6 +288,19 @@ protected override ImmutableArray<PendingBranch> Scan(ref bool badRegion)
             Analyze(compilation, method, node, diagnostics, useMethodSignatureReturnType: false, useMethodSignatureParameterTypes: false, methodSignatureOpt: null, returnTypes: null, initialState: null, callbackOpt);
         }
 
+        internal static void AnalyzeIfNeeded(
+            CSharpCompilation compilation,
+            BoundAttribute attribute,
+            DiagnosticBag diagnostics)
+        {
+            if (compilation.LanguageVersion < MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion())
+            {
+                return;
+            }
+
+            Analyze(compilation, null, attribute, diagnostics, useMethodSignatureReturnType: false, useMethodSignatureParameterTypes: false, methodSignatureOpt: null, returnTypes: null, initialState: null, callbackOpt: null);
+        }
+
         internal static void Analyze(
             CSharpCompilation compilation,
             BoundLambda lambda,
@@ -1097,7 +1110,13 @@ protected override LocalState ReachableBottomState()
 
         private void EnterParameters()
         {
-            var methodParameters = ((MethodSymbol)_symbol).Parameters;
+            var methodSymbol = _symbol as MethodSymbol;
+            if (methodSymbol is null)
+            {
+                return;
+            }
+
+            var methodParameters = methodSymbol.Parameters;
             var signatureParameters = _useMethodSignatureParameterTypes ? _methodSignatureOpt.Parameters : methodParameters;
             Debug.Assert(signatureParameters.Length == methodParameters.Length);
             int n = methodParameters.Length;
@@ -5726,6 +5745,18 @@ public override BoundNode VisitLockStatement(BoundLockStatement node)
             return null;
         }
 
+        public override BoundNode VisitAttribute(BoundAttribute node)
+        {
+            VisitArguments(node, node.ConstructorArguments, ImmutableArray<RefKind>.Empty, node.Constructor, argsToParamsOpt: node.ConstructorArgumentsToParamsOpt, expanded: node.ConstructorExpanded);
+            foreach (var assignment in node.NamedArguments)
+            {
+                Visit(assignment);
+            }
+
+            SetNotNullResult(node);
+            return null;
+        }
+
         protected override string Dump(LocalState state)
         {
             if (!state.Reachable)

src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs

@@ -6015,7 +6015,7 @@ protected override BoundExpression ShallowClone()
 
     internal sealed partial class BoundAttribute : BoundExpression
     {
-        public BoundAttribute(SyntaxNode syntax, MethodSymbol constructor, ImmutableArray<BoundExpression> constructorArguments, ImmutableArray<string> constructorArgumentNamesOpt, ImmutableArray<BoundExpression> namedArguments, LookupResultKind resultKind, TypeSymbol type, bool hasErrors = false)
+        public BoundAttribute(SyntaxNode syntax, MethodSymbol constructor, ImmutableArray<BoundExpression> constructorArguments, ImmutableArray<string> constructorArgumentNamesOpt, ImmutableArray<int> constructorArgumentsToParamsOpt, bool constructorExpanded, ImmutableArray<BoundExpression> namedArguments, LookupResultKind resultKind, TypeSymbol type, bool hasErrors = false)
             : base(BoundKind.Attribute, syntax, type, hasErrors || constructorArguments.HasErrors() || namedArguments.HasErrors())
         {
 
@@ -6026,6 +6026,8 @@ public BoundAttribute(SyntaxNode syntax, MethodSymbol constructor, ImmutableArra
             this.Constructor = constructor;
             this.ConstructorArguments = constructorArguments;
             this.ConstructorArgumentNamesOpt = constructorArgumentNamesOpt;
+            this.ConstructorArgumentsToParamsOpt = constructorArgumentsToParamsOpt;
+            this.ConstructorExpanded = constructorExpanded;
             this.NamedArguments = namedArguments;
             this._ResultKind = resultKind;
         }
@@ -6037,6 +6039,10 @@ public BoundAttribute(SyntaxNode syntax, MethodSymbol constructor, ImmutableArra
 
         public ImmutableArray<string> ConstructorArgumentNamesOpt { get; }
 
+        public ImmutableArray<int> ConstructorArgumentsToParamsOpt { get; }
+
+        public bool ConstructorExpanded { get; }
+
         public ImmutableArray<BoundExpression> NamedArguments { get; }
 
         private readonly LookupResultKind _ResultKind;
@@ -6047,11 +6053,11 @@ public override BoundNode Accept(BoundTreeVisitor visitor)
             return visitor.VisitAttribute(this);
         }
 
-        public BoundAttribute Update(MethodSymbol constructor, ImmutableArray<BoundExpression> constructorArguments, ImmutableArray<string> constructorArgumentNamesOpt, ImmutableArray<BoundExpression> namedArguments, LookupResultKind resultKind, TypeSymbol type)
+        public BoundAttribute Update(MethodSymbol constructor, ImmutableArray<BoundExpression> constructorArguments, ImmutableArray<string> constructorArgumentNamesOpt, ImmutableArray<int> constructorArgumentsToParamsOpt, bool constructorExpanded, ImmutableArray<BoundExpression> namedArguments, LookupResultKind resultKind, TypeSymbol type)
         {
-            if (constructor != this.Constructor || constructorArguments != this.ConstructorArguments || constructorArgumentNamesOpt != this.ConstructorArgumentNamesOpt || namedArguments != this.NamedArguments || resultKind != this.ResultKind || !TypeSymbol.Equals(type, this.Type, TypeCompareKind.ConsiderEverything))
+            if (constructor != this.Constructor || constructorArguments != this.ConstructorArguments || constructorArgumentNamesOpt != this.ConstructorArgumentNamesOpt || constructorArgumentsToParamsOpt != this.ConstructorArgumentsToParamsOpt || constructorExpanded != this.ConstructorExpanded || namedArguments != this.NamedArguments || resultKind != this.ResultKind || !TypeSymbol.Equals(type, this.Type, TypeCompareKind.ConsiderEverything))
             {
-                var result = new BoundAttribute(this.Syntax, constructor, constructorArguments, constructorArgumentNamesOpt, namedArguments, resultKind, type, this.HasErrors);
+                var result = new BoundAttribute(this.Syntax, constructor, constructorArguments, constructorArgumentNamesOpt, constructorArgumentsToParamsOpt, constructorExpanded, namedArguments, resultKind, type, this.HasErrors);
                 result.CopyAttributes(this);
                 return result;
             }
@@ -6060,7 +6066,7 @@ public BoundAttribute Update(MethodSymbol constructor, ImmutableArray<BoundExpre
 
         protected override BoundExpression ShallowClone()
         {
-            var result = new BoundAttribute(this.Syntax, this.Constructor, this.ConstructorArguments, this.ConstructorArgumentNamesOpt, this.NamedArguments, this.ResultKind, this.Type, this.HasErrors);
+            var result = new BoundAttribute(this.Syntax, this.Constructor, this.ConstructorArguments, this.ConstructorArgumentNamesOpt, this.ConstructorArgumentsToParamsOpt, this.ConstructorExpanded, this.NamedArguments, this.ResultKind, this.Type, this.HasErrors);
             result.CopyAttributes(this);
             return result;
         }
@@ -11801,7 +11807,7 @@ public override BoundNode VisitAttribute(BoundAttribute node)
             ImmutableArray<BoundExpression> constructorArguments = (ImmutableArray<BoundExpression>)this.VisitList(node.ConstructorArguments);
             ImmutableArray<BoundExpression> namedArguments = (ImmutableArray<BoundExpression>)this.VisitList(node.NamedArguments);
             TypeSymbol type = this.VisitType(node.Type);
-            return node.Update(node.Constructor, constructorArguments, node.ConstructorArgumentNamesOpt, namedArguments, node.ResultKind, type);
+            return node.Update(node.Constructor, constructorArguments, node.ConstructorArgumentNamesOpt, node.ConstructorArgumentsToParamsOpt, node.ConstructorExpanded, namedArguments, node.ResultKind, type);
         }
         public override BoundNode VisitObjectCreationExpression(BoundObjectCreationExpression node)
         {
@@ -13541,6 +13547,8 @@ public override TreeDumperNode VisitAttribute(BoundAttribute node, object arg)
                 new TreeDumperNode("constructor", node.Constructor, null),
                 new TreeDumperNode("constructorArguments", null, from x in node.ConstructorArguments select Visit(x, null)),
                 new TreeDumperNode("constructorArgumentNamesOpt", node.ConstructorArgumentNamesOpt, null),
+                new TreeDumperNode("constructorArgumentsToParamsOpt", node.ConstructorArgumentsToParamsOpt, null),
+                new TreeDumperNode("constructorExpanded", node.ConstructorExpanded, null),
                 new TreeDumperNode("namedArguments", null, from x in node.NamedArguments select Visit(x, null)),
                 new TreeDumperNode("resultKind", node.ResultKind, null),
                 new TreeDumperNode("type", node.Type, null),