From 7f077aa331629f8c5763b3a8f75f541dd379c0ee Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Thu, 19 Oct 2017 15:39:34 -0700 Subject: [PATCH 1/3] Do not suggest using implicit type for stackalloc initialization --- .../UseImplicitTypeTests.cs | 81 +++++++++++++++++++ ...CSharpUseImplicitTypeDiagnosticAnalyzer.cs | 15 +++- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs index c6697d411e062..e90543b099ddb 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs @@ -1749,5 +1749,86 @@ static void M() }", options: ImplicitTypeEverywhere()); } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] + public async Task DoNotSuggestVarOnStackAllocExpressions_SpanType() + { + await TestMissingInRegularAndScriptAsync( +@"using System; +namespace System +{ + public readonly ref struct Span + { + unsafe public Span(void* pointer, int length) { } + } +} +class C +{ + static void M() + { + [|Span|] x = stackalloc int [10]; + } +}", new TestParameters(options: ImplicitTypeEverywhere())); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] + public async Task DoNotSuggestVarOnStackAllocExpressions_SpanType_NestedConditional() + { + await TestMissingInRegularAndScriptAsync( +@"using System; +namespace System +{ + public readonly ref struct Span + { + unsafe public Span(void* pointer, int length) { } + } +} +class C +{ + static void M(bool choice) + { + [|Span|] x = choice ? stackalloc int [10] : stackalloc int [100]; + } +}", new TestParameters(options: ImplicitTypeEverywhere())); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] + public async Task DoNotSuggestVarOnStackAllocExpressions_SpanType_NestedCast() + { + await TestMissingInRegularAndScriptAsync( +@"using System; +namespace System +{ + public readonly ref struct Span + { + unsafe public Span(void* pointer, int length) { } + } +} +class C +{ + static void M() + { + [|Span|] x = (Span)stackalloc int [100]; + } +}", new TestParameters(options: ImplicitTypeEverywhere())); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] + public async Task DoNotSuggestVarOnStackAllocExpressions_PointerType() + { + await TestMissingInRegularAndScriptAsync( +@"using System; +class C +{ + unsafe static void M() + { + [|int*|] x = stackalloc int [10]; + } +}", new TestParameters(options: ImplicitTypeEverywhere())); + } } } diff --git a/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs index fa207c7230ac4..d3fc4068235e1 100644 --- a/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs @@ -107,9 +107,18 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman } var variable = variableDeclaration.Variables.Single(); - if (AssignmentSupportsStylePreference( - variable.Identifier, typeName, variable.Initializer.Value, - semanticModel, optionSet, cancellationToken)) + var initializer = variable.Initializer.Value; + + // Discourage using "var pointer = stackalloc", after introducing C# 7.2, as it might be confusing to users expecting a Span<> + // Check descendant nodes as well, because Span-creating stackallocs can be nested in other nodes (like conditional operators and casts). + // https://github.com/dotnet/roslyn/issues/22768 + if (initializer.DescendantNodesAndSelf().Any(node => node.IsKind(SyntaxKind.StackAllocArrayCreationExpression))) + { + issueSpan = default; + return false; + } + + if (AssignmentSupportsStylePreference(variable.Identifier, typeName, initializer, semanticModel, optionSet, cancellationToken)) { issueSpan = candidateIssueSpan; return true; From fef6618d6ac95d80a7570412801dcfc34f59024c Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Mon, 23 Oct 2017 16:25:28 -0700 Subject: [PATCH 2/3] PR comments --- .../UseImplicitTypeTests.cs | 150 ++++++++++++++++-- ...CSharpUseImplicitTypeDiagnosticAnalyzer.cs | 14 +- 2 files changed, 149 insertions(+), 15 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs index e90543b099ddb..8cb58e2fc3400 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseImplicitTypeTests.cs @@ -1754,8 +1754,8 @@ static void M() [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] public async Task DoNotSuggestVarOnStackAllocExpressions_SpanType() { - await TestMissingInRegularAndScriptAsync( -@"using System; + await TestMissingInRegularAndScriptAsync(@" +using System; namespace System { public readonly ref struct Span @@ -1776,8 +1776,8 @@ static void M() [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] public async Task DoNotSuggestVarOnStackAllocExpressions_SpanType_NestedConditional() { - await TestMissingInRegularAndScriptAsync( -@"using System; + await TestMissingInRegularAndScriptAsync(@" +using System; namespace System { public readonly ref struct Span @@ -1798,8 +1798,8 @@ static void M(bool choice) [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] public async Task DoNotSuggestVarOnStackAllocExpressions_SpanType_NestedCast() { - await TestMissingInRegularAndScriptAsync( -@"using System; + await TestMissingInRegularAndScriptAsync(@" +using System; namespace System { public readonly ref struct Span @@ -1818,17 +1818,147 @@ static void M() [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] - public async Task DoNotSuggestVarOnStackAllocExpressions_PointerType() + public async Task SuggestVarOnLambdasWithNestedStackAllocs() { - await TestMissingInRegularAndScriptAsync( -@"using System; + await TestInRegularAndScriptAsync(@" +using System.Linq; +class C +{ + unsafe static void M() + { + [|int|] x = new int[] { 1, 2, 3 }.First(i => + { + int* y = stackalloc int[10]; + return i == 1; + }); + } +}", @" +using System.Linq; +class C +{ + unsafe static void M() + { + var x = new int[] { 1, 2, 3 }.First(i => + { + int* y = stackalloc int[10]; + return i == 1; + }); + } +}", options: ImplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] + public async Task SuggestVarOnAnonymousMethodsWithNestedStackAllocs() + { + await TestInRegularAndScriptAsync(@" +using System.Linq; +class C +{ + unsafe static void M() + { + [|int|] x = new int[] { 1, 2, 3 }.First(delegate (int i) + { + int* y = stackalloc int[10]; + return i == 1; + }); + } +}", @" +using System.Linq; +class C +{ + unsafe static void M() + { + var x = new int[] { 1, 2, 3 }.First(delegate (int i) + { + int* y = stackalloc int[10]; + return i == 1; + }); + } +}", options: ImplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] + public async Task SuggestVarOnStackAllocsNestedInLambdas() + { + await TestInRegularAndScriptAsync(@" +using System.Linq; +class C +{ + unsafe static void M() + { + var x = new int[] { 1, 2, 3 }.First(i => + { + [|int*|] y = stackalloc int[10]; + return i == 1; + }); + } +}", @" +using System.Linq; +class C +{ + unsafe static void M() + { + var x = new int[] { 1, 2, 3 }.First(i => + { + var y = stackalloc int[10]; + return i == 1; + }); + } +}", options: ImplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] + public async Task SuggestVarOnStackAllocsNestedInAnonymousMethods() + { + await TestInRegularAndScriptAsync(@" +using System.Linq; +class C +{ + unsafe static void M() + { + var x = new int[] { 1, 2, 3 }.First(delegate (int i) + { + [|int*|] y = stackalloc int[10]; + return i == 1; + }); + } +}", @" +using System.Linq; +class C +{ + unsafe static void M() + { + var x = new int[] { 1, 2, 3 }.First(delegate (int i) + { + var y = stackalloc int[10]; + return i == 1; + }); + } +}", options: ImplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(22768, "https://github.com/dotnet/roslyn/issues/22768")] + public async Task SuggestVarOnStackAllocsInOuterMethodScope() + { + await TestInRegularAndScriptAsync(@" class C { unsafe static void M() { [|int*|] x = stackalloc int [10]; } -}", new TestParameters(options: ImplicitTypeEverywhere())); +}", @" +class C +{ + unsafe static void M() + { + var x = stackalloc int [10]; + } +}", options: ImplicitTypeEverywhere()); } } } diff --git a/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs index d3fc4068235e1..5f74fb5b89d2e 100644 --- a/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs @@ -109,16 +109,20 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman var variable = variableDeclaration.Variables.Single(); var initializer = variable.Initializer.Value; - // Discourage using "var pointer = stackalloc", after introducing C# 7.2, as it might be confusing to users expecting a Span<> - // Check descendant nodes as well, because Span-creating stackallocs can be nested in other nodes (like conditional operators and casts). - // https://github.com/dotnet/roslyn/issues/22768 - if (initializer.DescendantNodesAndSelf().Any(node => node.IsKind(SyntaxKind.StackAllocArrayCreationExpression))) + // Do not suggest var replacement for stackalloc span expressions. + // This will change the bound type from a span to a pointer. + if (!variableDeclaration.Type.IsKind(SyntaxKind.PointerType) + && initializer + .DescendantNodesAndSelf(descendIntoChildren: node => !node.IsAnyLambdaOrAnonymousMethod()) + .Any(node => node.IsKind(SyntaxKind.StackAllocArrayCreationExpression))) { issueSpan = default; return false; } - if (AssignmentSupportsStylePreference(variable.Identifier, typeName, initializer, semanticModel, optionSet, cancellationToken)) + if (AssignmentSupportsStylePreference( + variable.Identifier, typeName, initializer, + semanticModel, optionSet, cancellationToken)) { issueSpan = candidateIssueSpan; return true; From e7e9fa4088fe4785a3c6814b5492ebfbbd0b65a7 Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Tue, 24 Oct 2017 08:09:16 -0700 Subject: [PATCH 3/3] style comments --- .../CSharpUseImplicitTypeDiagnosticAnalyzer.cs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs index 5f74fb5b89d2e..169c36a64e3f6 100644 --- a/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseImplicitTypeDiagnosticAnalyzer.cs @@ -111,13 +111,17 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman // Do not suggest var replacement for stackalloc span expressions. // This will change the bound type from a span to a pointer. - if (!variableDeclaration.Type.IsKind(SyntaxKind.PointerType) - && initializer - .DescendantNodesAndSelf(descendIntoChildren: node => !node.IsAnyLambdaOrAnonymousMethod()) - .Any(node => node.IsKind(SyntaxKind.StackAllocArrayCreationExpression))) + if (!variableDeclaration.Type.IsKind(SyntaxKind.PointerType)) { - issueSpan = default; - return false; + var containsStackAlloc = initializer + .DescendantNodesAndSelf(descendIntoChildren: node => !node.IsAnyLambdaOrAnonymousMethod()) + .Any(node => node.IsKind(SyntaxKind.StackAllocArrayCreationExpression)); + + if (containsStackAlloc) + { + issueSpan = default; + return false; + } } if (AssignmentSupportsStylePreference(