Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IInvocationExpression and IArgument refinement and testing #11450

Closed
wants to merge 24 commits into from
Closed

IInvocationExpression and IArgument refinement and testing #11450

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
95eed9f
Beginning work on serious IOperation testing.
msJohnHamby Apr 20, 2016
f09e703
More testing.
msJohnHamby Apr 21, 2016
22f4439
More testing.
msJohnHamby Apr 21, 2016
f8fcd33
Progress on testing IInvocationExpression and IArgument.
msJohnHamby Apr 22, 2016
8be60e4
Rename IInvocationExpression.ArgumentsInSourceOrder to ArgumentsInEva…
msJohnHamby Apr 22, 2016
8b915a8
Renamed a test class.
msJohnHamby Apr 22, 2016
ed039e1
Finished renaming the test class.
msJohnHamby Apr 22, 2016
e089f4e
Added a delegate invocation test.
msJohnHamby Apr 22, 2016
c15bc77
More.
msJohnHamby Apr 23, 2016
da75981
Merge remote-tracking branch 'upstream/master' into IOperationRescue
msJohnHamby May 9, 2016
b6e3d29
More
msJohnHamby May 10, 2016
dcb51b2
Merge remote-tracking branch 'upstream/master' into IOperationRescue
msJohnHamby May 17, 2016
4f358d6
Removed ArgumentKind from IArgument.
msJohnHamby May 18, 2016
6843108
Progress on VB tests.
msJohnHamby May 19, 2016
297466f
Made virtual invocations test work for VB.
msJohnHamby May 19, 2016
a340980
Added more VB tests.
msJohnHamby May 19, 2016
f0805f9
Viable unit of C# and VB tests.
msJohnHamby May 20, 2016
4e5617e
Moved ArgumentsInEvaluationOrder from IInvocationExpression to IHasAr…
msJohnHamby May 20, 2016
4a94d83
Removed ArgumentBase class.
msJohnHamby May 20, 2016
dc65eaf
Partial responses to review feedback. Not ready for further review.
msJohnHamby May 24, 2016
375839d
Merge remote-tracking branch 'upstream/features/ioperation' into IOpe…
msJohnHamby May 25, 2016
8a72012
Employ helper methods to make tests smaller and easier to understand.
msJohnHamby Jun 2, 2016
5e97607
Review feedback now addressed, except for the issue of trying to use …
msJohnHamby Jun 3, 2016
b1c39ce
Beginning work to leverate LocalRewriter in argument processing.
msJohnHamby Jun 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 83 additions & 94 deletions src/Compilers/CSharp/Portable/BoundTree/Expression.cs
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/Compilers/CSharp/Test/Semantic/CSharpCompilerSemanticTest.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
<Compile Include="Diagnostics\DiagnosticAnalyzerTests.AllInOne.cs" />
<Compile Include="Diagnostics\DiagnosticAnalyzerTests.cs" />
<Compile Include="Diagnostics\GetDiagnosticsTests.cs" />
<Compile Include="Diagnostics\IInvocationExpressionTests.cs" />
<Compile Include="Diagnostics\OperationAnalyzerTests.cs" />
<Compile Include="FlowAnalysis\FlowDiagnosticTests.cs" />
<Compile Include="FlowAnalysis\FlowTestBase.cs" />
Expand Down
800 changes: 800 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Diagnostics/IInvocationExpressionTests.cs
View file
Open in desktop

Large diffs are not rendered by default.

13 changes: 6 additions & 7 deletions src/Compilers/CSharp/Test/Semantic/Diagnostics/OperationAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,7 @@ public void M6()
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(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)
Diagnostic(InvocationTestAnalyzer.BigParamArrayArgumentsDescriptor.Id, "M5(b: new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11})").WithLocation(47, 9)
);
}

Expand Down Expand Up @@ -1614,7 +1609,11 @@ public void M1()
.VerifyAnalyzerDiagnostics(new DiagnosticAnalyzer[] { new NullOperationSyntaxTestAnalyzer() }, null, null, false,
Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "M0()").WithLocation(10, 9),
Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "1").WithLocation(11, 12),
Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "1").WithLocation(12, 12));
Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "1").WithLocation(12, 12),
Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "new int[] { }").WithLocation(13, 12),
Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "new int[] { 1 }").WithLocation(14, 12),
Diagnostic(NullOperationSyntaxTestAnalyzer.ParamsArrayOperationDescriptor.Id, "new int[] { 1, 2}").WithLocation(15, 12)
);
}

[WorkItem(9113, "https://github.com/dotnet/roslyn/issues/9113")]
Expand Down
20 changes: 3 additions & 17 deletions src/Compilers/Core/CodeAnalysisTest/Diagnostics/OperationTestAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,14 +486,6 @@ public class InvocationTestAnalyzer : DiagnosticAnalyzer
DiagnosticSeverity.Warning,
isEnabledByDefault: true);

public static readonly DiagnosticDescriptor UseDefaultArgumentDescriptor = new DiagnosticDescriptor(
"UseDefaultArgument",
"Use default argument",
"Invocation uses default argument {0}",
ReliabilityCategory,
DiagnosticSeverity.Warning,
isEnabledByDefault: true);

public static readonly DiagnosticDescriptor InvalidArgumentDescriptor = new DiagnosticDescriptor(
"InvalidArgument",
"Invalid argument",
Expand All @@ -509,7 +501,6 @@ public sealed override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
{
return ImmutableArray.Create(BigParamArrayArgumentsDescriptor,
OutOfNumericalOrderArgumentsDescriptor,
UseDefaultArgumentDescriptor,
InvalidArgumentDescriptor);
}
}
Expand All @@ -529,14 +520,9 @@ public sealed override void Initialize(AnalysisContext context)
return;
}

if (argument.ArgumentKind == ArgumentKind.DefaultValue)
{
operationContext.ReportDiagnostic(Diagnostic.Create(UseDefaultArgumentDescriptor, invocation.Syntax.GetLocation(), argument.Parameter.Name));
}

TestAscendingArgument(operationContext, argument.Value, ref priorArgumentValue);

if (argument.ArgumentKind == ArgumentKind.ParamArray)
if (argument.Parameter.IsParams)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about intent here. ParameterSymbol.IsParams doesn't guarantee that the method was used in expanded form.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent is to analyzer arrays passed to params/ParamArray parameters in a uniform fashion whether the arguments were originally in array form or in expanded form.

{
IArrayCreationExpression arrayArgument = argument.Value as IArrayCreationExpression;
if (arrayArgument != null)
Expand Down Expand Up @@ -1085,7 +1071,7 @@ public sealed override void Initialize(AnalysisContext context)
{
IInvocationExpression invocation = (IInvocationExpression)operationContext.Operation;

foreach (IArgument argument in invocation.ArgumentsInSourceOrder)
foreach (IArgument argument in invocation.ArgumentsInEvaluationOrder)
{
if (argument.Parameter.IsParams)
{
Expand Down Expand Up @@ -1777,7 +1763,7 @@ public override void Visit(IOperation operation)
}
if (operation.Kind == OperationKind.Argument)
{
if (((IArgument)operation).ArgumentKind == ArgumentKind.ParamArray)
if (((IArgument)operation).Parameter.IsParams)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about intent here. ParameterSymbol.IsParams doesn't guarantee that the method was used in expanded form.

Copy link
Contributor Author

@msJohnHamby msJohnHamby Jun 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For purposes of this test, it doesn't matter whether the arguments were originally in array form or in expanded form.

{
_paramsList.Add(operation);
}
Expand Down
41 changes: 6 additions & 35 deletions src/Compilers/Core/Portable/Compilation/IExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ namespace Microsoft.CodeAnalysis.Semantics
/// </remarks>
public interface IHasArgumentsExpression : IOperation
{
/// <summary>
/// Arguments of the invocation, excluding the instance argument. Arguments are in the order that they will be evaluated,
/// and params/ParamArray arguments have been collected into arrays. Default values are supplied for
/// optional arguments missing in source.
/// </summary>
ImmutableArray<IArgument> ArgumentsInEvaluationOrder { get; }
/// <summary>
/// Arguments of the invocation, excluding the instance argument. Arguments are in parameter order,
/// and params/ParamArray arguments have been collected into arrays. Default values are supplied for
Expand Down Expand Up @@ -45,12 +51,6 @@ public interface IInvocationExpression : IHasArgumentsExpression
/// True if the invocation uses a virtual mechanism, and false otherwise.
/// </summary>
bool IsVirtual { get; }
/// <summary>
/// Arguments of the invocation, excluding the instance argument. Arguments are in the order specified in source,
/// and params/ParamArray arguments have been collected into arrays. Arguments are not present
/// unless supplied in source.
/// </summary>
ImmutableArray<IArgument> ArgumentsInSourceOrder { get; }
}

/// <summary>
Expand All @@ -62,10 +62,6 @@ public interface IInvocationExpression : IHasArgumentsExpression
/// </remarks>
public interface IArgument : IOperation
{
/// <summary>
/// Kind of argument.
/// </summary>
ArgumentKind ArgumentKind { get; }
/// <summary>
/// Parameter the argument matches.
/// </summary>
Expand All @@ -84,31 +80,6 @@ public interface IArgument : IOperation
IOperation OutConversion { get; }
}

/// <summary>
/// Kinds of arguments.
/// </summary>
public enum ArgumentKind
{
None = 0x0,

/// <summary>
/// Argument is specified positionally and matches the parameter of the same ordinality.
/// </summary>
Positional = 0x1,
/// <summary>
/// Argument is specified by name and matches the parameter of the same name.
/// </summary>
Named = 0x2,
/// <summary>
/// Argument becomes an element of an array that matches a trailing C# params or VB ParamArray parameter.
/// </summary>
ParamArray = 0x3,
/// <summary>
/// Argument was omitted in source but has a default value supplied automatically.
/// </summary>
DefaultValue = 0x4
}

/// <summary>
/// Represents a reference to an array element.
/// </summary>
Expand Down
9 changes: 1 addition & 8 deletions src/Compilers/Core/Portable/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,6 @@ Microsoft.CodeAnalysis.OperationKind.YieldBreakStatement = 12 -> Microsoft.CodeA
Microsoft.CodeAnalysis.OperationKind.YieldReturnStatement = 16 -> Microsoft.CodeAnalysis.OperationKind
Microsoft.CodeAnalysis.PortableExecutableReference.GetMetadata() -> Microsoft.CodeAnalysis.Metadata
Microsoft.CodeAnalysis.SemanticModel.GetOperation(Microsoft.CodeAnalysis.SyntaxNode node, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> Microsoft.CodeAnalysis.IOperation
Microsoft.CodeAnalysis.Semantics.ArgumentKind
Microsoft.CodeAnalysis.Semantics.ArgumentKind.DefaultValue = 4 -> Microsoft.CodeAnalysis.Semantics.ArgumentKind
Microsoft.CodeAnalysis.Semantics.ArgumentKind.Named = 2 -> Microsoft.CodeAnalysis.Semantics.ArgumentKind
Microsoft.CodeAnalysis.Semantics.ArgumentKind.None = 0 -> Microsoft.CodeAnalysis.Semantics.ArgumentKind
Microsoft.CodeAnalysis.Semantics.ArgumentKind.ParamArray = 3 -> Microsoft.CodeAnalysis.Semantics.ArgumentKind
Microsoft.CodeAnalysis.Semantics.ArgumentKind.Positional = 1 -> Microsoft.CodeAnalysis.Semantics.ArgumentKind
Microsoft.CodeAnalysis.Semantics.BinaryOperandsKind
Microsoft.CodeAnalysis.Semantics.BinaryOperandsKind.Boolean = 1536 -> Microsoft.CodeAnalysis.Semantics.BinaryOperandsKind
Microsoft.CodeAnalysis.Semantics.BinaryOperandsKind.Decimal = 1280 -> Microsoft.CodeAnalysis.Semantics.BinaryOperandsKind
Expand Down Expand Up @@ -324,7 +318,6 @@ Microsoft.CodeAnalysis.Semantics.ConversionKind.TryCast = 2 -> Microsoft.CodeAna
Microsoft.CodeAnalysis.Semantics.IAddressOfExpression
Microsoft.CodeAnalysis.Semantics.IAddressOfExpression.Reference.get -> Microsoft.CodeAnalysis.IOperation
Microsoft.CodeAnalysis.Semantics.IArgument
Microsoft.CodeAnalysis.Semantics.IArgument.ArgumentKind.get -> Microsoft.CodeAnalysis.Semantics.ArgumentKind
Microsoft.CodeAnalysis.Semantics.IArgument.InConversion.get -> Microsoft.CodeAnalysis.IOperation
Microsoft.CodeAnalysis.Semantics.IArgument.OutConversion.get -> Microsoft.CodeAnalysis.IOperation
Microsoft.CodeAnalysis.Semantics.IArgument.Parameter.get -> Microsoft.CodeAnalysis.IParameterSymbol
Expand Down Expand Up @@ -403,6 +396,7 @@ Microsoft.CodeAnalysis.Semantics.IForLoopStatement.Locals.get -> System.Collecti
Microsoft.CodeAnalysis.Semantics.IForWhileUntilLoopStatement
Microsoft.CodeAnalysis.Semantics.IForWhileUntilLoopStatement.Condition.get -> Microsoft.CodeAnalysis.IOperation
Microsoft.CodeAnalysis.Semantics.IHasArgumentsExpression
Microsoft.CodeAnalysis.Semantics.IHasArgumentsExpression.ArgumentsInEvaluationOrder.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Semantics.IArgument>
Microsoft.CodeAnalysis.Semantics.IHasArgumentsExpression.ArgumentsInParameterOrder.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Semantics.IArgument>
Microsoft.CodeAnalysis.Semantics.IHasArgumentsExpression.GetArgumentMatchingParameter(Microsoft.CodeAnalysis.IParameterSymbol parameter) -> Microsoft.CodeAnalysis.Semantics.IArgument
Microsoft.CodeAnalysis.Semantics.IHasOperatorMethodExpression
Expand All @@ -420,7 +414,6 @@ Microsoft.CodeAnalysis.Semantics.IInstanceReferenceExpression.InstanceReferenceK
Microsoft.CodeAnalysis.Semantics.IInvalidExpression
Microsoft.CodeAnalysis.Semantics.IInvalidStatement
Microsoft.CodeAnalysis.Semantics.IInvocationExpression
Microsoft.CodeAnalysis.Semantics.IInvocationExpression.ArgumentsInSourceOrder.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Semantics.IArgument>
Microsoft.CodeAnalysis.Semantics.IInvocationExpression.Instance.get -> Microsoft.CodeAnalysis.IOperation
Microsoft.CodeAnalysis.Semantics.IInvocationExpression.IsVirtual.get -> bool
Microsoft.CodeAnalysis.Semantics.IInvocationExpression.TargetMethod.get -> Microsoft.CodeAnalysis.IMethodSymbol
Expand Down
Loading