Browse Source

Fix false positives in FormatStringIssue when params-parameter is used in unexpanded form.

pull/45/merge
Daniel Grunwald 12 years ago
parent
commit
f53526218d
  1. 16
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringHelper.cs
  2. 5
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs
  3. 3
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs
  4. 51
      ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/FormatStringIssues/FormatStringTests.cs

16
ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringHelper.cs

@ -36,14 +36,13 @@ namespace ICSharpCode.NRefactory.CSharp
static string[] parameterNames = { "format", "frmt", "fmt" }; static string[] parameterNames = { "format", "frmt", "fmt" };
public static bool TryGetFormattingParameters(CSharpInvocationResolveResult invocationResolveResult, InvocationExpression invocationExpression, public static bool TryGetFormattingParameters(CSharpInvocationResolveResult invocationResolveResult, InvocationExpression invocationExpression,
out Expression formatArgument, out TextLocation formatStart, out IList<Expression> arguments, out Expression formatArgument, out IList<Expression> arguments,
Func<IParameter, Expression, bool> argumentFilter) Func<IParameter, Expression, bool> argumentFilter)
{ {
if (argumentFilter == null) if (argumentFilter == null)
argumentFilter = (p, e) => true; argumentFilter = (p, e) => true;
formatArgument = null; formatArgument = null;
formatStart = default(TextLocation);
arguments = new List<Expression>(); arguments = new List<Expression>();
var argumentToParameterMap = invocationResolveResult.GetArgumentToParameterMap(); var argumentToParameterMap = invocationResolveResult.GetArgumentToParameterMap();
var resolvedParameters = invocationResolveResult.Member.Parameters; var resolvedParameters = invocationResolveResult.Member.Parameters;
@ -51,15 +50,22 @@ namespace ICSharpCode.NRefactory.CSharp
for (int i = 0; i < allArguments.Length; i++) { for (int i = 0; i < allArguments.Length; i++) {
var parameterIndex = argumentToParameterMap[i]; var parameterIndex = argumentToParameterMap[i];
if (parameterIndex < 0 || parameterIndex >= resolvedParameters.Count) { if (parameterIndex < 0 || parameterIndex >= resolvedParameters.Count) {
// No valid mapping for this parameter, skip it // No valid mapping for this argument, skip it
continue; continue;
} }
var parameter = resolvedParameters[parameterIndex]; var parameter = resolvedParameters[parameterIndex];
var argument = allArguments[i]; var argument = allArguments[i];
if (parameter.Type.IsKnownType(KnownTypeCode.String) && parameterNames.Contains(parameter.Name)) { if (parameter.Type.IsKnownType(KnownTypeCode.String) && parameterNames.Contains(parameter.Name)) {
formatArgument = argument; formatArgument = argument;
formatStart = argument.StartLocation; } else if (formatArgument != null && parameter.IsParams && !invocationResolveResult.IsExpandedForm) {
} else if ((formatArgument != null || parameter.IsParams) && argumentFilter(parameter, argument)) { var ace = argument as ArrayCreateExpression;
if (ace == null || ace.Initializer.IsNull)
return false;
foreach (var element in ace.Initializer.Elements) {
if (argumentFilter(parameter, element))
arguments.Add(argument);
}
} else if (formatArgument != null && argumentFilter(parameter, argument)) {
arguments.Add(argument); arguments.Add(argument);
} }
} }

5
ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs

@ -69,9 +69,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
return; return;
Expression formatArgument; Expression formatArgument;
IList<Expression> formatArguments; IList<Expression> formatArguments;
TextLocation formatStart;
if (!FormatStringHelper.TryGetFormattingParameters(invocationResolveResult, invocationExpression, if (!FormatStringHelper.TryGetFormattingParameters(invocationResolveResult, invocationExpression,
out formatArgument, out formatStart, out formatArguments, null)) { out formatArgument, out formatArguments, null)) {
return; return;
} }
var primitiveArgument = formatArgument as PrimitiveExpression; var primitiveArgument = formatArgument as PrimitiveExpression;
@ -79,7 +78,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
return; return;
var format = (string)primitiveArgument.Value; var format = (string)primitiveArgument.Value;
var parsingResult = context.ParseFormatString(format); var parsingResult = context.ParseFormatString(format);
CheckSegments(parsingResult.Segments, formatStart, formatArguments, invocationExpression); CheckSegments(parsingResult.Segments, formatArgument.StartLocation, formatArguments, invocationExpression);
} }
void CheckSegments(IList<IFormatStringSegment> segments, TextLocation formatStart, IList<Expression> formatArguments, AstNode anchor) void CheckSegments(IList<IFormatStringSegment> segments, TextLocation formatStart, IList<Expression> formatArguments, AstNode anchor)

3
ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs

@ -215,7 +215,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
{ {
Expression formatArgument; Expression formatArgument;
IList<Expression> formatArguments; IList<Expression> formatArguments;
TextLocation formatStart;
// Only check parameters that are of type object: String means it is neccessary, others // 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). // means that there is another problem (ie no matching overload of the method).
Func<IParameter, Expression, bool> predicate = (parameter, argument) => { Func<IParameter, Expression, bool> predicate = (parameter, argument) => {
@ -229,7 +228,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
return typeDefinition.IsKnownType(KnownTypeCode.Object); return typeDefinition.IsKnownType(KnownTypeCode.Object);
}; };
if (FormatStringHelper.TryGetFormattingParameters(invocationResolveResult, invocationExpression, if (FormatStringHelper.TryGetFormattingParameters(invocationResolveResult, invocationExpression,
out formatArgument, out formatStart, out formatArguments, predicate)) { out formatArgument, out formatArguments, predicate)) {
foreach (var argument in formatArguments) { foreach (var argument in formatArguments) {
CheckExpressionInAutoCallContext(argument); CheckExpressionInAutoCallContext(argument);
} }

51
ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/FormatStringIssues/FormatStringTests.cs

@ -70,6 +70,23 @@ class TestClass
Assert.AreEqual (0, issues.Count); Assert.AreEqual (0, issues.Count);
} }
[Test]
public void IgnoresCallWithUnknownNumberOfArguments()
{
var input = @"
class TestClass
{
string Bar(params object[] args)
{
return string.Format(""{1}"", args);
}
}";
TestRefactoringContext context;
var issues = GetIssues (new FormatStringIssue (), input, out context);
Assert.AreEqual (0, issues.Count);
}
[Test] [Test]
public void FormatItemIndexOutOfRangeOfArguments() public void FormatItemIndexOutOfRangeOfArguments()
{ {
@ -87,6 +104,23 @@ class TestClass
Assert.AreEqual (1, issues.Count); Assert.AreEqual (1, issues.Count);
} }
[Test]
public void FormatItemIndexOutOfRangeOfArguments_ExplicitArrayCreation()
{
var input = @"
class TestClass
{
void Foo()
{
string.Format(""{1}"", new object[] { 1 });
}
}";
TestRefactoringContext context;
var issues = GetIssues (new FormatStringIssue (), input, out context);
Assert.AreEqual (1, issues.Count);
}
[Test] [Test]
public void FormatItemMissingEndBrace() public void FormatItemMissingEndBrace()
{ {
@ -138,6 +172,23 @@ class TestClass
Assert.AreEqual (0, issues.Count); Assert.AreEqual (0, issues.Count);
} }
[Test]
public void IgnoresStringWithGoodArguments_ExplicitArrayCreation()
{
var input = @"
class TestClass
{
void Foo()
{
string.Format(""{1}"", new object[] { ""arg0"", ""arg1"" });
}
}";
TestRefactoringContext context;
var issues = GetIssues (new FormatStringIssue (), input, out context);
Assert.AreEqual (0, issues.Count);
}
[Test] [Test]
public void IgnoresNonFormattingCall() public void IgnoresNonFormattingCall()
{ {

Loading…
Cancel
Save