From 287c94ab783f68ac236f4dc62df8b58c61bd0ead Mon Sep 17 00:00:00 2001 From: Balaji Krishnan Date: Mon, 27 Apr 2015 20:39:54 -0700 Subject: [PATCH] Introduce Constants should not be offered for Nulls Don't offer Introduce Constants on NullLiterals. They could cause semantic errors when used in a replace all occurrences context and are offer no value anyway. --- .../IntroduceVariableTests.cs | 25 ++++++++++++++++--- .../CSharpIntroduceVariableService.cs | 11 ++++++++ .../AbstractIntroduceVariableService.State.cs | 3 --- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/IntroduceVariable/IntroduceVariableTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/IntroduceVariable/IntroduceVariableTests.cs index 6c03cffabd9..517d4b86352 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/IntroduceVariable/IntroduceVariableTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/IntroduceVariable/IntroduceVariableTests.cs @@ -613,13 +613,12 @@ public void TestMissingOnVariableWrite() } [WorkItem(544577)] + [WorkItem(909152)] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)] public void TestExpressionTLambda() { - Test( -@"using System ; using System . Linq . Expressions ; class Program { static Expression < Func < int ? , char ? > > e1 = c => [|null|] ; } ", -@"using System ; using System . Linq . Expressions ; class Program { private const char ? {|Rename:P|} = null ; static Expression < Func < int ? , char ? > > e1 = c => P ; } ", -index: 1); + TestMissing( +@"using System ; using System . Linq . Expressions ; class Program { static Expression < Func < int ? , char ? > > e1 = c => [|null|] ; } "); } [WorkItem(544915)] @@ -2323,5 +2322,23 @@ class TestClass Test(code, expected, index: 2, compareTokens: false); } + + [WorkItem(909152)] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)] + public void TestMissingOnNullLiteral() + { + TestMissing( +@"class C1 { } +class C2 { } +class Test +{ + void M() + { + C1 c1 = [|null|]; + C2 c2 = null; + } +} +"); + } } } diff --git a/src/Features/CSharp/IntroduceVariable/CSharpIntroduceVariableService.cs b/src/Features/CSharp/IntroduceVariable/CSharpIntroduceVariableService.cs index 404a00d6afe..0777fc4ca5f 100644 --- a/src/Features/CSharp/IntroduceVariable/CSharpIntroduceVariableService.cs +++ b/src/Features/CSharp/IntroduceVariable/CSharpIntroduceVariableService.cs @@ -96,13 +96,24 @@ protected override bool IsInAttributeArgumentInitializer(ExpressionSyntax expres return false; } + /// + /// Checks for conditions where we should not generate a variable for an expression + /// protected override bool CanIntroduceVariableFor(ExpressionSyntax expression) { + // (a) If that's the only expression in a statement. + // Otherwise we'll end up with something like "v;" which is not legal in C#. if (expression.WalkUpParentheses().IsParentKind(SyntaxKind.ExpressionStatement)) { return false; } + // (b) For Null Literals, as AllOccurences could introduce semantic errors. + if (expression is LiteralExpressionSyntax && ((LiteralExpressionSyntax)expression).IsKind(SyntaxKind.NullLiteralExpression)) + { + return false; + } + return true; } diff --git a/src/Features/Core/IntroduceVariable/AbstractIntroduceVariableService.State.cs b/src/Features/Core/IntroduceVariable/AbstractIntroduceVariableService.State.cs index 7b2f890a4a2..24eb9f84897 100644 --- a/src/Features/Core/IntroduceVariable/AbstractIntroduceVariableService.State.cs +++ b/src/Features/Core/IntroduceVariable/AbstractIntroduceVariableService.State.cs @@ -216,9 +216,6 @@ private TExpressionSyntax GetExpressionUnderSpan(SyntaxTree tree, TextSpan textS private bool CanIntroduceVariable( CancellationToken cancellationToken) { - // Don't generate a variable for an expression that's the only expression in a - // statement. Otherwise we'll end up with something like "v;" which is not - // legal in C#. if (!_service.CanIntroduceVariableFor(this.Expression)) { return false; -- GitLab