Browse Source

Merge pull request #113 from riviti/master

Improvements to CodeIssues and Actions
newNRvisualizers
Mike Krüger 14 years ago
parent
commit
6a50e62a7e
  1. 2
      ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj
  2. 85
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertToInitializer/AccessPath.cs
  3. 100
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertToInitializer/StatementsToInitializerConverter.cs
  4. 18
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AssignmentMadeToSameVariableIssue.cs
  5. 57
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/StringIsNullOrEmptyIssue.cs
  6. 65
      ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/AssignmentMadeToSameVariableIssueTests.cs
  7. 68
      ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/StringIsNullOrEmptyInspectorTests.cs

2
ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj

@ -476,7 +476,6 @@
<Compile Include="Refactoring\NamingHelper.cs" /> <Compile Include="Refactoring\NamingHelper.cs" />
<Compile Include="Refactoring\CodeActions\ConvertToInitializer\ConvertToInitializerAction.cs" /> <Compile Include="Refactoring\CodeActions\ConvertToInitializer\ConvertToInitializerAction.cs" />
<Compile Include="Refactoring\CodeActions\ConvertToInitializer\StatementsToInitializerConverter.cs" /> <Compile Include="Refactoring\CodeActions\ConvertToInitializer\StatementsToInitializerConverter.cs" />
<Compile Include="Refactoring\CodeActions\ConvertToInitializer\InitializerPath.cs" />
<Compile Include="Refactoring\CodeActions\MoveToOuterScopeAction.cs" /> <Compile Include="Refactoring\CodeActions\MoveToOuterScopeAction.cs" />
<Compile Include="Refactoring\CodeIssues\VariableDeclaredInWideScopeIssue.cs" /> <Compile Include="Refactoring\CodeIssues\VariableDeclaredInWideScopeIssue.cs" />
<Compile Include="Refactoring\CodeIssues\ParameterCanBeDemotedIssue\ParameterCanBeDemotedIssue.cs" /> <Compile Include="Refactoring\CodeIssues\ParameterCanBeDemotedIssue\ParameterCanBeDemotedIssue.cs" />
@ -503,6 +502,7 @@
<Compile Include="Refactoring\CodeIssues\ParameterCanBeDemotedIssue\IsArrayTypeCriterion.cs" /> <Compile Include="Refactoring\CodeIssues\ParameterCanBeDemotedIssue\IsArrayTypeCriterion.cs" />
<Compile Include="Refactoring\CodeIssues\ParameterCanBeDemotedIssue\SupportsIndexingCriterion.cs" /> <Compile Include="Refactoring\CodeIssues\ParameterCanBeDemotedIssue\SupportsIndexingCriterion.cs" />
<Compile Include="Refactoring\CodeIssues\RedundantWhereWithPredicateIssue.cs" /> <Compile Include="Refactoring\CodeIssues\RedundantWhereWithPredicateIssue.cs" />
<Compile Include="Refactoring\CodeActions\ConvertToInitializer\AccessPath.cs" />
</ItemGroup> </ItemGroup>
<ItemGroup> <ItemGroup>
<ProjectReference Include="..\ICSharpCode.NRefactory\ICSharpCode.NRefactory.csproj"> <ProjectReference Include="..\ICSharpCode.NRefactory\ICSharpCode.NRefactory.csproj">

85
ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertToInitializer/InitializerPath.cs → ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertToInitializer/AccessPath.cs

@ -32,25 +32,16 @@ using System.Linq;
namespace ICSharpCode.NRefactory.CSharp.Refactoring namespace ICSharpCode.NRefactory.CSharp.Refactoring
{ {
class InitializerPath class AccessPath
{ {
InitializerPath(object anchor) public AccessPath(IVariable target)
{ {
this.anchor = anchor; VariableRoot = target;
MemberPath = new List<IMember>(); MemberPath = new List<IMember>();
} }
public InitializerPath(IVariable target) : this((object)target) public static AccessPath FromResolveResult(ResolveResult resolveResult)
{ {
}
public InitializerPath(IMember target) : this((object)target)
{
}
public static InitializerPath FromResolveResult(ResolveResult resolveResult)
{
InitializerPath initializerPath = null;
var memberPath = new List<IMember>(); var memberPath = new List<IMember>();
var currentResolveResult = resolveResult; var currentResolveResult = resolveResult;
do { do {
@ -59,42 +50,38 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
memberPath.Add(memberResolveResult.Member); memberPath.Add(memberResolveResult.Member);
currentResolveResult = memberResolveResult.TargetResult; currentResolveResult = memberResolveResult.TargetResult;
} else if (currentResolveResult is LocalResolveResult) { } else if (currentResolveResult is LocalResolveResult) {
// This is the root variable
var localResolveResult = (LocalResolveResult)currentResolveResult; var localResolveResult = (LocalResolveResult)currentResolveResult;
memberPath.Reverse(); memberPath.Reverse();
initializerPath = new InitializerPath(localResolveResult.Variable) { return new AccessPath(localResolveResult.Variable) {
MemberPath = memberPath MemberPath = memberPath
}; };
break;
} else if (currentResolveResult is ThisResolveResult) { } else if (currentResolveResult is ThisResolveResult) {
break; break;
} else { } else {
// Unsupported path
return null; return null;
} }
} while (currentResolveResult != null); } while (currentResolveResult != null);
if (initializerPath == null) { memberPath.Reverse();
// This path is rooted at a member return new AccessPath(null) {
memberPath.Reverse(); MemberPath = memberPath
initializerPath = new InitializerPath(memberPath [0]) { };
MemberPath = memberPath.Skip(1).ToList()
};
}
return initializerPath;
} }
public InitializerPath GetParentPath() public AccessPath GetParentPath()
{ {
if (MemberPath.Count < 1) if (MemberPath.Count < 1)
throw new InvalidOperationException("Cannot get the parent path of a path that does not contain any members."); throw new InvalidOperationException("Cannot get the parent path of a path that does not contain any members.");
return new InitializerPath(anchor) { return new AccessPath(VariableRoot) {
MemberPath = MemberPath.Take(MemberPath.Count - 1).ToList() MemberPath = MemberPath.Take(MemberPath.Count - 1).ToList()
}; };
} }
public bool IsSubPath(InitializerPath other) public bool IsSubPath(AccessPath other)
{ {
if (!other.anchor.Equals(anchor)) if (!object.Equals(other.VariableRoot, VariableRoot))
return false; return false;
if (MemberPath.Count <= other.MemberPath.Count) if (MemberPath.Count <= other.MemberPath.Count)
return false; return false;
@ -105,22 +92,19 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
return true; return true;
} }
object anchor; public IVariable VariableRoot { get; set; }
public IVariable VariableRoot { public int PartCount {
get { return anchor as IVariable; } get {
} return MemberPath.Count + (VariableRoot == null ? 0 : 1);
}
public IMember MemberRoot {
get { return anchor as IMember; }
} }
public string RootName { public string RootName {
get { get {
if (anchor is IMember) if (VariableRoot != null)
return (anchor as IMember).Name; return VariableRoot.Name;
else return MemberPath.First().Name;
return (anchor as IVariable).Name;
} }
} }
@ -128,12 +112,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
public override bool Equals(object obj) public override bool Equals(object obj)
{ {
if (obj.GetType() != typeof(InitializerPath)) if (obj.GetType() != typeof(AccessPath))
return false; return false;
var other = (InitializerPath)obj; var other = (AccessPath)obj;
if (!object.Equals(anchor, other.anchor)) if (!object.Equals(VariableRoot, other.VariableRoot))
return false; return false;
if (MemberPath.Count != other.MemberPath.Count) if (MemberPath.Count != other.MemberPath.Count)
@ -148,29 +132,28 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
public override int GetHashCode() public override int GetHashCode()
{ {
int hash = anchor.GetHashCode(); int hash = VariableRoot != null ? VariableRoot.GetHashCode() : 37;
foreach (var member in MemberPath) foreach (var member in MemberPath)
hash ^= member.GetHashCode(); hash ^= 31 * member.GetHashCode();
return hash; return hash;
} }
public static bool operator==(InitializerPath left, InitializerPath right) public static bool operator==(AccessPath left, AccessPath right)
{ {
return object.Equals(left, right); return object.Equals(left, right);
} }
public static bool operator!=(InitializerPath left, InitializerPath right) public static bool operator!=(AccessPath left, AccessPath right)
{ {
return !object.Equals(left, right); return !object.Equals(left, right);
} }
public override string ToString() public override string ToString()
{ {
if (MemberPath.Count > 0) string memberPathString = string.Join(".", MemberPath.Select<IMember, string>(member => member.Name));
return string.Format("[InitializerPath: {0}.{1}]", RootName, if (VariableRoot == null)
string.Join(".", MemberPath.Select<IMember, string>(member => member.Name))); return string.Format("[AccessPath: {0}]", memberPathString);
else return string.Format("[AccessPath: {0}.{1}]", VariableRoot.Name, memberPathString);
return string.Format("[InitializerPath: {0}]", RootName);
} }
} }

100
ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertToInitializer/StatementsToInitializerConverter.cs

@ -37,9 +37,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
{ {
public class StatementsToInitializerConverter public class StatementsToInitializerConverter
{ {
IDictionary<InitializerPath, Expression> initializers = new Dictionary<InitializerPath, Expression>(); IDictionary<AccessPath, Expression> accessPaths = new Dictionary<AccessPath, Expression>();
IList<Comment> comments = new List<Comment>(); IList<Comment> comments = new List<Comment>();
InitializerPath mainInitializerPath; AccessPath mainAccessPath;
RefactoringContext context; RefactoringContext context;
public StatementsToInitializerConverter(RefactoringContext context) public StatementsToInitializerConverter(RefactoringContext context)
@ -50,13 +50,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
void Initialize(AstNode targetNode) void Initialize(AstNode targetNode)
{ {
var target = context.Resolve(targetNode); var target = context.Resolve(targetNode);
if (target is LocalResolveResult) { var targetInitializerPath = AccessPath.FromResolveResult(target);
mainInitializerPath = new InitializerPath(((LocalResolveResult)target).Variable); if (targetInitializerPath == null)
} else if (target is MemberResolveResult) { throw new ArgumentException(string.Format("Could not create the main initializer path from resolve result ({0})", target));
mainInitializerPath = new InitializerPath(((MemberResolveResult)target).Member);
} else { mainAccessPath = targetInitializerPath;
throw new ArgumentException("variableInitializer must target a variable or a member.");
}
} }
public VariableInitializer ConvertToInitializer(VariableInitializer variableInitializer, ref IList<AstNode> statements) public VariableInitializer ConvertToInitializer(VariableInitializer variableInitializer, ref IList<AstNode> statements)
@ -67,11 +65,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
throw new ArgumentNullException("statements"); throw new ArgumentNullException("statements");
Initialize(variableInitializer); Initialize(variableInitializer);
initializers [mainInitializerPath] = variableInitializer.Initializer.Clone(); accessPaths [mainAccessPath] = variableInitializer.Initializer.Clone();
Convert(statements); Convert(statements);
statements = ReplacementNodeHelper.GetReplacedNodes(initializers [mainInitializerPath]); statements = ReplacementNodeHelper.GetReplacedNodes(accessPaths [mainAccessPath]);
return new VariableInitializer(mainInitializerPath.RootName, initializers [mainInitializerPath]); return new VariableInitializer(mainAccessPath.RootName, accessPaths [mainAccessPath]);
} }
public AssignmentExpression ConvertToInitializer(AssignmentExpression assignmentExpression, ref IList<AstNode> statements) public AssignmentExpression ConvertToInitializer(AssignmentExpression assignmentExpression, ref IList<AstNode> statements)
@ -84,11 +82,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
throw new ArgumentException("assignmentExpression.Right must be an ObjectCreateExpression", "assignmentExpression"); throw new ArgumentException("assignmentExpression.Right must be an ObjectCreateExpression", "assignmentExpression");
Initialize(assignmentExpression.Left); Initialize(assignmentExpression.Left);
initializers [mainInitializerPath] = assignmentExpression.Right.Clone(); accessPaths [mainAccessPath] = assignmentExpression.Right.Clone();
Convert(statements); Convert(statements);
statements = ReplacementNodeHelper.GetReplacedNodes(initializers [mainInitializerPath]); statements = ReplacementNodeHelper.GetReplacedNodes(accessPaths [mainAccessPath]);
return new AssignmentExpression(new IdentifierExpression(mainInitializerPath.RootName), initializers [mainInitializerPath]); return new AssignmentExpression(new IdentifierExpression(mainAccessPath.RootName), accessPaths [mainAccessPath]);
} }
void Convert(IList<AstNode> originalStatements) void Convert(IList<AstNode> originalStatements)
@ -123,7 +121,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
VariableInitializer variableInitializer; VariableInitializer variableInitializer;
var variableDeclarationStatement = node as VariableDeclarationStatement; var variableDeclarationStatement = node as VariableDeclarationStatement;
if (variableDeclarationStatement == null) { if (variableDeclarationStatement == null) {
variableInitializer = VariableInitializer.Null;
return false; return false;
} }
variableInitializer = variableDeclarationStatement.Variables.FirstOrNullObject(); variableInitializer = variableDeclarationStatement.Variables.FirstOrNullObject();
@ -142,7 +139,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
var invocationExpression = expressionStatement.Expression as InvocationExpression; var invocationExpression = expressionStatement.Expression as InvocationExpression;
if (invocationExpression == null) if (invocationExpression == null)
return false; return false;
var target = invocationExpression.Target;
var invocationResolveResult = context.Resolve(invocationExpression) as InvocationResolveResult; var invocationResolveResult = context.Resolve(invocationExpression) as InvocationResolveResult;
if (invocationResolveResult == null) if (invocationResolveResult == null)
return false; return false;
@ -152,15 +148,15 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
if (targetResult is MemberResolveResult) if (targetResult is MemberResolveResult)
return false; return false;
ArrayInitializerExpression tuple = new ArrayInitializerExpression(); var tuple = new ArrayInitializerExpression();
foreach (var argument in invocationExpression.Arguments) { foreach (var argument in invocationExpression.Arguments) {
var argumentLocalResolveResult = context.Resolve(argument) as LocalResolveResult; var argumentLocalResolveResult = context.Resolve(argument) as LocalResolveResult;
if (argumentLocalResolveResult != null) { if (argumentLocalResolveResult != null) {
var initializerPath = InitializerPath.FromResolveResult(argumentLocalResolveResult); var initializerPath = AccessPath.FromResolveResult(argumentLocalResolveResult);
if (initializerPath == null || !initializers.ContainsKey(initializerPath)) if (initializerPath == null || !accessPaths.ContainsKey(initializerPath))
return false; return false;
// Add a clone, since we do not yet know if this is where the initializer will be used // Add a clone, since we do not yet know if this is where the initializer will be used
var initializerClone = initializers[initializerPath].Clone(); var initializerClone = accessPaths[initializerPath].Clone();
tuple.Elements.Add(initializerClone); tuple.Elements.Add(initializerClone);
} else { } else {
tuple.Elements.Add(argument.Clone()); tuple.Elements.Add(argument.Clone());
@ -168,11 +164,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
} }
ReplacementNodeHelper.AddReplacementAnnotation(tuple, expressionStatement); ReplacementNodeHelper.AddReplacementAnnotation(tuple, expressionStatement);
var targetPath = InitializerPath.FromResolveResult(targetResult); var targetPath = AccessPath.FromResolveResult(targetResult);
if (targetPath == null || !initializers.ContainsKey(targetPath)) if (targetPath == null || !accessPaths.ContainsKey(targetPath))
return false; return false;
InsertImplicitInitializersForPath(targetPath); InsertImplicitInitializersForPath(targetPath);
var targetInitializer = initializers [targetPath]; var targetInitializer = accessPaths [targetPath];
AddToInitializer(targetInitializer, tuple); AddToInitializer(targetInitializer, tuple);
return true; return true;
} }
@ -196,29 +192,29 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
void AddNewVariable(IVariable variable, Expression initializer, AstNode node) void AddNewVariable(IVariable variable, Expression initializer, AstNode node)
{ {
var variablePath = new InitializerPath(variable); var variablePath = new AccessPath(variable);
var rightResolveResult = context.Resolve(initializer) as LocalResolveResult; var rightResolveResult = context.Resolve(initializer) as LocalResolveResult;
if (rightResolveResult != null) { if (rightResolveResult != null) {
var rightPath = InitializerPath.FromResolveResult(rightResolveResult); var rightPath = AccessPath.FromResolveResult(rightResolveResult);
if (rightPath != null && initializers.ContainsKey(rightPath)) { if (rightPath != null && accessPaths.ContainsKey(rightPath)) {
var rightInitializer = initializers [rightPath]; var rightInitializer = accessPaths [rightPath];
ReplacementNodeHelper.AddReplacementAnnotation(rightInitializer, node); ReplacementNodeHelper.AddReplacementAnnotation(rightInitializer, node);
initializers.Remove(rightPath); accessPaths.Remove(rightPath);
initializers [variablePath] = rightInitializer; accessPaths [variablePath] = rightInitializer;
if (rightPath == mainInitializerPath) { if (rightPath == mainAccessPath) {
mainInitializerPath = variablePath; mainAccessPath = variablePath;
} }
} }
} else { } else {
initializers [variablePath] = ReplacementNodeHelper.CloneWithReplacementAnnotation(initializer, node); accessPaths [variablePath] = ReplacementNodeHelper.CloneWithReplacementAnnotation(initializer, node);
} }
} }
void AddOldAnnotationsToInitializer(InitializerPath targetPath, Expression initializer) void AddOldAnnotationsToInitializer(AccessPath targetPath, IAnnotatable initializer)
{ {
if (targetPath != null) { if (targetPath != null) {
if (initializers.ContainsKey(targetPath)) { if (accessPaths.ContainsKey(targetPath)) {
foreach (var astNode in ReplacementNodeHelper.GetAllReplacementAnnotations(initializers[targetPath])) { foreach (var astNode in ReplacementNodeHelper.GetAllReplacementAnnotations(accessPaths[targetPath])) {
initializer.AddAnnotation(astNode); initializer.AddAnnotation(astNode);
} }
} }
@ -231,16 +227,16 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
var leftResolveResult = context.Resolve(left); var leftResolveResult = context.Resolve(left);
Expression initializer; Expression initializer;
if (rightResolveResult != null) { if (rightResolveResult != null) {
var rightPath = InitializerPath.FromResolveResult(rightResolveResult); var rightPath = AccessPath.FromResolveResult(rightResolveResult);
if (initializers.ContainsKey(rightPath)) { if (accessPaths.ContainsKey(rightPath)) {
initializer = initializers [rightPath]; initializer = accessPaths [rightPath];
} else { } else {
initializer = right.Clone(); initializer = right.Clone();
} }
} else { } else {
initializer = right.Clone(); initializer = right.Clone();
} }
var leftPath = InitializerPath.FromResolveResult(leftResolveResult); var leftPath = AccessPath.FromResolveResult(leftResolveResult);
if (leftPath == null) { if (leftPath == null) {
return false; return false;
} }
@ -249,15 +245,15 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
// to the same variable/member. // to the same variable/member.
AddOldAnnotationsToInitializer(leftPath, initializer); AddOldAnnotationsToInitializer(leftPath, initializer);
if (leftPath.MemberPath.Count == 0) { if (leftPath.PartCount == 1) {
ReplacementNodeHelper.AddReplacementAnnotation(initializer, node); ReplacementNodeHelper.AddReplacementAnnotation(initializer, node);
initializers [leftPath] = initializer; accessPaths [leftPath] = initializer;
return true; return true;
} }
if (!(leftResolveResult is MemberResolveResult)) if (!(leftResolveResult is MemberResolveResult))
return false; return false;
Debug.Assert(leftPath.MemberPath.Count > 0, "No top level assignment should get here."); Debug.Assert(leftPath.PartCount > 1, "No top level assignment should get here.");
var parentKey = leftPath.GetParentPath(); var parentKey = leftPath.GetParentPath();
var member = leftPath.MemberPath.Last(); var member = leftPath.MemberPath.Last();
@ -266,13 +262,13 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
if (!success) if (!success)
return false; return false;
var parentInitializer = initializers [parentKey]; var parentInitializer = accessPaths [parentKey];
AddToInitializer(parentInitializer, comments.ToArray()); AddToInitializer(parentInitializer, comments.ToArray());
comments.Clear(); comments.Clear();
AddToInitializer(parentInitializer, new NamedExpression(member.Name, initializer)); AddToInitializer(parentInitializer, new NamedExpression(member.Name, initializer));
ReplacementNodeHelper.AddReplacementAnnotation(initializer, node); ReplacementNodeHelper.AddReplacementAnnotation(initializer, node);
initializers [leftPath] = initializer; accessPaths [leftPath] = initializer;
return true; return true;
} }
@ -347,8 +343,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
var resolveResult = context.Resolve(memberReference) as MemberResolveResult; var resolveResult = context.Resolve(memberReference) as MemberResolveResult;
if (resolveResult == null) if (resolveResult == null)
continue; continue;
var initializerPath = InitializerPath.FromResolveResult(resolveResult); var initializerPath = AccessPath.FromResolveResult(resolveResult);
if (initializerPath != null && initializers.ContainsKey(initializerPath)) if (initializerPath != null && accessPaths.ContainsKey(initializerPath))
return true; return true;
} }
return false; return false;
@ -356,12 +352,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
bool VariableHasBeenConverted(IVariable variable) bool VariableHasBeenConverted(IVariable variable)
{ {
return initializers.Any(item => item.Key.VariableRoot.Equals(variable)); return accessPaths.Any(item => item.Key.VariableRoot.Equals(variable));
} }
bool InsertImplicitInitializersForPath(InitializerPath path) bool InsertImplicitInitializersForPath(AccessPath path)
{ {
if (initializers.ContainsKey(path)) if (accessPaths.ContainsKey(path))
return true; return true;
if (path.MemberPath.Count == 0) if (path.MemberPath.Count == 0)
@ -371,11 +367,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
if (!success) if (!success)
return false; return false;
var parentInitializer = initializers [parentPath]; var parentInitializer = accessPaths [parentPath];
var initializer = new ArrayInitializerExpression(); var initializer = new ArrayInitializerExpression();
var namedExpression = new NamedExpression(path.MemberPath [path.MemberPath.Count - 1].Name, initializer); var namedExpression = new NamedExpression(path.MemberPath [path.MemberPath.Count - 1].Name, initializer);
AddToInitializer(parentInitializer, namedExpression); AddToInitializer(parentInitializer, namedExpression);
initializers [path] = initializer; accessPaths [path] = initializer;
return true; return true;
} }

18
ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AssignmentMadeToSameVariableIssue.cs

@ -29,6 +29,7 @@ using System.Collections.Generic;
using ICSharpCode.NRefactory.Semantics; using ICSharpCode.NRefactory.Semantics;
using ICSharpCode.NRefactory.TypeSystem; using ICSharpCode.NRefactory.TypeSystem;
using ICSharpCode.NRefactory.PatternMatching; using ICSharpCode.NRefactory.PatternMatching;
using System.Linq;
namespace ICSharpCode.NRefactory.CSharp.Refactoring namespace ICSharpCode.NRefactory.CSharp.Refactoring
{ {
@ -62,14 +63,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
var resolveResult = ctx.Resolve (assignmentExpression.Left); var resolveResult = ctx.Resolve (assignmentExpression.Left);
var memberResolveResult = resolveResult as MemberResolveResult; var memberResolveResult = resolveResult as MemberResolveResult;
if (memberResolveResult != null) { if (memberResolveResult != null) {
if (!(memberResolveResult.Member is IField)) var memberResolveResult2 = ctx.Resolve (assignmentExpression.Right) as MemberResolveResult;
if (memberResolveResult2 == null || !AreEquivalent(memberResolveResult, memberResolveResult2))
return; return;
if (!assignmentExpression.Left.Match (assignmentExpression.Right).Success) {
// in case: this.field = field
var memberResolveResult2 = ctx.Resolve (assignmentExpression.Right) as MemberResolveResult;
if (memberResolveResult2 == null || memberResolveResult.Member != memberResolveResult2.Member)
return;
}
} else if (resolveResult is LocalResolveResult) { } else if (resolveResult is LocalResolveResult) {
if (!assignmentExpression.Left.Match (assignmentExpression.Right).Success) if (!assignmentExpression.Left.Match (assignmentExpression.Right).Success)
return; return;
@ -89,6 +85,14 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
AddIssue (node, ctx.TranslateString ("CS1717:Assignment made to same variable"), AddIssue (node, ctx.TranslateString ("CS1717:Assignment made to same variable"),
new [] { new CodeAction (ctx.TranslateString ("Remove assignment"), action) }); new [] { new CodeAction (ctx.TranslateString ("Remove assignment"), action) });
} }
static bool AreEquivalent(ResolveResult first, ResolveResult second)
{
var firstPath = InitializerPath.FromResolveResult(first);
var secondPath = InitializerPath.FromResolveResult(second);
return firstPath != null && firstPath.Equals(secondPath) && !firstPath.MemberPath.Any(m => !(m is IField)) &&
(firstPath.MemberRoot == null || firstPath.MemberRoot is IField);
}
} }
} }
} }

57
ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/StringIsNullOrEmptyIssue.cs

@ -41,47 +41,76 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
public class StringIsNullOrEmptyIssue : ICodeIssueProvider public class StringIsNullOrEmptyIssue : ICodeIssueProvider
{ {
static readonly Pattern pattern = new Choice { static readonly Pattern pattern = new Choice {
// str == null || str == "" // str == null || str == ""
// str == null || str.Length == 0
new BinaryOperatorExpression ( new BinaryOperatorExpression (
PatternHelper.CommutativeOperator(new AnyNode ("str"), BinaryOperatorType.Equality, new NullReferenceExpression ()), PatternHelper.CommutativeOperator (new AnyNode ("str"), BinaryOperatorType.Equality, new NullReferenceExpression ()),
BinaryOperatorType.ConditionalOr, BinaryOperatorType.ConditionalOr,
PatternHelper.CommutativeOperator(new Backreference ("str"), BinaryOperatorType.Equality, new PrimitiveExpression ("")) new Choice {
PatternHelper.CommutativeOperator (new Backreference ("str"), BinaryOperatorType.Equality, new PrimitiveExpression ("")),
PatternHelper.CommutativeOperator (
new MemberReferenceExpression (new Backreference ("str"), "Length"),
BinaryOperatorType.Equality,
new PrimitiveExpression (0)
)
}
), ),
// str == "" || str == null // str == "" || str == null
// str.Length == 0 || str == null
new BinaryOperatorExpression ( new BinaryOperatorExpression (
PatternHelper.CommutativeOperator(new AnyNode ("str"), BinaryOperatorType.Equality, new PrimitiveExpression ("")), new Choice {
PatternHelper.CommutativeOperator (new AnyNode ("str"), BinaryOperatorType.Equality, new PrimitiveExpression ("")),
PatternHelper.CommutativeOperator (
new MemberReferenceExpression (new AnyNode ("str"), "Length"),
BinaryOperatorType.Equality,
new PrimitiveExpression (0)
)
},
BinaryOperatorType.ConditionalOr, BinaryOperatorType.ConditionalOr,
PatternHelper.CommutativeOperator(new Backreference ("str"), BinaryOperatorType.Equality, new NullReferenceExpression ()) PatternHelper.CommutativeOperator(new Backreference ("str"), BinaryOperatorType.Equality, new NullReferenceExpression ())
), )
}; };
static readonly Pattern negPattern = new Choice { static readonly Pattern negPattern = new Choice {
// str != null && str != "" // str != null && str != ""
// str != null && str.Length != 0
new BinaryOperatorExpression ( new BinaryOperatorExpression (
PatternHelper.CommutativeOperator(new AnyNode ("str"), BinaryOperatorType.InEquality, new NullReferenceExpression ()), PatternHelper.CommutativeOperator(new AnyNode ("str"), BinaryOperatorType.InEquality, new NullReferenceExpression ()),
BinaryOperatorType.ConditionalAnd, BinaryOperatorType.ConditionalAnd,
PatternHelper.CommutativeOperator(new Backreference ("str"), BinaryOperatorType.InEquality, new PrimitiveExpression ("")) new Choice {
PatternHelper.CommutativeOperator (new Backreference ("str"), BinaryOperatorType.InEquality, new PrimitiveExpression ("")),
PatternHelper.CommutativeOperator (
new MemberReferenceExpression (new Backreference ("str"), "Length"),
BinaryOperatorType.InEquality,
new PrimitiveExpression (0)
)
}
), ),
// str != "" && str != null // str != "" && str != null
// str.Length != 0 && str != null
new BinaryOperatorExpression ( new BinaryOperatorExpression (
PatternHelper.CommutativeOperator(new AnyNode ("str"), BinaryOperatorType.InEquality, new PrimitiveExpression ("")), new Choice {
PatternHelper.CommutativeOperator (new AnyNode ("str"), BinaryOperatorType.InEquality, new PrimitiveExpression ("")),
PatternHelper.CommutativeOperator (
new MemberReferenceExpression (new AnyNode ("str"), "Length"),
BinaryOperatorType.InEquality,
new PrimitiveExpression (0)
)
},
BinaryOperatorType.ConditionalAnd, BinaryOperatorType.ConditionalAnd,
PatternHelper.CommutativeOperator(new Backreference ("str"), BinaryOperatorType.InEquality, new NullReferenceExpression ()) PatternHelper.CommutativeOperator(new Backreference ("str"), BinaryOperatorType.InEquality, new NullReferenceExpression ())
), )
}; };
public IEnumerable<CodeIssue> GetIssues(BaseRefactoringContext context) public IEnumerable<CodeIssue> GetIssues(BaseRefactoringContext context)
{ {
return new GatherVisitor(context, this).GetIssues(); return new GatherVisitor(context).GetIssues();
} }
class GatherVisitor : GatherVisitorBase class GatherVisitor : GatherVisitorBase
{ {
readonly StringIsNullOrEmptyIssue inspector; public GatherVisitor (BaseRefactoringContext ctx) : base (ctx)
public GatherVisitor (BaseRefactoringContext ctx, StringIsNullOrEmptyIssue inspector) : base (ctx)
{ {
this.inspector = inspector;
} }
public override void VisitBinaryOperatorExpression(BinaryOperatorExpression binaryOperatorExpression) public override void VisitBinaryOperatorExpression(BinaryOperatorExpression binaryOperatorExpression)

65
ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/AssignmentMadeToSameVariableIssueTests.cs

@ -127,7 +127,7 @@ class TestClass
}"; }";
Test<AssignmentMadeToSameVariableIssue> (input, 2, output); Test<AssignmentMadeToSameVariableIssue> (input, 2, output);
} }
[Test] [Test]
public void TestNoIssue () public void TestNoIssue ()
{ {
@ -146,6 +146,69 @@ class TestClass
this.a = a; this.a = a;
Prop = Prop; Prop = Prop;
} }
}";
Test<AssignmentMadeToSameVariableIssue> (input, 0);
}
[Test]
public void IgnoresAssignmentWithDifferentRootObjects ()
{
var input = @"
class TestClass
{
int a;
void TestMethod (TestClass tc)
{
a = tc.a;
}
}";
Test<AssignmentMadeToSameVariableIssue> (input, 0);
}
[Test]
public void NestedFieldAccess ()
{
var input = @"
class TestClass
{
int a;
TestClass nested;
void TestMethod ()
{
nested.nested.a = nested.nested.a;
}
}";
var output = @"
class TestClass
{
int a;
TestClass nested;
void TestMethod ()
{
}
}";
Test<AssignmentMadeToSameVariableIssue> (input, 1, output);
}
[Test]
public void NestedPropertyAccess ()
{
var input = @"
class TestClass
{
int a;
TestClass nested { get; set; }
void TestMethod ()
{
nested.nested.a = nested.nested.a;
}
}"; }";
Test<AssignmentMadeToSameVariableIssue> (input, 0); Test<AssignmentMadeToSameVariableIssue> (input, 0);
} }

68
ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/StringIsNullOrEmptyInspectorTests.cs

@ -410,7 +410,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeIssues
} }
}"); }");
} }
[Test] [Test]
public void TestInspectorCaseSN8 () public void TestInspectorCaseSN8 ()
{ {
@ -422,7 +422,39 @@ namespace ICSharpCode.NRefactory.CSharp.CodeIssues
; ;
} }
}"; }";
TestRefactoringContext context;
var issues = GetIssues (new StringIsNullOrEmptyIssue (), input, out context);
Assert.AreEqual (1, issues.Count);
CheckFix (context, issues, @"class Foo
{
void Bar (string str)
{
if (string.IsNullOrEmpty (str))
;
}
}");
}
[TestCase("str == null || str.Length == 0")]
[TestCase("str == null || 0 == str.Length")]
[TestCase("str.Length == 0 || str == null")]
[TestCase("0 == str.Length || str == null")]
[TestCase("null == str || str.Length == 0")]
[TestCase("null == str || 0 == str.Length")]
[TestCase("str.Length == 0 || null == str")]
[TestCase("0 == str.Length || null == str")]
public void TestInspectorCaseNL (string expression)
{
var input = @"class Foo
{
void Bar (string str)
{
if (" + expression + @")
;
}
}";
TestRefactoringContext context; TestRefactoringContext context;
var issues = GetIssues (new StringIsNullOrEmptyIssue (), input, out context); var issues = GetIssues (new StringIsNullOrEmptyIssue (), input, out context);
Assert.AreEqual (1, issues.Count); Assert.AreEqual (1, issues.Count);
@ -433,6 +465,38 @@ namespace ICSharpCode.NRefactory.CSharp.CodeIssues
if (string.IsNullOrEmpty (str)) if (string.IsNullOrEmpty (str))
; ;
} }
}");
}
[TestCase("str != null && str.Length != 0")]
[TestCase("str != null && 0 != str.Length")]
[TestCase("str.Length != 0 && str != null")]
[TestCase("0 != str.Length && str != null")]
[TestCase("null != str && str.Length != 0")]
[TestCase("null != str && 0 != str.Length")]
[TestCase("str.Length != 0 && null != str")]
[TestCase("0 != str.Length && null != str")]
public void TestInspectorCaseLN (string expression)
{
var input = @"class Foo
{
void Bar (string str)
{
if (" + expression + @")
;
}
}";
TestRefactoringContext context;
var issues = GetIssues (new StringIsNullOrEmptyIssue (), input, out context);
Assert.AreEqual (1, issues.Count);
CheckFix (context, issues, @"class Foo
{
void Bar (string str)
{
if (!string.IsNullOrEmpty (str))
;
}
}"); }");
} }
} }

Loading…
Cancel
Save