diff --git a/src/Compilers/CSharp/Portable/BoundTree/Expression.cs b/src/Compilers/CSharp/Portable/BoundTree/Expression.cs index fd7cef9a5454928015d13c98d7dec9946c43cbda..fc3716cc64f6cc5446b593aceb5c9eb55b633808 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/Expression.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/Expression.cs @@ -51,7 +51,7 @@ ImmutableArray IInvocationExpression.ArgumentsInSourceOrder ArrayBuilder sourceOrderArguments = ArrayBuilder.GetInstance(this.Arguments.Length); for (int argumentIndex = 0; argumentIndex < this.Arguments.Length; argumentIndex++) { - IArgument argument = DeriveArgument(this.ArgsToParamsOpt.IsDefault ? argumentIndex : this.ArgsToParamsOpt[argumentIndex], argumentIndex, this.Arguments, this.ArgumentNamesOpt, this.ArgumentRefKindsOpt, this.Method.Parameters); + IArgument argument = DeriveArgument(this.ArgsToParamsOpt.IsDefault ? argumentIndex : this.ArgsToParamsOpt[argumentIndex], argumentIndex, this.Arguments, this.ArgumentNamesOpt, this.ArgumentRefKindsOpt, this.Method.Parameters, this.Syntax); sourceOrderArguments.Add(argument); if (argument.ArgumentKind == ArgumentKind.ParamArray) { @@ -62,12 +62,12 @@ ImmutableArray IInvocationExpression.ArgumentsInSourceOrder return sourceOrderArguments.ToImmutableAndFree(); } } - - ImmutableArray IInvocationExpression.ArgumentsInParameterOrder => DeriveArguments(this.Arguments, this.ArgumentNamesOpt, this.ArgsToParamsOpt, this.ArgumentRefKindsOpt, this.Method.Parameters); + + ImmutableArray IInvocationExpression.ArgumentsInParameterOrder => DeriveArguments(this.Arguments, this.ArgumentNamesOpt, this.ArgsToParamsOpt, this.ArgumentRefKindsOpt, this.Method.Parameters, this.Syntax); IArgument IInvocationExpression.ArgumentMatchingParameter(IParameterSymbol parameter) { - return ArgumentMatchingParameter(this.Arguments, this.ArgsToParamsOpt, this.ArgumentNamesOpt, this.ArgumentRefKindsOpt, parameter.ContainingSymbol as Symbols.MethodSymbol, parameter); + return ArgumentMatchingParameter(this.Arguments, this.ArgsToParamsOpt, this.ArgumentNamesOpt, this.ArgumentRefKindsOpt, parameter.ContainingSymbol as Symbols.MethodSymbol, parameter, this.Syntax); } protected override OperationKind ExpressionKind => OperationKind.InvocationExpression; @@ -82,7 +82,7 @@ public override void Accept(OperationVisitor visitor) return visitor.VisitInvocationExpression(this, argument); } - internal static ImmutableArray DeriveArguments(ImmutableArray boundArguments, ImmutableArray argumentNames, ImmutableArray argumentsToParameters, ImmutableArray argumentRefKinds, ImmutableArray parameters) + internal static ImmutableArray DeriveArguments(ImmutableArray boundArguments, ImmutableArray argumentNames, ImmutableArray argumentsToParameters, ImmutableArray argumentRefKinds, ImmutableArray parameters, SyntaxNode invocationSyntax) { ArrayBuilder arguments = ArrayBuilder.GetInstance(boundArguments.Length); for (int parameterIndex = 0; parameterIndex < parameters.Length; parameterIndex++) @@ -112,12 +112,12 @@ internal static ImmutableArray DeriveArguments(ImmutableArray DeriveArguments(ImmutableArray s_argumentMappings = new ConditionalWeakTable(); - private static IArgument DeriveArgument(int parameterIndex, int argumentIndex, ImmutableArray boundArguments, ImmutableArray argumentNames, ImmutableArray argumentRefKinds, ImmutableArray parameters) + private static IArgument DeriveArgument(int parameterIndex, int argumentIndex, ImmutableArray boundArguments, ImmutableArray argumentNames, ImmutableArray argumentRefKinds, ImmutableArray parameters, SyntaxNode invocationSyntax) { if (argumentIndex >= boundArguments.Length) { @@ -136,7 +136,7 @@ private static IArgument DeriveArgument(int parameterIndex, int argumentIndex, I Symbols.ParameterSymbol lastParameter = parameters[parameters.Length - 1]; if (lastParameter.IsParams) { - return new Argument(ArgumentKind.ParamArray, lastParameter, CreateParamArray(lastParameter, boundArguments, argumentIndex)); + return new Argument(ArgumentKind.ParamArray, lastParameter, CreateParamArray(lastParameter, boundArguments, argumentIndex, invocationSyntax)); } } @@ -167,7 +167,7 @@ private static IArgument DeriveArgument(int parameterIndex, int argumentIndex, I argument.Type.TypeKind != TypeKind.Array || !argument.Type.Equals(parameters[parameters.Length - 1].Type, ignoreCustomModifiersAndArraySizesAndLowerBounds: true))) { - return new Argument(ArgumentKind.ParamArray, parameters[parameters.Length - 1], CreateParamArray(parameters[parameters.Length - 1], boundArguments, argumentIndex)); + return new Argument(ArgumentKind.ParamArray, parameters[parameters.Length - 1], CreateParamArray(parameters[parameters.Length - 1], boundArguments, argumentIndex, invocationSyntax)); } else { @@ -178,30 +178,33 @@ private static IArgument DeriveArgument(int parameterIndex, int argumentIndex, I return new Argument(ArgumentKind.Named, parameters[parameterIndex], argument); }); } - - private static IExpression CreateParamArray(IParameterSymbol parameter, ImmutableArray boundArguments, int firstArgumentElementIndex) + + private static IExpression CreateParamArray(IParameterSymbol parameter, ImmutableArray boundArguments, int firstArgumentElementIndex, SyntaxNode invocationSyntax) { if (parameter.Type.TypeKind == TypeKind.Array) { IArrayTypeSymbol arrayType = (IArrayTypeSymbol)parameter.Type; - ArrayBuilder paramArrayArguments = ArrayBuilder.GetInstance(boundArguments.Length - firstArgumentElementIndex); + ArrayBuilder builder = ArrayBuilder.GetInstance(boundArguments.Length - firstArgumentElementIndex); + for (int index = firstArgumentElementIndex; index < boundArguments.Length; index++) { - paramArrayArguments.Add(boundArguments[index]); + builder.Add(boundArguments[index]); } + var paramArrayArguments = builder.ToImmutableAndFree(); - return new ArrayCreation(arrayType, paramArrayArguments.ToImmutableAndFree(), boundArguments.Length - 1 > firstArgumentElementIndex ? boundArguments[firstArgumentElementIndex].Syntax : null); + // Use the invocation syntax node if there is no actual syntax available for the argument (because the paramarray is empty.) + return new ArrayCreation(arrayType, paramArrayArguments, paramArrayArguments.Length > 0 ? paramArrayArguments[0].Syntax : invocationSyntax); } return null; } - internal static IArgument ArgumentMatchingParameter(ImmutableArray arguments, ImmutableArray argumentsToParameters, ImmutableArray argumentNames, ImmutableArray argumentRefKinds, Symbols.MethodSymbol targetMethod, IParameterSymbol parameter) + internal static IArgument ArgumentMatchingParameter(ImmutableArray arguments, ImmutableArray argumentsToParameters, ImmutableArray argumentNames, ImmutableArray argumentRefKinds, Symbols.MethodSymbol targetMethod, IParameterSymbol parameter, SyntaxNode invocationSyntax) { int argumentIndex = ArgumentIndexMatchingParameter(arguments, argumentsToParameters, targetMethod, parameter); if (argumentIndex >= 0) { - return DeriveArgument(parameter.Ordinal, argumentIndex, arguments, argumentNames, argumentRefKinds, targetMethod.Parameters); + return DeriveArgument(parameter.Ordinal, argumentIndex, arguments, argumentNames, argumentRefKinds, targetMethod.Parameters, invocationSyntax); } return null; @@ -462,11 +465,11 @@ internal partial class BoundObjectCreationExpression : IObjectCreationExpression IMethodSymbol IObjectCreationExpression.Constructor => this.Constructor; - ImmutableArray IObjectCreationExpression.ConstructorArguments => BoundCall.DeriveArguments(this.Arguments, this.ArgumentNamesOpt, this.ArgsToParamsOpt, this.ArgumentRefKindsOpt, this.Constructor.Parameters); + ImmutableArray IObjectCreationExpression.ConstructorArguments => BoundCall.DeriveArguments(this.Arguments, this.ArgumentNamesOpt, this.ArgsToParamsOpt, this.ArgumentRefKindsOpt, this.Constructor.Parameters, this.Syntax); IArgument IObjectCreationExpression.ArgumentMatchingParameter(IParameterSymbol parameter) { - return BoundCall.ArgumentMatchingParameter(this.Arguments, this.ArgsToParamsOpt, this.ArgumentNamesOpt, this.ArgumentRefKindsOpt, this.Constructor, parameter); + return BoundCall.ArgumentMatchingParameter(this.Arguments, this.ArgsToParamsOpt, this.ArgumentNamesOpt, this.ArgumentRefKindsOpt, this.Constructor, parameter, this.Syntax); } ImmutableArray IObjectCreationExpression.MemberInitializers diff --git a/src/Compilers/CSharp/Test/Semantic/Diagnostics/OperationAnalyzerTests.cs b/src/Compilers/CSharp/Test/Semantic/Diagnostics/OperationAnalyzerTests.cs index 3658ecbd697ff7c8bc1db4760eb51e09fefae833..d18842b595e120ba3c8ea5586aa76743adf05434 100644 --- a/src/Compilers/CSharp/Test/Semantic/Diagnostics/OperationAnalyzerTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Diagnostics/OperationAnalyzerTests.cs @@ -263,6 +263,7 @@ public void M2() M0(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); M0(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13); M0(1); + M0(1, 2); M0(1, 2, 4, 3); } @@ -301,13 +302,13 @@ public void M6() Diagnostic(InvocationTestAnalyzer.OutOfNumericalOrderArgumentsDescriptor.Id, "4").WithLocation(17, 33), Diagnostic(InvocationTestAnalyzer.BigParamArrayArgumentsDescriptor.Id, "M0(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12)").WithLocation(19, 9), Diagnostic(InvocationTestAnalyzer.BigParamArrayArgumentsDescriptor.Id, "M0(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13)").WithLocation(20, 9), - Diagnostic(InvocationTestAnalyzer.OutOfNumericalOrderArgumentsDescriptor.Id, "3").WithLocation(22, 21), - Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3(0)").WithArguments("y").WithLocation(32, 9), - Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3(y: null)").WithArguments("x").WithLocation(33, 9), - Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3(x: 0)").WithArguments("y").WithLocation(34, 9), - Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3()").WithArguments("x").WithLocation(35, 9), - Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3()").WithArguments("y").WithLocation(35, 9), - Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M5(b: new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11})").WithArguments("x").WithLocation(46, 9) + Diagnostic(InvocationTestAnalyzer.OutOfNumericalOrderArgumentsDescriptor.Id, "3").WithLocation(23, 21), + Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3(0)").WithArguments("y").WithLocation(33, 9), + Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3(y: null)").WithArguments("x").WithLocation(34, 9), + Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3(x: 0)").WithArguments("y").WithLocation(35, 9), + Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3()").WithArguments("x").WithLocation(36, 9), + Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M3()").WithArguments("y").WithLocation(36, 9), + Diagnostic(InvocationTestAnalyzer.UseDefaultArgumentDescriptor.Id, "M5(b: new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11})").WithArguments("x").WithLocation(47, 9) ); } @@ -1329,5 +1330,36 @@ void Foo() Diagnostic(StaticMemberTestAnalyzer.StaticMemberDescriptor.Id, "D.Method()").WithLocation(30, 9) ); } + + [WorkItem(8520, "https://github.com/dotnet/roslyn/issues/8520")] + [Fact] + public void NullOperationSyntaxCSharp() + { + const string source = @" +class C +{ + public void M0(params int[] b) + { + } + + public void M1() + { + M0(); + M0(1); + M0(1, 2); + M0(new int[] { }); + M0(new int[] { 1 }); + M0(new int[] { 1, 2}); + } +} +"; + CreateCompilationWithMscorlib45(source) + .VerifyDiagnostics() + .VerifyAnalyzerDiagnostics(new DiagnosticAnalyzer[] { new NullOperationSyntaxTestAnalyzer() }, null, null, false, + // TODO: missing a params array for M0() + // https://github.com/dotnet/roslyn/issues/8566 + Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "1").WithLocation(11, 12), + Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "1").WithLocation(12, 12)); + } } } diff --git a/src/Compilers/Core/CodeAnalysisTest/Diagnostics/OperationTestAnalyzer.cs b/src/Compilers/Core/CodeAnalysisTest/Diagnostics/OperationTestAnalyzer.cs index d1abf94d024f3c6c1d03d96ce99975a531cef29e..524c9b4390e50ee9c832185042cd554e55f91289 100644 --- a/src/Compilers/Core/CodeAnalysisTest/Diagnostics/OperationTestAnalyzer.cs +++ b/src/Compilers/Core/CodeAnalysisTest/Diagnostics/OperationTestAnalyzer.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis.Diagnostics; @@ -1306,4 +1307,93 @@ public sealed override void Initialize(AnalysisContext context) OperationKind.EventAssignmentExpression); } } + + public class NullOperationSyntaxTestAnalyzer : DiagnosticAnalyzer + { + private const string ReliabilityCategory = "Reliability"; + + // We should not see this warning triggered by any code + public static readonly DiagnosticDescriptor NullOperationSyntaxDescriptor = new DiagnosticDescriptor( + "NullOperationSyntax", + "null operation Syntax found", + "An IOperation with Syntax property of value null is found", + ReliabilityCategory, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + // since we don't expect to see the first diagnostic, we created this one to make sure + // the test didn't pass because the analyzer crashed. + public static readonly DiagnosticDescriptor ParamsArrayOperationDescriptor = new DiagnosticDescriptor( + "ParamsArray", + "Params array argument found", + "A params array argument is found", + ReliabilityCategory, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + public sealed override ImmutableArray SupportedDiagnostics + { + get { return ImmutableArray.Create(NullOperationSyntaxDescriptor, ParamsArrayOperationDescriptor); } + } + + public sealed override void Initialize(AnalysisContext context) + { + context.RegisterOperationAction( + (operationContext) => + { + var nullList = new List(); + var paramsList = new List(); + var collector = new Walker(nullList, paramsList); + collector.Visit(operationContext.Operation); + + foreach (var nullSyntaxOperation in nullList) + { + operationContext.ReportDiagnostic( + Diagnostic.Create(NullOperationSyntaxDescriptor, null)); + } + foreach (var paramsarrayArgumentOperation in paramsList) + { + operationContext.ReportDiagnostic( + Diagnostic.Create(ParamsArrayOperationDescriptor, + paramsarrayArgumentOperation.Syntax.GetLocation())); + } + + }, + OperationKind.InvocationExpression); + } + + // this OperationWalker collect: + // 1. all the operation with null Syntax property + // 2. all the params array argument operations + private sealed class Walker : OperationWalker + { + private readonly List _nullList; + private readonly List _paramsList; + + public Walker(List nullList, List paramsList) + { + _nullList = nullList; + _paramsList = paramsList; + } + + public override void Visit(IOperation operation) + { + if (operation != null) + { + if (operation.Syntax == null) + { + _nullList.Add(operation); + } + if (operation.Kind == OperationKind.Argument) + { + if (((IArgument)operation).ArgumentKind == ArgumentKind.ParamArray) + { + _paramsList.Add(operation); + } + } + } + base.Visit(operation); + } + } + } } \ No newline at end of file diff --git a/src/Compilers/VisualBasic/Test/Semantic/Diagnostics/OperationAnalyzerTests.vb b/src/Compilers/VisualBasic/Test/Semantic/Diagnostics/OperationAnalyzerTests.vb index 742172dae32e65ddc0f6732fe97468bfc002c976..7b491076e01dd0d37176508b5a607261f4e42834 100644 --- a/src/Compilers/VisualBasic/Test/Semantic/Diagnostics/OperationAnalyzerTests.vb +++ b/src/Compilers/VisualBasic/Test/Semantic/Diagnostics/OperationAnalyzerTests.vb @@ -1430,5 +1430,40 @@ End Class Diagnostic(StaticMemberTestAnalyzer.StaticMemberDescriptor.Id, "D.P").WithLocation(26, 17), Diagnostic(StaticMemberTestAnalyzer.StaticMemberDescriptor.Id, "D.Method()").WithLocation(27, 9)) End Sub + + + Public Sub NullOperationSyntaxVisualBasic() + Dim source = + + + + + + ' TODO: array should not be treated as ParamArray argument + ' https://github.com/dotnet/roslyn/issues/8570 + Dim comp = CompilationUtils.CreateCompilationWithMscorlibAndVBRuntime(source) + comp.VerifyDiagnostics() + comp.VerifyAnalyzerDiagnostics({New NullOperationSyntaxTestAnalyzer}, Nothing, Nothing, False, + Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "M0()").WithLocation(6, 9), + Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "M0(1)").WithLocation(7, 9), + Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "M0(1, 2)").WithLocation(8, 9), + Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "New Integer() { }").WithLocation(9, 12), + Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "New Integer() { 1 }").WithLocation(10, 12), + Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "New Integer() { 1, 2 }").WithLocation(11, 12)) + End Sub End Class End Namespace