pimhwang
/
SharpDevelop
mirror of https://github.com/icsharpcode/SharpDevelop.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

[CodeActions] Greatly improved OptionalParameterCouldBeSkippedIssue. 

It now handles named parameters much better, has more actions, and less code duplication.
newNRvisualizers
Simon Lindgren 14 years ago
parent
82ab94fa1f
commit
cdfde8bcef
2 changed files with 190 additions and 67 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 137
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/OptionalParameterCouldBeSkippedIssue.cs
  2. 120
      ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/OptionalParameterCouldBeSkippedTests.cs

137
ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/OptionalParameterCouldBeSkippedIssue.cs
Unescape View File

@ -28,6 +28,7 @@ using ICSharpCode.NRefactory.CSharp.Resolver; @@ -28,6 +28,7 @@ using ICSharpCode.NRefactory.CSharp.Resolver;
using System.Linq;
using ICSharpCode.NRefactory.Semantics;
using ICSharpCode.NRefactory.TypeSystem;
using System;
namespace ICSharpCode.NRefactory.CSharp.Refactoring
{
@ -52,50 +53,66 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring @@ -52,50 +53,66 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
public override void VisitObjectCreateExpression(ObjectCreateExpression objectCreateExpression)
{
base.VisitObjectCreateExpression(objectCreateExpression);
var invocationResolveResult = ctx.Resolve(objectCreateExpression) as CSharpInvocationResolveResult;
if (invocationResolveResult == null)
return;
var arguments = objectCreateExpression.Arguments.ToArray();
var redundantArguments = GetRedundantArguments(arguments, invocationResolveResult);
foreach (var argument in redundantArguments) {
string actionMessage = GetActionTitle(objectCreateExpression.Arguments, argument);
var action = new CodeAction(actionMessage, script => {
var newArgumentList = objectCreateExpression.Arguments
.Where(arg => arg.StartLocation < argument.StartLocation)
.Select(arg => arg.Clone());
var newInvocation = new ObjectCreateExpression(objectCreateExpression.Type.Clone(), newArgumentList);
script.Replace(objectCreateExpression, newInvocation);
});
var issueMessage = ctx.TranslateString("Argument is identical to the default value");
AddIssue(argument, issueMessage, new [] { action });
}
CheckMethodCall(objectCreateExpression, objectCreateExpression.Arguments,
(objectCreation, args) => new ObjectCreateExpression(objectCreation.Type.Clone(), args));
}
public override void VisitInvocationExpression(InvocationExpression invocationExpression)
{
base.VisitInvocationExpression(invocationExpression);
CheckMethodCall(invocationExpression, invocationExpression.Arguments,
(invocation, args) => new InvocationExpression(invocation.Target.Clone(), args));
}
var invocationResolveResult = ctx.Resolve(invocationExpression) as CSharpInvocationResolveResult;
void CheckMethodCall<T> (T node, IEnumerable<Expression> arguments, Func<T, IEnumerable<Expression>, T> generateReplacement) where T: AstNode
{
var invocationResolveResult = ctx.Resolve(node) as CSharpInvocationResolveResult;
if (invocationResolveResult == null)
return;
var arguments = invocationExpression.Arguments.ToArray();
string actionMessage = ctx.TranslateString("Remove redundant arguments");
var redundantArguments = GetRedundantArguments(arguments.ToArray(), invocationResolveResult);
var action = new CodeAction(actionMessage, script => {
var newArgumentList = arguments
.Where(arg => !redundantArguments.Contains(arg))
.Select(arg => arg.Clone());
var newInvocation = generateReplacement(node, newArgumentList);
script.Replace(node, newInvocation);
});
var issueMessage = ctx.TranslateString("Argument is identical to the default value");
var lastPositionalArgument = redundantArguments.FirstOrDefault(expression => !(expression is NamedArgumentExpression));
bool hasNamedArguments = false;
var redundantArguments = GetRedundantArguments(arguments, invocationResolveResult);
foreach (var argument in redundantArguments) {
string actionMessage = GetActionTitle(invocationExpression.Arguments, argument);
var action = new CodeAction(actionMessage, script => {
var newArgumentList = invocationExpression.Arguments
.Where(arg => arg.StartLocation < argument.StartLocation)
var localArgument = argument;
var actions = new List<CodeAction>();
actions.Add(action);
if (localArgument is NamedArgumentExpression || localArgument == lastPositionalArgument) {
var title = ctx.TranslateString("Remove this argument");
actions.Add(new CodeAction(title, script => {
var newArgumentList = arguments
.Where(arg => arg != localArgument)
.Select(arg => arg.Clone());
var newInvocation = new InvocationExpression(invocationExpression.Target.Clone(), newArgumentList);
script.Replace(invocationExpression, newInvocation);
});
var issueMessage = ctx.TranslateString("Argument is identical to the default value");
AddIssue(argument, issueMessage, new [] { action });
var newInvocation = generateReplacement(node, newArgumentList);
script.Replace(node, newInvocation);
}));
hasNamedArguments = true;
} else {
var title = ctx.TranslateString("Remove this and the following positional arguments");
actions.Add(new CodeAction(title, script => {
var newArgumentList = arguments
.Where(arg => arg.StartLocation < localArgument.StartLocation && !(arg is NamedArgumentExpression))
.Select(arg => arg.Clone());
var newInvocation = generateReplacement(node, newArgumentList);
script.Replace(node, newInvocation);
}));
}
AddIssue(localArgument, issueMessage, actions);
}
}
@ -110,36 +127,34 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring @@ -110,36 +127,34 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
// This particular parameter is an error, but keep trying the other ones
continue;
var parameter = resolvedParameters[parameterIndex];
if (parameter.IsParams)
// before params all optional arguments are needed, otherwise some of the
// param arguments will be shifted out of the params into the fixed parameters
break;
if (!parameter.IsOptional)
// There can be no optional parameters preceding a required one
var argument = arguments[i];
if (argument is PrimitiveExpression) {
if (parameter.IsParams)
// before positional params arguments all optional arguments are needed, otherwise some of the
// param arguments will be shifted out of the params into the fixed parameters
break;
if (!parameter.IsOptional)
// There can be no optional parameters preceding a required one
break;
var argumentResolveResult = ctx.Resolve(argument) as ConstantResolveResult;
if (argumentResolveResult == null || parameter.ConstantValue != argumentResolveResult.ConstantValue)
// Stop here since any arguments before this one has to be there
// to enable the passing of this argument
break;
yield return argument;
} else if (argument is NamedArgumentExpression) {
var expression = ((NamedArgumentExpression)argument).Expression as PrimitiveExpression;
if (expression == null)
continue;
var expressionResolveResult = ctx.Resolve(expression) as ConstantResolveResult;
if (expressionResolveResult == null || parameter.ConstantValue != expressionResolveResult.ConstantValue)
// continue, since there can still be more arguments that are redundant
continue;
yield return argument;
} else {
// This is a non-constant positional argument => no more redundancies are possible
break;
var argument = arguments[i] as PrimitiveExpression;
if (argument == null)
continue;
var argumentResolveResult = ctx.Resolve(argument) as ConstantResolveResult;
if (parameter.ConstantValue != argumentResolveResult.ConstantValue)
// Stop here since any arguments before this one has to be there
// to enable the passing of this argument
break;
yield return argument;
}
}
string GetActionTitle(IEnumerable<Expression> arguments, Expression firstArgumentToRemove)
{
var redundantArgumentCount = arguments
.Where(arg => arg.StartLocation >= firstArgumentToRemove.StartLocation)
.Count();
if (redundantArgumentCount == 1) {
return ctx.TranslateString("Remove this argument");
}
else {
return ctx.TranslateString("Remove this and the following arguments");
}
}
}
}

120
ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/OptionalParameterCouldBeSkippedTests.cs
Unescape View File

@ -52,7 +52,7 @@ class TestClass @@ -52,7 +52,7 @@ class TestClass
var issues = GetIssues(new OptionalParameterCouldBeSkippedIssue(), input, out context);
Assert.AreEqual(1, issues.Count);
var issue = issues [0];
Assert.AreEqual(1, issue.Actions.Count);
Assert.AreEqual(2, issue.Actions.Count);
CheckFix(context, issues [0], @"
class TestClass
@ -87,7 +87,7 @@ class TestClass @@ -87,7 +87,7 @@ class TestClass
var issues = GetIssues(new OptionalParameterCouldBeSkippedIssue(), input, out context);
Assert.AreEqual(1, issues.Count);
var issue = issues [0];
Assert.AreEqual(1, issue.Actions.Count);
Assert.AreEqual(2, issue.Actions.Count);
CheckFix(context, issues [0], @"
class TestClass
@ -122,7 +122,7 @@ class TestClass @@ -122,7 +122,7 @@ class TestClass
var issues = GetIssues(new OptionalParameterCouldBeSkippedIssue(), input, out context);
Assert.AreEqual(1, issues.Count);
var issue = issues [0];
Assert.AreEqual(1, issue.Actions.Count);
Assert.AreEqual(2, issue.Actions.Count);
CheckFix(context, issues [0], @"
class TestClass
@ -156,8 +156,9 @@ class TestClass @@ -156,8 +156,9 @@ class TestClass
TestRefactoringContext context;
var issues = GetIssues(new OptionalParameterCouldBeSkippedIssue(), input, out context);
Assert.AreEqual(3, issues.Count);
var issue = issues [0];
Assert.AreEqual(1, issue.Actions.Count);
Assert.AreEqual(2, issues[0].Actions.Count);
Assert.AreEqual(2, issues[1].Actions.Count);
Assert.AreEqual(2, issues[2].Actions.Count);
CheckFix(context, issues [2], @"
class TestClass
@ -172,7 +173,7 @@ class TestClass @@ -172,7 +173,7 @@ class TestClass
}
}");
}
[Test]
public void IgnoresIfParamsAreUsed()
{
@ -192,6 +193,113 @@ class TestClass @@ -192,6 +193,113 @@ class TestClass
var issues = GetIssues(new OptionalParameterCouldBeSkippedIssue(), input, out context);
Assert.AreEqual(0, issues.Count);
}
[Test]
public void NamedArgument()
{
var input = @"
class TestClass
{
void Foo(string a1 = ""a1"", string a2 = ""a2"")
{
}
void Bar()
{
Foo (a2: ""a2"");
}
}";
TestRefactoringContext context;
var issues = GetIssues(new OptionalParameterCouldBeSkippedIssue(), input, out context);
Assert.AreEqual(1, issues.Count);
Assert.AreEqual(2, issues[0].Actions.Count);
CheckFix(context, issues [0], @"
class TestClass
{
void Foo(string a1 = ""a1"", string a2 = ""a2"")
{
}
void Bar()
{
Foo ();
}
}");
}
[Test]
public void DoesNotStopAtDifferingNamedParameters()
{
var input = @"
class TestClass
{
void Foo(string a1 = ""a1"", string a2 = ""a2"", string a3 = ""a3"")
{
}
void Bar()
{
Foo (""a1"", ""a2"", a3: ""non-default"");
}
}";
TestRefactoringContext context;
var issues = GetIssues(new OptionalParameterCouldBeSkippedIssue(), input, out context);
Assert.AreEqual(2, issues.Count);
Assert.AreEqual(2, issues[0].Actions.Count);
Assert.AreEqual(2, issues[1].Actions.Count);
CheckFix(context, issues [1], @"
class TestClass
{
void Foo(string a1 = ""a1"", string a2 = ""a2"", string a3 = ""a3"")
{
}
void Bar()
{
Foo (a3: ""non-default"");
}
}");
}
[Test]
public void DoesNotStopAtNamedParamsArray()
{
var input = @"
class TestClass
{
void Foo(string a1 = ""a1"", string a2 = ""a2"", params string[] extras)
{
}
void Bar()
{
Foo (""a1"", ""a2"", extras: new [] { ""extra1"" });
}
}";
TestRefactoringContext context;
var issues = GetIssues(new OptionalParameterCouldBeSkippedIssue(), input, out context);
Assert.AreEqual(2, issues.Count);
Assert.AreEqual(2, issues[0].Actions.Count);
Assert.AreEqual(2, issues[1].Actions.Count);
// TODO: Fix formatting
CheckFix(context, issues [1], @"
class TestClass
{
void Foo(string a1 = ""a1"", string a2 = ""a2"", params string[] extras)
{
}
void Bar()
{
Foo (extras: new[] {
""extra1""
});
}
}");
}
}
}

Write Preview
Loading…
Cancel
Save