diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj
index 4ee3cda294..b1a32abb74 100644
--- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj
+++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj
@@ -396,13 +396,14 @@
-
+
+
@@ -417,6 +418,7 @@
+
diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringHelper.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringHelper.cs
new file mode 100644
index 0000000000..9531dd1243
--- /dev/null
+++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringHelper.cs
@@ -0,0 +1,70 @@
+//
+// FormatStringHelper.cs
+//
+// Author:
+// Simon Lindgren
+//
+// Copyright (c) 2012 Simon Lindgren
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+using System;
+using System.Collections.Generic;
+using ICSharpCode.NRefactory.CSharp.Resolver;
+using System.Linq;
+using ICSharpCode.NRefactory.TypeSystem;
+
+namespace ICSharpCode.NRefactory.CSharp
+{
+ class FormatStringHelper
+ {
+ static string[] parameterNames = { "format", "frmt", "fmt" };
+
+ public static bool TryGetFormattingParameters(CSharpInvocationResolveResult invocationResolveResult, InvocationExpression invocationExpression,
+ out Expression formatArgument, out TextLocation formatStart, out IList arguments,
+ Func argumentFilter)
+ {
+ if (argumentFilter == null)
+ argumentFilter = (p, e) => true;
+
+ formatArgument = null;
+ formatStart = default(TextLocation);
+ arguments = new List();
+ var argumentToParameterMap = invocationResolveResult.GetArgumentToParameterMap();
+ var resolvedParameters = invocationResolveResult.Member.Parameters;
+ var allArguments = invocationExpression.Arguments.ToArray();
+ for (int i = 0; i < allArguments.Length; i++) {
+ var parameterIndex = argumentToParameterMap[i];
+ if (parameterIndex < 0 || parameterIndex >= resolvedParameters.Count) {
+ // No valid mapping for this parameter, skip it
+ continue;
+ }
+ var parameter = resolvedParameters[parameterIndex];
+ var argument = allArguments[i];
+ if (parameter.Type.IsKnownType(KnownTypeCode.String) && parameterNames.Contains(parameter.Name)) {
+ formatArgument = argument;
+ formatStart = argument.StartLocation;
+ } else if ((formatArgument != null || parameter.IsParams) && argumentFilter(parameter, argument)) {
+ arguments.Add(argument);
+ }
+ }
+ return formatArgument != null;
+ }
+ }
+}
+
diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs
similarity index 73%
rename from ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssue.cs
rename to ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs
index c8afd65009..ae7778b53f 100644
--- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssue.cs
+++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs
@@ -25,7 +25,6 @@
// THE SOFTWARE.
using System;
using System.Collections.Generic;
-using ICSharpCode.NRefactory.TypeSystem;
using ICSharpCode.NRefactory.CSharp.Resolver;
using System.Linq;
using ICSharpCode.NRefactory.Utils;
@@ -58,11 +57,17 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
var invocationResolveResult = context.Resolve(invocationExpression) as CSharpInvocationResolveResult;
if (invocationResolveResult == null)
return;
- string format;
+ Expression formatArgument;
IList formatArguments;
TextLocation formatStart;
- if (!TryGetFormattingParameters(invocationResolveResult, invocationExpression, out format, out formatStart, out formatArguments))
+ if (!FormatStringHelper.TryGetFormattingParameters(invocationResolveResult, invocationExpression,
+ out formatArgument, out formatStart, out formatArguments, null)) {
return;
+ }
+ var primitiveArgument = formatArgument as PrimitiveExpression;
+ if (primitiveArgument == null || !(primitiveArgument.Value is String))
+ return;
+ var format = (string)primitiveArgument.Value;
var parsingResult = context.ParseFormatString(format);
CheckSegments(parsingResult.Segments, formatStart, formatArguments, invocationExpression);
}
@@ -99,35 +104,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
}
}
}
-
- static string[] parameterNames = { "format", "frmt", "fmt" };
-
- bool TryGetFormattingParameters(CSharpInvocationResolveResult invocationResolveResult, InvocationExpression invocationExpression,
- out string format, out TextLocation formatStart, out IList arguments)
- {
- format = null;
- formatStart = default(TextLocation);
- arguments = new List();
- var argumentToParameterMap = invocationResolveResult.GetArgumentToParameterMap();
- var resolvedParameters = invocationResolveResult.Member.Parameters;
- var allArguments = invocationExpression.Arguments.ToArray();
- for (int i = 0; i < allArguments.Length; i++) {
- var parameterIndex = argumentToParameterMap[i];
- if (parameterIndex < 0 || parameterIndex >= resolvedParameters.Count) {
- // No valid mapping for this parameter, skip it
- continue;
- }
- var parameter = resolvedParameters[parameterIndex];
- var argument = allArguments[i];
- if (parameter.Type.IsKnownType(KnownTypeCode.String) && parameterNames.Contains(parameter.Name) && argument is PrimitiveExpression) {
- format = (string)((PrimitiveExpression)argument).Value;
- formatStart = argument.StartLocation;
- } else if (format != null || parameter.IsParams) {
- arguments.Add(argument);
- }
- }
- return format != null;
- }
}
}
}
diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs
index 202a304c2e..ee0192f78c 100644
--- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs
+++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs
@@ -60,6 +60,29 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
}
HashSet processedNodes = new HashSet();
+
+ void CheckExpressionInAutoCallContext(Expression expression)
+ {
+ if (expression is InvocationExpression) {
+ CheckInvocationInAutoCallContext((InvocationExpression)expression);
+ }
+ }
+
+ void CheckInvocationInAutoCallContext(InvocationExpression invocationExpression)
+ {
+ var memberExpression = invocationExpression.Target as MemberReferenceExpression;
+ if (memberExpression == null) {
+ return;
+ }
+ var resolveResult = ctx.Resolve(invocationExpression) as CSharpInvocationResolveResult;
+ if (resolveResult == null) {
+ return;
+ }
+ if (resolveResult.Member.Name != "ToString") {
+ return;
+ }
+ AddRedundantToStringIssue(memberExpression, invocationExpression);
+ }
void AddRedundantToStringIssue(MemberReferenceExpression memberExpression, InvocationExpression invocationExpression)
{
@@ -125,48 +148,41 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
expressions.Add(expression);
}
}
-
- void CheckExpressionInAutoCallContext(Expression expression)
- {
- if (expression is InvocationExpression) {
- CheckInvocationInAutoCallContext((InvocationExpression)expression);
- }
- }
-
- void CheckInvocationInAutoCallContext(InvocationExpression invocationExpression)
- {
- var memberExpression = invocationExpression.Target as MemberReferenceExpression;
- if (memberExpression == null) {
- return;
- }
- var resolveResult = ctx.Resolve(invocationExpression) as CSharpInvocationResolveResult;
- if (resolveResult == null) {
- return;
- }
- if (resolveResult.Member.Name != "ToString") {
- return;
- }
- AddRedundantToStringIssue(memberExpression, invocationExpression);
- }
#endregion
- #region Invocation expressions
+ #region Invocation expression
public override void VisitInvocationExpression(InvocationExpression invocationExpression)
{
base.VisitInvocationExpression(invocationExpression);
- var memberExpression = invocationExpression.Target as MemberReferenceExpression;
- if (memberExpression == null) {
- return;
- }
- var targetResolveResult = ctx.Resolve(invocationExpression) as CSharpInvocationResolveResult;
- if (targetResolveResult == null) {
+ var invocationResolveResult = ctx.Resolve(invocationExpression) as CSharpInvocationResolveResult;
+ if (invocationResolveResult == null) {
return;
}
- if (targetResolveResult.TargetResult.Type.IsKnownType(KnownTypeCode.String) && targetResolveResult.Member.Name == "ToString") {
- AddRedundantToStringIssue(memberExpression, invocationExpression);
+ IMember member = invocationResolveResult.Member;
+
+ // "".ToString()
+ CheckTargetedObject(invocationExpression, invocationResolveResult.TargetResult.Type, member);
+
+ // Check list of members that call ToString() automatically
+ CheckAutomaticToStringCallers(invocationExpression, member);
+
+ // Check formatting calls
+ CheckFormattingCall(invocationExpression, invocationResolveResult);
+ }
+
+ void CheckTargetedObject(InvocationExpression invocationExpression, IType type, IMember member)
+ {
+ var memberExpression = invocationExpression.Target as MemberReferenceExpression;
+ if (memberExpression != null) {
+ if (type.IsKnownType(KnownTypeCode.String) && member.Name == "ToString") {
+ AddRedundantToStringIssue(memberExpression, invocationExpression);
+ }
}
- IMember member = targetResolveResult.Member;
+ }
+
+ void CheckAutomaticToStringCallers(InvocationExpression invocationExpression, IMember member)
+ {
if (member.IsOverride) {
member = InheritanceHelper.GetBaseMember(member);
if (member == null) {
@@ -175,12 +191,28 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
}
var key = new Tuple(member.ReflectionName, invocationExpression.Arguments.Count);
Tuple checkInfo;
- if (!membersCallingToString.TryGetValue(key, out checkInfo)) {
- return;
+ if (membersCallingToString.TryGetValue(key, out checkInfo)) {
+ var arguments = invocationExpression.Arguments.ToList();
+ for (int i = checkInfo.Item1; i < Math.Min(invocationExpression.Arguments.Count, checkInfo.Item2 + 1); ++i) {
+ CheckExpressionInAutoCallContext(arguments[i]);
+ }
}
- var arguments = invocationExpression.Arguments.ToList();
- for (int i = checkInfo.Item1; i < Math.Min(invocationExpression.Arguments.Count, checkInfo.Item2 + 1); ++i) {
- CheckExpressionInAutoCallContext(arguments[i]);
+ }
+
+ void CheckFormattingCall(InvocationExpression invocationExpression, CSharpInvocationResolveResult invocationResolveResult)
+ {
+ Expression formatArgument;
+ IList formatArguments;
+ TextLocation formatStart;
+ // Only check parameters that are of type object: String means it is neccessary, others
+ // means that there is another problem (ie no matching overload of the method).
+ Func predicate =
+ (parameter, argument) => parameter.Type.GetDefinition().IsKnownType(KnownTypeCode.Object);
+ if (FormatStringHelper.TryGetFormattingParameters(invocationResolveResult, invocationExpression,
+ out formatArgument, out formatStart, out formatArguments, predicate)) {
+ foreach (var argument in formatArguments) {
+ CheckExpressionInAutoCallContext(argument);
+ }
}
}
#endregion
diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/FormatStringTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/FormatStringIssues/FormatStringTests.cs
similarity index 100%
rename from ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/FormatStringTests.cs
rename to ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/FormatStringIssues/FormatStringTests.cs
diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantToStringTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantToStringTests.cs
index 7e346ce789..bbe384a340 100644
--- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantToStringTests.cs
+++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantToStringTests.cs
@@ -103,6 +103,91 @@ class Foo
}");
}
+ [Test]
+ public void FormatStringTests ()
+ {
+ var input = @"
+class Foo
+{
+ void Bar (int i)
+ {
+ string s = string.Format(""{0}"", i.ToString());
+ }
+}";
+
+ TestRefactoringContext context;
+ var issues = GetIssues (new RedundantToStringIssue (), input, out context);
+ Assert.AreEqual (1, issues.Count);
+ CheckFix (context, issues, @"
+class Foo
+{
+ void Bar (int i)
+ {
+ string s = string.Format(""{0}"", i);
+ }
+}");
+ }
+
+ [Test]
+ public void HandlesNonLiteralFormatParameter ()
+ {
+ var input = @"
+class Foo
+{
+ void Bar (int i)
+ {
+ string format = ""{0}"";
+ string s = string.Format(format, i.ToString());
+ }
+}";
+
+ TestRefactoringContext context;
+ var issues = GetIssues (new RedundantToStringIssue (), input, out context);
+ Assert.AreEqual (1, issues.Count);
+ CheckFix (context, issues, @"
+class Foo
+{
+ void Bar (int i)
+ {
+ string format = ""{0}"";
+ string s = string.Format(format, i);
+ }
+}");
+ }
+
+ [Test]
+ public void FormatStringWithNonObjectParameterTests ()
+ {
+ var input = @"
+class Foo
+{
+ void Bar (int i)
+ {
+ string s = FakeFormat(""{0} {1}"", i.ToString(), i.ToString());
+ }
+
+ void FakeFormat(string format, string arg0, object arg1)
+ {
+ }
+}";
+
+ TestRefactoringContext context;
+ var issues = GetIssues (new RedundantToStringIssue (), input, out context);
+ Assert.AreEqual (1, issues.Count);
+ CheckFix (context, issues, @"
+class Foo
+{
+ void Bar (int i)
+ {
+ string s = FakeFormat(""{0} {1}"", i.ToString(), i);
+ }
+
+ void FakeFormat(string format, string arg0, object arg1)
+ {
+ }
+}");
+ }
+
[Test]
public void DetectsBlacklistedCalls ()
{
diff --git a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj
index 828ae243f3..4f0cfee082 100644
--- a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj
+++ b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj
@@ -291,12 +291,12 @@
-
+
@@ -328,6 +328,7 @@
+