From c518c640f1f9d6229b6b9b3cf5b3db59f980627a Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Thu, 13 Sep 2012 20:29:01 +0200 Subject: [PATCH 01/14] [Refactoring] Add LocalReferenceFinder. --- .../ICSharpCode.NRefactory.CSharp.csproj | 3 +- .../Refactoring/LocalReferenceFinder.cs | 138 ++++++++++++++++++ 2 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 ICSharpCode.NRefactory.CSharp/Refactoring/LocalReferenceFinder.cs diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj index d1517435ec..ccaca925cb 100644 --- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj +++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj @@ -503,6 +503,7 @@ + @@ -534,4 +535,4 @@ - \ No newline at end of file + diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/LocalReferenceFinder.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/LocalReferenceFinder.cs new file mode 100644 index 0000000000..04de48afab --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/LocalReferenceFinder.cs @@ -0,0 +1,138 @@ +// +// LocalReferenceFinder.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.Collections.Generic; +using System.Linq; +using ICSharpCode.NRefactory.CSharp.Resolver; +using ICSharpCode.NRefactory.Semantics; +using ICSharpCode.NRefactory.TypeSystem; +using System; +using ICSharpCode.NRefactory.Utils; +using System.Diagnostics; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring +{ + + /// + /// Finds references to IVariables. + /// + /// + /// This class is more efficient than + /// if there is already a resolved tree or if multiple searches needs + /// to be performed. + /// + public class LocalReferenceFinder + { + LocalReferenceLocator locator; + + MultiDictionary> references = new MultiDictionary>(); + + public LocalReferenceFinder(CSharpAstResolver resolver) + { + locator = new LocalReferenceLocator(resolver, this); + } + + public LocalReferenceFinder(BaseRefactoringContext context) : this(context.Resolver) + { + } + + HashSet visitedRoots = new HashSet(); + + void VisitIfNeccessary(AstNode rootNode) + { + // If any of the parent nodes are recorded as visited, + // we don't need to traverse rootNode this time. + var tmpRoot = rootNode; + while(tmpRoot != null){ + if (visitedRoots.Contains(tmpRoot)) + return; + tmpRoot = tmpRoot.Parent; + } + + rootNode.AcceptVisitor(locator); + visitedRoots.Add(rootNode); + } + + /// + /// Finds the references to . + /// + /// + /// Root node for the search. + /// + /// + /// The variable to find references for. + /// + /// + /// Will be called for each reference found. + /// + /// + /// When a single is reused for multiple + /// searches, which references outside of are + /// or are not reported is undefined. + /// + public void FindReferences (AstNode rootNode, IVariable variable, FoundReferenceCallback action) + { + VisitIfNeccessary(rootNode); + var lookup = (ILookup>)references; + if (!lookup.Contains(variable)) + return; + var iList = references[variable]; + foreach (var reference in iList) { + action(reference.Item1, reference.Item2); + } + } + + class LocalReferenceLocator : DepthFirstAstVisitor + { + CSharpAstResolver resolver; + + LocalReferenceFinder referenceFinder; + + public LocalReferenceLocator(CSharpAstResolver resolver, LocalReferenceFinder referenceFinder) + { + this.resolver = resolver; + this.referenceFinder = referenceFinder; + } + + IList processedVariables = new List(); + + protected override void VisitChildren(AstNode node) + { + var localResolveResult = resolver.Resolve(node) as LocalResolveResult; + if (localResolveResult != null && !processedVariables.Contains(localResolveResult.Variable)) { + referenceFinder.references.Add(localResolveResult.Variable, Tuple.Create(node, localResolveResult)); + + processedVariables.Add(localResolveResult.Variable); + base.VisitChildren(node); + Debug.Assert(processedVariables.Contains(localResolveResult.Variable), "Variable should still be in the list of processed variables."); + processedVariables.Remove(localResolveResult.Variable); + } else { + base.VisitChildren(node); + } + } + } + } +} From 6ec44d10d56b5eff361ca51c5f549d0d4410dc9e Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Thu, 13 Sep 2012 22:22:34 +0200 Subject: [PATCH 02/14] Replace FindReferences with LocalReferenceFinder in {Variable,Parameter}OnlyAssignedIssue. --- .../LocalVariableOnlyAssignedIssue.cs | 13 ++++++------- .../ParameterOnlyAssignedIssue.cs | 13 ++++++------- .../VariableOnlyAssignedIssue.cs | 18 +++++------------- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/LocalVariableOnlyAssignedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/LocalVariableOnlyAssignedIssue.cs index c619d03157..0e6eacbc1f 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/LocalVariableOnlyAssignedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/LocalVariableOnlyAssignedIssue.cs @@ -36,19 +36,18 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring IssueMarker = IssueMarker.Underline)] public class LocalVariableOnlyAssignedIssue : VariableOnlyAssignedIssue { - internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) + internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext ctx) { - return new GatherVisitor (ctx, unit); + return new GatherVisitor (ctx); } class GatherVisitor : GatherVisitorBase { - SyntaxTree unit; - - public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) + LocalReferenceFinder referenceFinder; + public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { - this.unit = unit; + referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitVariableInitializer (VariableInitializer variableInitializer) @@ -62,7 +61,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var resolveResult = ctx.Resolve (variableInitializer) as LocalResolveResult; if (resolveResult == null) return; - if (!TestOnlyAssigned (ctx, unit, resolveResult.Variable)) + if (!TestOnlyAssigned (referenceFinder, decl.Parent, resolveResult.Variable)) return; AddIssue (variableInitializer.NameToken, ctx.TranslateString ("Local variable is assigned by its value is never used")); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/ParameterOnlyAssignedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/ParameterOnlyAssignedIssue.cs index bea8b1599e..4a8abe3732 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/ParameterOnlyAssignedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/ParameterOnlyAssignedIssue.cs @@ -35,19 +35,18 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring IssueMarker = IssueMarker.Underline)] public class ParameterOnlyAssignedIssue : VariableOnlyAssignedIssue { - internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) + internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext ctx) { - return new GatherVisitor (ctx, unit); + return new GatherVisitor (ctx); } private class GatherVisitor : GatherVisitorBase { - SyntaxTree unit; - - public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) + LocalReferenceFinder referenceFinder; + public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { - this.unit = unit; + referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitParameterDeclaration (ParameterDeclaration parameterDeclaration) @@ -58,7 +57,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (resolveResult == null) return; if (parameterDeclaration.ParameterModifier == ParameterModifier.Out || parameterDeclaration.ParameterModifier == ParameterModifier.Ref - || !TestOnlyAssigned (ctx, unit, resolveResult.Variable)) + || !TestOnlyAssigned (referenceFinder, parameterDeclaration.Parent, resolveResult.Variable)) return; AddIssue (parameterDeclaration.NameToken, ctx.TranslateString ("Parameter is assigned by its value is never used")); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/VariableOnlyAssignedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/VariableOnlyAssignedIssue.cs index a4b965ed69..d86c334959 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/VariableOnlyAssignedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/VariableOnlyAssignedIssue.cs @@ -25,30 +25,22 @@ // THE SOFTWARE. using System.Collections.Generic; -using System.Linq; -using ICSharpCode.NRefactory.CSharp.Resolver; using ICSharpCode.NRefactory.TypeSystem; namespace ICSharpCode.NRefactory.CSharp.Refactoring { public abstract class VariableOnlyAssignedIssue : ICodeIssueProvider { - static FindReferences refFinder = new FindReferences (); public IEnumerable GetIssues (BaseRefactoringContext context) { - var unit = context.RootNode as SyntaxTree; - if (unit == null) - return Enumerable.Empty (); - return GetGatherVisitor (context, unit).GetIssues (); + return GetGatherVisitor (context).GetIssues (); } - protected static bool TestOnlyAssigned (BaseRefactoringContext ctx, SyntaxTree unit, IVariable variable) + protected static bool TestOnlyAssigned (LocalReferenceFinder referenceFinder, AstNode rootNode, IVariable variable) { var assignment = false; var nonAssignment = false; - refFinder.FindLocalReferences (variable, ctx.UnresolvedFile, unit, ctx.Compilation, - (node, resolveResult) => - { + referenceFinder.FindReferences (rootNode, variable, (node, resolveResult) => { if (node is ParameterDeclaration) return; @@ -83,10 +75,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring } } nonAssignment = true; - }, ctx.CancellationToken); + }); return assignment && !nonAssignment; } - internal abstract GatherVisitorBase GetGatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit); + internal abstract GatherVisitorBase GetGatherVisitor (BaseRefactoringContext ctx); } } From b1860b1f076873895bc9df8df60bfda68cab7ef0 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Thu, 13 Sep 2012 22:25:42 +0200 Subject: [PATCH 03/14] Replace FindReferences with LocalReferenceFinder in AccessToClosureIssue. --- .../AccessToClosureIssues/AccessToClosureIssue.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs index c54af78854..9f108acfe9 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs @@ -35,7 +35,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { public abstract class AccessToClosureIssue : ICodeIssueProvider { - static FindReferences refFinder = new FindReferences (); static ControlFlowGraphBuilder cfgBuilder = new ControlFlowGraphBuilder (); public string Title @@ -75,7 +74,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { - SyntaxTree unit; string title; AccessToClosureIssue issueProvider; @@ -84,7 +82,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring : base (context) { this.title = context.TranslateString (issueProvider.Title); - this.unit = unit; + this.referenceFinder = new LocalReferenceFinder(context); this.issueProvider = issueProvider; } @@ -124,10 +122,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring base.VisitParameterDeclaration (parameterDeclaration); } - void FindLocalReferences (IVariable variable, FoundReferenceCallback callback) + LocalReferenceFinder referenceFinder; + + void FindLocalReferences (AstNode rootNode, IVariable variable, FoundReferenceCallback callback) { - refFinder.FindLocalReferences (variable, ctx.UnresolvedFile, unit, ctx.Compilation, callback, - ctx.CancellationToken); + referenceFinder.FindReferences(rootNode, variable, callback); } void CheckVariable (IVariable variable, Statement env) @@ -139,7 +138,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var envLookup = new Dictionary (); envLookup [env] = root; - FindLocalReferences (variable, (astNode, resolveResult) => + FindLocalReferences (env, variable, (astNode, resolveResult) => AddNode (envLookup, new Node (astNode, issueProvider.GetNodeKind (astNode)))); root.SortChildren (); From 1d129a6a74bba951ce62941e4a73b5d0daa4bfc1 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Thu, 13 Sep 2012 22:26:40 +0200 Subject: [PATCH 04/14] Replace FindReferences with LocalReferenceFinder in RedundantAssignmentIssue. --- .../CodeIssues/RedundantAssignmentIssue.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantAssignmentIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantAssignmentIssue.cs index 3effea84aa..ebfcee99b2 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantAssignmentIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantAssignmentIssue.cs @@ -48,14 +48,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { - static FindReferences refFinder = new FindReferences (); - - SyntaxTree unit; + LocalReferenceFinder referenceFinder; public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) : base (ctx) { - this.unit = unit; + referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitParameterDeclaration (ParameterDeclaration parameterDeclaration) @@ -89,11 +87,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { if (rootStatement == null || resolveResult == null) return; + var references = new HashSet (); var refStatements = new HashSet (); var usedInLambda = false; - refFinder.FindLocalReferences (resolveResult.Variable, ctx.UnresolvedFile, unit, ctx.Compilation, - (astNode, rr) => { + referenceFinder.FindReferences (rootStatement, resolveResult.Variable, (astNode, rr) => { if (usedInLambda || astNode == variableDecl) return; @@ -116,7 +114,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring } parent = parent.Parent; } - }, ctx.CancellationToken); + }); // stop analyzing if the variable is used in any lambda expression or anonymous method if (usedInLambda) From b6cbd38edd371739066249732dcebd6cbcefe959 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Thu, 13 Sep 2012 22:28:08 +0200 Subject: [PATCH 05/14] Add check to disqualify most invocations early in OptionalParameterCouldBeSkippedIssue. --- .../OptionalParameterCouldBeSkippedIssue.cs | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/OptionalParameterCouldBeSkippedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/OptionalParameterCouldBeSkippedIssue.cs index 6c7f378a4d..792ac2fa9e 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/OptionalParameterCouldBeSkippedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/OptionalParameterCouldBeSkippedIssue.cs @@ -66,15 +66,24 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring (invocation, args) => new InvocationExpression(invocation.Target.Clone(), args)); } - void CheckMethodCall (T node, IEnumerable arguments, Func, T> generateReplacement) where T: AstNode + void CheckMethodCall (T node, IEnumerable args, Func, T> generateReplacement) where T: AstNode { + // The first two checks are unnecessary, but eliminates the majority of calls early, + // improving performance. + var arguments = args.ToArray(); + if (arguments.Length == 0) + return; + var lastArg = arguments[arguments.Length - 1]; + if (!(lastArg is PrimitiveExpression || lastArg is NamedArgumentExpression)) + return; + var invocationResolveResult = ctx.Resolve(node) as CSharpInvocationResolveResult; if (invocationResolveResult == null) return; string actionMessage = ctx.TranslateString("Remove redundant arguments"); - var redundantArguments = GetRedundantArguments(arguments.ToArray(), invocationResolveResult); + var redundantArguments = GetRedundantArguments(arguments, invocationResolveResult); var action = new CodeAction(actionMessage, script => { var newArgumentList = arguments .Where(arg => !redundantArguments.Contains(arg)) @@ -84,7 +93,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring }); var issueMessage = ctx.TranslateString("Argument is identical to the default value"); var lastPositionalArgument = redundantArguments.FirstOrDefault(expression => !(expression is NamedArgumentExpression)); - bool hasNamedArguments = false; foreach (var argument in redundantArguments) { var localArgument = argument; @@ -100,7 +108,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring 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 => { @@ -116,11 +123,13 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring } } - IEnumerable GetRedundantArguments(Expression[] arguments, CSharpInvocationResolveResult invocationResolveResult) + IList GetRedundantArguments(Expression[] arguments, CSharpInvocationResolveResult invocationResolveResult) { var argumentToParameterMap = invocationResolveResult.GetArgumentToParameterMap(); var resolvedParameters = invocationResolveResult.Member.Parameters; + IList redundantArguments = new List(); + for (int i = arguments.Length - 1; i >= 0; i--) { var parameterIndex = argumentToParameterMap[i]; if (parameterIndex == -1) @@ -141,7 +150,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring // Stop here since any arguments before this one has to be there // to enable the passing of this argument break; - yield return argument; + redundantArguments.Add(argument); } else if (argument is NamedArgumentExpression) { var expression = ((NamedArgumentExpression)argument).Expression as PrimitiveExpression; if (expression == null) @@ -150,12 +159,13 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (expressionResolveResult == null || parameter.ConstantValue != expressionResolveResult.ConstantValue) // continue, since there can still be more arguments that are redundant continue; - yield return argument; + redundantArguments.Add(argument); } else { // This is a non-constant positional argument => no more redundancies are possible break; } } + return redundantArguments; } } } From 8be6f487e9a090337a5a7d98b623023088951937 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Fri, 14 Sep 2012 00:59:05 +0200 Subject: [PATCH 06/14] Replace FindReferences with LocalReferenceFinder in VariableNotUsedIssue. --- .../LocalVariableNotUsedIssue.cs | 18 +++++++++++------- .../ParameterNotUsedIssue.cs | 16 ++++++++++------ .../VariableNotUsedIssue.cs | 18 ++++++------------ 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/LocalVariableNotUsedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/LocalVariableNotUsedIssue.cs index f6645bf9ef..c6900afc93 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/LocalVariableNotUsedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/LocalVariableNotUsedIssue.cs @@ -35,19 +35,22 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring IssueMarker = IssueMarker.GrayOut)] public class LocalVariableNotUsedIssue : VariableNotUsedIssue { - internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context, SyntaxTree unit) + internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context) { - return new GatherVisitor (context, unit); + return new GatherVisitor (context, this); } class GatherVisitor : GatherVisitorBase { - SyntaxTree unit; + LocalReferenceFinder referenceFinder; - public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) + VariableNotUsedIssue inspector; + + public GatherVisitor (BaseRefactoringContext ctx, VariableNotUsedIssue inspector) : base (ctx) { - this.unit = unit; + this.inspector = inspector; + this.referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitVariableInitializer (VariableInitializer variableInitializer) @@ -65,7 +68,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (resolveResult == null) return; - if (FindUsage (ctx, unit, resolveResult.Variable, variableInitializer)) + var b = inspector.FindUsage(referenceFinder, decl.Parent, resolveResult.Variable, variableInitializer); + if (b) return; AddIssue (variableInitializer.NameToken, ctx.TranslateString ("Remove unused local variable"), @@ -90,7 +94,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (resolveResult == null) return; - if (FindUsage (ctx, unit, resolveResult.Variable, foreachStatement.VariableNameToken)) + if (inspector.FindUsage (referenceFinder, foreachStatement, resolveResult.Variable, foreachStatement.VariableNameToken)) return; AddIssue (foreachStatement.VariableNameToken, ctx.TranslateString ("Local variable is never used")); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/ParameterNotUsedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/ParameterNotUsedIssue.cs index d8b24b8b56..6cf5bfbc5a 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/ParameterNotUsedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/ParameterNotUsedIssue.cs @@ -38,19 +38,22 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring public class ParameterNotUsedIssue : VariableNotUsedIssue { - internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context, SyntaxTree unit) + internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context) { - return new GatherVisitor (context, unit); + return new GatherVisitor (context, this); } class GatherVisitor : GatherVisitorBase { - SyntaxTree unit; + VariableNotUsedIssue inspector; - public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) + LocalReferenceFinder referenceFinder; + + public GatherVisitor (BaseRefactoringContext ctx, VariableNotUsedIssue inspector) : base (ctx) { - this.unit = unit; + this.inspector = inspector; + this.referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitMethodDeclaration(MethodDeclaration methodDeclaration) @@ -81,7 +84,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var resolveResult = ctx.Resolve (parameterDeclaration) as LocalResolveResult; if (resolveResult == null) return; - if (FindUsage (ctx, unit, resolveResult.Variable, parameterDeclaration)) + + if (inspector.FindUsage (referenceFinder, parameterDeclaration.Parent, resolveResult.Variable, parameterDeclaration)) return; AddIssue (parameterDeclaration.NameToken, ctx.TranslateString ("Parameter is never used")); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/VariableNotUsedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/VariableNotUsedIssue.cs index c00869dddf..9e33c81bd6 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/VariableNotUsedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/VariableNotUsedIssue.cs @@ -26,35 +26,29 @@ using System.Collections.Generic; using System.Linq; -using ICSharpCode.NRefactory.CSharp.Resolver; using ICSharpCode.NRefactory.TypeSystem; namespace ICSharpCode.NRefactory.CSharp.Refactoring { public abstract class VariableNotUsedIssue : ICodeIssueProvider { - static FindReferences refFinder = new FindReferences (); - public IEnumerable GetIssues (BaseRefactoringContext context) { var unit = context.RootNode as SyntaxTree; if (unit == null) return Enumerable.Empty (); - return GetGatherVisitor(context, unit).GetIssues (); + return GetGatherVisitor(context).GetIssues (); } - protected static bool FindUsage (BaseRefactoringContext context, SyntaxTree unit, IVariable variable, - AstNode declaration) + public bool FindUsage (LocalReferenceFinder referenceFinder, AstNode rootNode, IVariable variable, AstNode declaration) { var found = false; - refFinder.FindLocalReferences (variable, context.UnresolvedFile, unit, context.Compilation, - (node, resolveResult) => - { - found = found || node != declaration; - }, context.CancellationToken); + referenceFinder.FindReferences (rootNode, variable, (node, resolveResult) => { + found |= node != declaration; + }); return found; } - internal abstract GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context, SyntaxTree unit); + internal abstract GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context); } } From 7b86a384dff2164450eb5f5b967b35fc64fd8372 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Fri, 14 Sep 2012 01:16:01 +0200 Subject: [PATCH 07/14] [CodeIssues] Optimize IncorrectExceptionParameterOrderingIssue. --- .../CodeIssues/IncorrectExceptionParameterOrderingIssue.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/IncorrectExceptionParameterOrderingIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/IncorrectExceptionParameterOrderingIssue.cs index 6a39b2de52..dc7ab72d0f 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/IncorrectExceptionParameterOrderingIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/IncorrectExceptionParameterOrderingIssue.cs @@ -58,9 +58,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring public override void VisitObjectCreateExpression(ObjectCreateExpression objectCreateExpression) { - var type = context.Resolve(objectCreateExpression.Type) as TypeResolveResult; - if (type == null) - return; var parameters = objectCreateExpression.Arguments; if (parameters.Count != 2) return; @@ -69,6 +66,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (firstParam == null || firstParam.Value.GetType() != typeof(string) || secondParam == null || firstParam.Value.GetType() != typeof(string)) return; + var type = context.Resolve(objectCreateExpression.Type) as TypeResolveResult; + if (type == null) + return; var leftLength = (firstParam.Value as string).Length; var rightLength = (secondParam.Value as string).Length; From ea1c7410f07bade338f5b489bccf8eeaa445eccf Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Fri, 14 Sep 2012 17:39:21 +0200 Subject: [PATCH 08/14] Speed up RedundantToStringIssue a bit. --- .../CodeIssues/RedundantToStringIssue.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs index 8d7df7b841..ef39599247 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantToStringIssue.cs @@ -64,7 +64,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring void CheckExpressionInAutoCallContext(Expression expression) { - if (expression is InvocationExpression) { + if (expression is InvocationExpression && !processedNodes.Contains(expression)) { CheckInvocationInAutoCallContext((InvocationExpression)expression); } } @@ -75,12 +75,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (memberExpression == null) { return; } - var resolveResult = ctx.Resolve(invocationExpression) as CSharpInvocationResolveResult; - if (resolveResult == null) { + if (memberExpression.MemberName != "ToString" || invocationExpression.Arguments.Any ()) { return; } - var member = resolveResult.Member; - if (member.Name != "ToString" || member.Parameters.Count != 0) { + + var resolveResult = ctx.Resolve(invocationExpression) as CSharpInvocationResolveResult; + if (resolveResult == null) { return; } AddRedundantToStringIssue(memberExpression, invocationExpression); @@ -88,9 +88,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring void AddRedundantToStringIssue(MemberReferenceExpression memberExpression, InvocationExpression invocationExpression) { - if (processedNodes.Contains(invocationExpression)) { - return; - } + // Simon Lindgren 2012-09-14: Previously there was a check here to see if the node had already been processed + // This has been moved out to the callers, to check it earlier for a 30-40% run time reduction processedNodes.Add(invocationExpression); AddIssue(memberExpression.DotToken.StartLocation, invocationExpression.RParToken.EndLocation, @@ -176,7 +175,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring void CheckTargetedObject(InvocationExpression invocationExpression, IType type, IMember member) { var memberExpression = invocationExpression.Target as MemberReferenceExpression; - if (memberExpression != null) { + if (memberExpression != null && !processedNodes.Contains(invocationExpression)) { if (type.IsKnownType(KnownTypeCode.String) && member.Name == "ToString") { AddRedundantToStringIssue(memberExpression, invocationExpression); } From db31f5f80dba025e0cf4b02b3c60b1f987f9d6f2 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Fri, 14 Sep 2012 17:42:17 +0200 Subject: [PATCH 09/14] Add fast check to discard most invocations early in FormatStringIssue. --- .../FormatStringIssues/FormatStringIssue.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs index ae7778b53f..a9bd959e22 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/FormatStringIssues/FormatStringIssue.cs @@ -54,6 +54,15 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring public override void VisitInvocationExpression(InvocationExpression invocationExpression) { base.VisitInvocationExpression(invocationExpression); + + // Speed up the inspector by discarding some invocations early + var hasEligibleArgument = invocationExpression.Arguments.Any(argument => { + var primitiveArg = argument as PrimitiveExpression; + return primitiveArg != null && primitiveArg.Value is string; + }); + if (!hasEligibleArgument) + return; + var invocationResolveResult = context.Resolve(invocationExpression) as CSharpInvocationResolveResult; if (invocationResolveResult == null) return; @@ -65,7 +74,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring return; } var primitiveArgument = formatArgument as PrimitiveExpression; - if (primitiveArgument == null || !(primitiveArgument.Value is String)) + if (primitiveArgument == null || !(primitiveArgument.Value is string)) return; var format = (string)primitiveArgument.Value; var parsingResult = context.ParseFormatString(format); From 2e21c6e2f06f65fc85b22cb3c11b6c3aa1cab579 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Fri, 14 Sep 2012 17:43:27 +0200 Subject: [PATCH 10/14] Add fast check to discard most invocations early in ReferenceEqualsCalledWithValueTypeIssue. --- .../CodeIssues/ReferenceEqualsCalledWithValueTypeIssue.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ReferenceEqualsCalledWithValueTypeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ReferenceEqualsCalledWithValueTypeIssue.cs index 24f6671fd5..f9d6571491 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ReferenceEqualsCalledWithValueTypeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ReferenceEqualsCalledWithValueTypeIssue.cs @@ -54,6 +54,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { base.VisitInvocationExpression (invocationExpression); + // Quickly determine if this invocation is eligible to speed up the inspector + var nameToken = invocationExpression.Target.GetChildByRole(Roles.Identifier); + if (nameToken.Name != "ReferenceEquals") + return; + var resolveResult = ctx.Resolve (invocationExpression) as InvocationResolveResult; if (resolveResult == null || resolveResult.Member.DeclaringTypeDefinition == null || From 6458118cebf664d4bdb25a3635c49a7d5b7b9ef8 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Fri, 14 Sep 2012 18:35:38 +0200 Subject: [PATCH 11/14] Replace FindReferences with LocalReferenceFinder in ForControlVariableNotModifiedIssue. --- .../ForControlVariableNotModifiedIssue.cs | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ForControlVariableNotModifiedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ForControlVariableNotModifiedIssue.cs index 78fa121783..cb259f433c 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ForControlVariableNotModifiedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ForControlVariableNotModifiedIssue.cs @@ -26,7 +26,6 @@ using System.Collections.Generic; using System.Linq; -using ICSharpCode.NRefactory.CSharp.Resolver; using ICSharpCode.NRefactory.PatternMatching; using ICSharpCode.NRefactory.Semantics; @@ -39,24 +38,19 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring IssueMarker = IssueMarker.Underline)] public class ForControlVariableNotModifiedIssue : ICodeIssueProvider { - static FindReferences refFinder = new FindReferences (); - public IEnumerable GetIssues (BaseRefactoringContext context) { - var unit = context.RootNode as SyntaxTree; - if (unit == null) - return Enumerable.Empty (); - - return new GatherVisitor (context, unit).GetIssues (); + return new GatherVisitor (context).GetIssues (); } class GatherVisitor : GatherVisitorBase { - SyntaxTree unit; - public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) + LocalReferenceFinder referenceFinder; + + public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { - this.unit = unit; + this.referenceFinder = new LocalReferenceFinder(ctx); } static VariableInitializer GetControlVariable(VariableDeclarationStatement variableDecl, @@ -112,7 +106,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring return; var modified = false; - refFinder.FindLocalReferences (localResolveResult.Variable, ctx.UnresolvedFile, unit, ctx.Compilation, + referenceFinder.FindReferences (forStatement, localResolveResult.Variable, (node, resolveResult) => { if (modified) @@ -129,7 +123,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var assignment = node.Parent as AssignmentExpression; modified = assignment != null && assignment.Left == node; - }, ctx.CancellationToken); + }); if (!modified) AddIssue (controlVariable.NameToken, From 6c2336b58225bff4daa496a9e88ecb6afd8063e9 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Sat, 15 Sep 2012 14:11:23 +0200 Subject: [PATCH 12/14] [CodeIssues] Add helper FindReferences(AstNode, IVariable) to BaseRefactoringContext. --- .../ICSharpCode.NRefactory.CSharp.csproj | 1 - .../Refactoring/BaseRefactoringContext.cs | 10 ++++ .../AccessToClosureIssue.cs | 18 ++---- .../ForControlVariableNotModifiedIssue.cs | 36 ++++++------ .../CodeIssues/RedundantAssignmentIssue.cs | 53 ++++++++--------- .../LocalVariableNotUsedIssue.cs | 30 +++++----- .../ParameterNotUsedIssue.cs | 20 +++---- .../VariableNotUsedIssue.cs | 54 ------------------ .../LocalVariableOnlyAssignedIssue.cs | 4 +- .../ParameterOnlyAssignedIssue.cs | 26 ++++----- .../VariableOnlyAssignedIssue.cs | 57 ++++++++++--------- .../Refactoring/LocalReferenceFinder.cs | 49 +++++++++++----- 12 files changed, 160 insertions(+), 198 deletions(-) delete mode 100644 ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/VariableNotUsedIssue.cs diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj index ccaca925cb..37925b3369 100644 --- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj +++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj @@ -324,7 +324,6 @@ - diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/BaseRefactoringContext.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/BaseRefactoringContext.cs index 32a84ba23e..6f4ccfc720 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/BaseRefactoringContext.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/BaseRefactoringContext.cs @@ -36,6 +36,7 @@ using ICSharpCode.NRefactory.Editor; using System.ComponentModel.Design; using ICSharpCode.NRefactory.CSharp.Analysis; using ICSharpCode.NRefactory.Utils; +using System.Collections.Generic; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -87,6 +88,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { this.resolver = resolver; this.cancellationToken = cancellationToken; + this.referenceFinder = new LocalReferenceFinder(resolver); } @@ -161,6 +163,14 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { return new CompositeFormatStringParser().Parse(source); } + + LocalReferenceFinder referenceFinder; + + public IList FindReferences(AstNode rootNode, IVariable variable) + { + return referenceFinder.FindReferences(rootNode, variable); + } + #endregion /// diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs index 9f108acfe9..d797d320dd 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/AccessToClosureIssues/AccessToClosureIssue.cs @@ -27,7 +27,6 @@ using System.Collections.Generic; using System.Linq; using ICSharpCode.NRefactory.CSharp.Analysis; -using ICSharpCode.NRefactory.CSharp.Resolver; using ICSharpCode.NRefactory.Semantics; using ICSharpCode.NRefactory.TypeSystem; @@ -82,7 +81,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring : base (context) { this.title = context.TranslateString (issueProvider.Title); - this.referenceFinder = new LocalReferenceFinder(context); this.issueProvider = issueProvider; } @@ -122,24 +120,18 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring base.VisitParameterDeclaration (parameterDeclaration); } - LocalReferenceFinder referenceFinder; - - void FindLocalReferences (AstNode rootNode, IVariable variable, FoundReferenceCallback callback) + void CheckVariable(IVariable variable, Statement env) { - referenceFinder.FindReferences(rootNode, variable, callback); - } - - void CheckVariable (IVariable variable, Statement env) - { - if (!issueProvider.IsTargetVariable (variable)) + if (!issueProvider.IsTargetVariable(variable)) return; var root = new Environment (env, env); var envLookup = new Dictionary (); envLookup [env] = root; - FindLocalReferences (env, variable, (astNode, resolveResult) => - AddNode (envLookup, new Node (astNode, issueProvider.GetNodeKind (astNode)))); + foreach (var result in ctx.FindReferences(env, variable)) { + AddNode(envLookup, new Node(result.Node, issueProvider.GetNodeKind(result.Node))); + } root.SortChildren (); CollectIssues (root, variable.Name); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ForControlVariableNotModifiedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ForControlVariableNotModifiedIssue.cs index cb259f433c..4f2da17b48 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ForControlVariableNotModifiedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ForControlVariableNotModifiedIssue.cs @@ -45,12 +45,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { - LocalReferenceFinder referenceFinder; - public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { - this.referenceFinder = new LocalReferenceFinder(ctx); } static VariableInitializer GetControlVariable(VariableDeclarationStatement variableDecl, @@ -105,25 +102,24 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (localResolveResult == null) return; + var results = ctx.FindReferences (forStatement, localResolveResult.Variable); var modified = false; - referenceFinder.FindReferences (forStatement, localResolveResult.Variable, - (node, resolveResult) => - { - if (modified) - return; - - var unary = node.Parent as UnaryOperatorExpression; - if (unary != null && unary.Expression == node) { - modified = unary.Operator == UnaryOperatorType.Decrement || - unary.Operator == UnaryOperatorType.PostDecrement || - unary.Operator == UnaryOperatorType.Increment || - unary.Operator == UnaryOperatorType.PostIncrement; - return; - } + foreach (var result in results) { + if (modified) + break; + var node = result.Node; + var unary = node.Parent as UnaryOperatorExpression; + if (unary != null && unary.Expression == node) { + modified = unary.Operator == UnaryOperatorType.Decrement || + unary.Operator == UnaryOperatorType.PostDecrement || + unary.Operator == UnaryOperatorType.Increment || + unary.Operator == UnaryOperatorType.PostIncrement; + continue; + } - var assignment = node.Parent as AssignmentExpression; - modified = assignment != null && assignment.Left == node; - }); + var assignment = node.Parent as AssignmentExpression; + modified = assignment != null && assignment.Left == node; + } if (!modified) AddIssue (controlVariable.NameToken, diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantAssignmentIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantAssignmentIssue.cs index ebfcee99b2..96d56b23bd 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantAssignmentIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantAssignmentIssue.cs @@ -26,7 +26,6 @@ using System.Collections.Generic; using System.Linq; -using ICSharpCode.NRefactory.CSharp.Resolver; using ICSharpCode.NRefactory.Semantics; namespace ICSharpCode.NRefactory.CSharp.Refactoring @@ -48,12 +47,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { - LocalReferenceFinder referenceFinder; - public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) : base (ctx) { - referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitParameterDeclaration (ParameterDeclaration parameterDeclaration) @@ -91,30 +87,35 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var references = new HashSet (); var refStatements = new HashSet (); var usedInLambda = false; - referenceFinder.FindReferences (rootStatement, resolveResult.Variable, (astNode, rr) => { - if (usedInLambda || astNode == variableDecl) - return; - - var parent = astNode.Parent; - while (!(parent == null || parent is Statement || parent is LambdaExpression)) - parent = parent.Parent; - if (parent == null) - return; - - var statement = parent as Statement; - if (statement != null) { - references.Add (astNode); - refStatements.Add (statement); - } + var results = ctx.FindReferences (rootStatement, resolveResult.Variable); + foreach (var result in results) { + var node = result.Node; + if (node == variableDecl) + continue; + + var parent = node.Parent; + while (!(parent == null || parent is Statement || parent is LambdaExpression)) + parent = parent.Parent; + if (parent == null) + continue; + + var statement = parent as Statement; + if (statement != null) { + references.Add (node); + refStatements.Add (statement); + } - while (parent != null && parent != rootStatement) { - if (parent is LambdaExpression || parent is AnonymousMethodExpression) { - usedInLambda = true; - break; - } - parent = parent.Parent; + while (parent != null && parent != rootStatement) { + if (parent is LambdaExpression || parent is AnonymousMethodExpression) { + usedInLambda = true; + break; } - }); + parent = parent.Parent; + } + if (usedInLambda) { + break; + } + } // stop analyzing if the variable is used in any lambda expression or anonymous method if (usedInLambda) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/LocalVariableNotUsedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/LocalVariableNotUsedIssue.cs index c6900afc93..4eebbc615a 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/LocalVariableNotUsedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/LocalVariableNotUsedIssue.cs @@ -25,6 +25,8 @@ // THE SOFTWARE. using ICSharpCode.NRefactory.Semantics; +using System.Linq; +using ICSharpCode.NRefactory.TypeSystem; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -33,24 +35,22 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring Category = IssueCategories.Redundancies, Severity = Severity.Warning, IssueMarker = IssueMarker.GrayOut)] - public class LocalVariableNotUsedIssue : VariableNotUsedIssue + public class LocalVariableNotUsedIssue : ICodeIssueProvider { - internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context) + #region ICodeIssueProvider implementation + + public System.Collections.Generic.IEnumerable GetIssues(BaseRefactoringContext context) { - return new GatherVisitor (context, this); + return new GatherVisitor (context).GetIssues (); } + #endregion + class GatherVisitor : GatherVisitorBase { - LocalReferenceFinder referenceFinder; - - VariableNotUsedIssue inspector; - - public GatherVisitor (BaseRefactoringContext ctx, VariableNotUsedIssue inspector) + public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { - this.inspector = inspector; - this.referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitVariableInitializer (VariableInitializer variableInitializer) @@ -68,8 +68,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (resolveResult == null) return; - var b = inspector.FindUsage(referenceFinder, decl.Parent, resolveResult.Variable, variableInitializer); - if (b) + if (IsUsed (decl.Parent, resolveResult.Variable, variableInitializer)) return; AddIssue (variableInitializer.NameToken, ctx.TranslateString ("Remove unused local variable"), @@ -94,11 +93,16 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (resolveResult == null) return; - if (inspector.FindUsage (referenceFinder, foreachStatement, resolveResult.Variable, foreachStatement.VariableNameToken)) + if (IsUsed (foreachStatement, resolveResult.Variable, foreachStatement.VariableNameToken)) return; AddIssue (foreachStatement.VariableNameToken, ctx.TranslateString ("Local variable is never used")); } + + bool IsUsed(AstNode rootNode, IVariable variable, AstNode variableNode) + { + return ctx.FindReferences(rootNode, variable).Any(result => result.Node != variableNode); + } } } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/ParameterNotUsedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/ParameterNotUsedIssue.cs index 6cf5bfbc5a..cb9a51d76d 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/ParameterNotUsedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/ParameterNotUsedIssue.cs @@ -27,6 +27,7 @@ using ICSharpCode.NRefactory.Semantics; using System.Linq; using ICSharpCode.NRefactory.TypeSystem; +using System.Collections.Generic; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -35,25 +36,20 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring Category = IssueCategories.Redundancies, Severity = Severity.Warning, IssueMarker = IssueMarker.GrayOut)] - public class ParameterNotUsedIssue : VariableNotUsedIssue + public class ParameterNotUsedIssue : ICodeIssueProvider { - - internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context) + #region ICodeIssueProvider implementation + public IEnumerable GetIssues(BaseRefactoringContext context) { - return new GatherVisitor (context, this); + return new GatherVisitor (context).GetIssues (); } + #endregion class GatherVisitor : GatherVisitorBase { - VariableNotUsedIssue inspector; - - LocalReferenceFinder referenceFinder; - - public GatherVisitor (BaseRefactoringContext ctx, VariableNotUsedIssue inspector) + public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { - this.inspector = inspector; - this.referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitMethodDeclaration(MethodDeclaration methodDeclaration) @@ -85,7 +81,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (resolveResult == null) return; - if (inspector.FindUsage (referenceFinder, parameterDeclaration.Parent, resolveResult.Variable, parameterDeclaration)) + if (ctx.FindReferences (parameterDeclaration.Parent, resolveResult.Variable).Any(r => r.Node != parameterDeclaration)) return; AddIssue (parameterDeclaration.NameToken, ctx.TranslateString ("Parameter is never used")); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/VariableNotUsedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/VariableNotUsedIssue.cs deleted file mode 100644 index 9e33c81bd6..0000000000 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableNotUsedIssues/VariableNotUsedIssue.cs +++ /dev/null @@ -1,54 +0,0 @@ -// -// VariableNotUsedIssue.cs -// -// Author: -// Mansheng Yang -// -// Copyright (c) 2012 Mansheng Yang -// -// 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.Collections.Generic; -using System.Linq; -using ICSharpCode.NRefactory.TypeSystem; - -namespace ICSharpCode.NRefactory.CSharp.Refactoring -{ - public abstract class VariableNotUsedIssue : ICodeIssueProvider - { - public IEnumerable GetIssues (BaseRefactoringContext context) - { - var unit = context.RootNode as SyntaxTree; - if (unit == null) - return Enumerable.Empty (); - return GetGatherVisitor(context).GetIssues (); - } - - public bool FindUsage (LocalReferenceFinder referenceFinder, AstNode rootNode, IVariable variable, AstNode declaration) - { - var found = false; - referenceFinder.FindReferences (rootNode, variable, (node, resolveResult) => { - found |= node != declaration; - }); - return found; - } - - internal abstract GatherVisitorBase GetGatherVisitor (BaseRefactoringContext context); - } -} diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/LocalVariableOnlyAssignedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/LocalVariableOnlyAssignedIssue.cs index 0e6eacbc1f..d30d245654 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/LocalVariableOnlyAssignedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/LocalVariableOnlyAssignedIssue.cs @@ -43,11 +43,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { - LocalReferenceFinder referenceFinder; public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { - referenceFinder = new LocalReferenceFinder(ctx); } public override void VisitVariableInitializer (VariableInitializer variableInitializer) @@ -61,7 +59,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var resolveResult = ctx.Resolve (variableInitializer) as LocalResolveResult; if (resolveResult == null) return; - if (!TestOnlyAssigned (referenceFinder, decl.Parent, resolveResult.Variable)) + if (!TestOnlyAssigned (ctx, decl.Parent, resolveResult.Variable)) return; AddIssue (variableInitializer.NameToken, ctx.TranslateString ("Local variable is assigned by its value is never used")); diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/ParameterOnlyAssignedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/ParameterOnlyAssignedIssue.cs index 4a8abe3732..0d5ab0c8da 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/ParameterOnlyAssignedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/ParameterOnlyAssignedIssue.cs @@ -23,7 +23,6 @@ // 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 ICSharpCode.NRefactory.Semantics; namespace ICSharpCode.NRefactory.CSharp.Refactoring @@ -35,32 +34,33 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring IssueMarker = IssueMarker.Underline)] public class ParameterOnlyAssignedIssue : VariableOnlyAssignedIssue { - internal override GatherVisitorBase GetGatherVisitor (BaseRefactoringContext ctx) + internal override GatherVisitorBase GetGatherVisitor(BaseRefactoringContext ctx) { - return new GatherVisitor (ctx); + return new GatherVisitor(ctx); } private class GatherVisitor : GatherVisitorBase { - LocalReferenceFinder referenceFinder; - public GatherVisitor (BaseRefactoringContext ctx) + public GatherVisitor(BaseRefactoringContext ctx) : base (ctx) { - referenceFinder = new LocalReferenceFinder(ctx); } - public override void VisitParameterDeclaration (ParameterDeclaration parameterDeclaration) + public override void VisitParameterDeclaration(ParameterDeclaration parameterDeclaration) { - base.VisitParameterDeclaration (parameterDeclaration); + base.VisitParameterDeclaration(parameterDeclaration); - var resolveResult = ctx.Resolve (parameterDeclaration) as LocalResolveResult; + var resolveResult = ctx.Resolve(parameterDeclaration) as LocalResolveResult; if (resolveResult == null) return; - if (parameterDeclaration.ParameterModifier == ParameterModifier.Out || parameterDeclaration.ParameterModifier == ParameterModifier.Ref - || !TestOnlyAssigned (referenceFinder, parameterDeclaration.Parent, resolveResult.Variable)) + + var parameterModifier = parameterDeclaration.ParameterModifier; + if (parameterModifier == ParameterModifier.Out || parameterModifier == ParameterModifier.Ref || + !TestOnlyAssigned(ctx, parameterDeclaration.Parent, resolveResult.Variable)) { return; - AddIssue (parameterDeclaration.NameToken, - ctx.TranslateString ("Parameter is assigned by its value is never used")); + } + AddIssue(parameterDeclaration.NameToken, + ctx.TranslateString("Parameter is assigned by its value is never used")); } } } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/VariableOnlyAssignedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/VariableOnlyAssignedIssue.cs index d86c334959..6c6d6820c2 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/VariableOnlyAssignedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/VariableOnlyAssignedIssues/VariableOnlyAssignedIssue.cs @@ -36,46 +36,47 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring return GetGatherVisitor (context).GetIssues (); } - protected static bool TestOnlyAssigned (LocalReferenceFinder referenceFinder, AstNode rootNode, IVariable variable) + protected static bool TestOnlyAssigned(BaseRefactoringContext ctx, AstNode rootNode, IVariable variable) { var assignment = false; var nonAssignment = false; - referenceFinder.FindReferences (rootNode, variable, (node, resolveResult) => { - if (node is ParameterDeclaration) - return; + foreach (var result in ctx.FindReferences(rootNode, variable)) { + var node = result.Node; + if (node is ParameterDeclaration) + continue; - if (node is VariableInitializer) { - if (!(node as VariableInitializer).Initializer.IsNull) - assignment = true; - return; - } + if (node is VariableInitializer) { + if (!(node as VariableInitializer).Initializer.IsNull) + assignment = true; + continue; + } - if (node is IdentifierExpression) { - var parent = node.Parent; - if (parent is AssignmentExpression) { - if (((AssignmentExpression)parent).Left == node) { - assignment = true; - return; - } - } else if (parent is UnaryOperatorExpression) { - var op = ((UnaryOperatorExpression)parent).Operator; - switch (op) { + if (node is IdentifierExpression) { + var parent = node.Parent; + if (parent is AssignmentExpression) { + if (((AssignmentExpression)parent).Left == node) { + assignment = true; + continue; + } + } else if (parent is UnaryOperatorExpression) { + var op = ((UnaryOperatorExpression)parent).Operator; + switch (op) { case UnaryOperatorType.Increment: case UnaryOperatorType.PostIncrement: case UnaryOperatorType.Decrement: case UnaryOperatorType.PostDecrement: assignment = true; - return; - } - } else if (parent is DirectionExpression) { - if (((DirectionExpression)parent).FieldDirection == FieldDirection.Out) { - assignment = true; - return; - } + continue; + } + } else if (parent is DirectionExpression) { + if (((DirectionExpression)parent).FieldDirection == FieldDirection.Out) { + assignment = true; + continue; } } - nonAssignment = true; - }); + } + nonAssignment = true; + } return assignment && !nonAssignment; } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/LocalReferenceFinder.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/LocalReferenceFinder.cs index 04de48afab..0cde50ada6 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/LocalReferenceFinder.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/LocalReferenceFinder.cs @@ -29,7 +29,6 @@ using System.Linq; using ICSharpCode.NRefactory.CSharp.Resolver; using ICSharpCode.NRefactory.Semantics; using ICSharpCode.NRefactory.TypeSystem; -using System; using ICSharpCode.NRefactory.Utils; using System.Diagnostics; @@ -48,7 +47,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { LocalReferenceLocator locator; - MultiDictionary> references = new MultiDictionary>(); + MultiDictionary references = new MultiDictionary(); + + HashSet visitedRoots = new HashSet(); public LocalReferenceFinder(CSharpAstResolver resolver) { @@ -59,8 +60,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { } - HashSet visitedRoots = new HashSet(); - void VisitIfNeccessary(AstNode rootNode) { // If any of the parent nodes are recorded as visited, @@ -72,8 +71,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring tmpRoot = tmpRoot.Parent; } - rootNode.AcceptVisitor(locator); - visitedRoots.Add(rootNode); + locator.ProccessRoot (rootNode); } /// @@ -93,15 +91,15 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring /// searches, which references outside of are /// or are not reported is undefined. /// - public void FindReferences (AstNode rootNode, IVariable variable, FoundReferenceCallback action) + public IList FindReferences(AstNode rootNode, IVariable variable) { - VisitIfNeccessary(rootNode); - var lookup = (ILookup>)references; - if (!lookup.Contains(variable)) - return; - var iList = references[variable]; - foreach (var reference in iList) { - action(reference.Item1, reference.Item2); + lock (locator) { + VisitIfNeccessary(rootNode); + var lookup = (ILookup)references; + if (!lookup.Contains(variable)) + return new List(); + // Clone the list for thread safety + return references[variable].ToList(); } } @@ -119,11 +117,19 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring IList processedVariables = new List(); + public void ProccessRoot (AstNode rootNode) + { + rootNode.AcceptVisitor(this); + referenceFinder.visitedRoots.Add(rootNode); + } + protected override void VisitChildren(AstNode node) { + if (referenceFinder.visitedRoots.Contains(node)) + return; var localResolveResult = resolver.Resolve(node) as LocalResolveResult; if (localResolveResult != null && !processedVariables.Contains(localResolveResult.Variable)) { - referenceFinder.references.Add(localResolveResult.Variable, Tuple.Create(node, localResolveResult)); + referenceFinder.references.Add(localResolveResult.Variable, new ReferenceResult(node, localResolveResult)); processedVariables.Add(localResolveResult.Variable); base.VisitChildren(node); @@ -135,4 +141,17 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring } } } + + public class ReferenceResult + { + public ReferenceResult (AstNode node, LocalResolveResult resolveResult) + { + Node = node; + ResolveResult = resolveResult; + } + + public AstNode Node { get; private set; } + + public LocalResolveResult ResolveResult { get; private set; } + } } From 1726ac4fcd952b50574be682deaf14deb734e09f Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Sat, 15 Sep 2012 14:12:19 +0200 Subject: [PATCH 13/14] Replace FindReferences with context.FindReferences in ValueParameterUnusedIssue. --- .../CodeIssues/ValueParameterUnusedIssue.cs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ValueParameterUnusedIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ValueParameterUnusedIssue.cs index 6a35677b6a..8d0412c412 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ValueParameterUnusedIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ValueParameterUnusedIssue.cs @@ -45,13 +45,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { - readonly BaseRefactoringContext context; - - readonly FindReferences findRef = new FindReferences(); - public GatherVisitor(BaseRefactoringContext context, ValueParameterUnusedIssue inspector) : base (context) { - this.context = context; } public override void VisitIndexerDeclaration(IndexerDeclaration indexerDeclaration) @@ -77,22 +72,22 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (!IsEligible(body)) return; - var localResolveResult = context.GetResolverStateBefore(body) + var localResolveResult = ctx.GetResolverStateBefore(body) .LookupSimpleNameOrTypeName("value", new List(), NameLookupMode.Expression) as LocalResolveResult; if (localResolveResult == null) return; - var variable = localResolveResult.Variable; bool referenceFound = false; - var syntaxTree = (SyntaxTree)context.RootNode; - findRef.FindLocalReferences(variable, context.UnresolvedFile, syntaxTree, context.Compilation, (n, entity) => { - if (n.StartLocation >= body.StartLocation && n.EndLocation <= body.EndLocation) { + foreach (var result in ctx.FindReferences (body, localResolveResult.Variable)) { + var node = result.Node; + if (node.StartLocation >= body.StartLocation && node.EndLocation <= body.EndLocation) { referenceFound = true; + break; } - }, CancellationToken.None); + } if(!referenceFound) - AddIssue(anchor, context.TranslateString("The " + accessorName + " does not use the 'value' parameter")); + AddIssue(anchor, ctx.TranslateString("The " + accessorName + " does not use the 'value' parameter")); } static bool IsEligible(BlockStatement body) From cdf2e04c816b6ee55b3aadccde76935bd86c03e8 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Sat, 15 Sep 2012 14:12:44 +0200 Subject: [PATCH 14/14] Replace FindReferences with context.FindReferences in MultipleEnumerationIssue. --- .../CodeIssues/MultipleEnumerationIssue.cs | 68 ++++++++----------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/MultipleEnumerationIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/MultipleEnumerationIssue.cs index 5312964071..9ab54cd39e 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/MultipleEnumerationIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/MultipleEnumerationIssue.cs @@ -42,11 +42,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { public IEnumerable GetIssues (BaseRefactoringContext context) { - var unit = context.RootNode as SyntaxTree; - if (unit == null) - return Enumerable.Empty (); - - return new GatherVisitor (context, unit).GetIssues (); + return new GatherVisitor (context).GetIssues (); } class AnalysisStatementCollector : DepthFirstAstVisitor @@ -110,15 +106,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { - static FindReferences refFinder = new FindReferences (); - - SyntaxTree unit; HashSet collectedAstNodes; - public GatherVisitor (BaseRefactoringContext ctx, SyntaxTree unit) + public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { - this.unit = unit; this.collectedAstNodes = new HashSet (); } @@ -139,7 +131,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring base.VisitParameterDeclaration (parameterDeclaration); var resolveResult = ctx.Resolve (parameterDeclaration) as LocalResolveResult; - CollectIssues (parameterDeclaration, resolveResult); + CollectIssues (parameterDeclaration, parameterDeclaration.Parent, resolveResult); } public override void VisitVariableInitializer (VariableInitializer variableInitializer) @@ -147,7 +139,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring base.VisitVariableInitializer (variableInitializer); var resolveResult = ctx.Resolve (variableInitializer) as LocalResolveResult; - CollectIssues (variableInitializer, resolveResult); + CollectIssues (variableInitializer, variableInitializer.Parent.Parent, resolveResult); } static bool IsAssignment (AstNode node) @@ -200,42 +192,42 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring HashSet collectedNodes; Dictionary nodeDegree; // number of enumerations a node can reach - void FindReferences (AstNode variableDecl, IVariable variable) + void FindReferences (AstNode variableDecl, AstNode rootNode, IVariable variable) { references = new HashSet (); refStatements = new HashSet (); lambdaExpressions = new HashSet (); - refFinder.FindLocalReferences (variable, ctx.UnresolvedFile, unit, ctx.Compilation, - (astNode, resolveResult) => { - if (astNode == variableDecl) - return; - - var parent = astNode.Parent; - while (!(parent == null || parent is Statement || parent is LambdaExpression)) - parent = parent.Parent; - if (parent == null) - return; - - // lambda expression with expression body, should be analyzed separately - var expr = parent as LambdaExpression; - if (expr != null) { - if (IsAssignment (astNode) || IsEnumeration (astNode)) { - references.Add (astNode); - lambdaExpressions.Add (expr); - } - return; - } + foreach (var result in ctx.FindReferences (rootNode, variable)) { + var astNode = result.Node; + if (astNode == variableDecl) + continue; - var statement = (Statement)parent; + var parent = astNode.Parent; + while (!(parent == null || parent is Statement || parent is LambdaExpression)) + parent = parent.Parent; + if (parent == null) + continue; + + // lambda expression with expression body, should be analyzed separately + var expr = parent as LambdaExpression; + if (expr != null) { if (IsAssignment (astNode) || IsEnumeration (astNode)) { references.Add (astNode); - refStatements.Add (statement); + lambdaExpressions.Add (expr); } - }, ctx.CancellationToken); + continue; + } + + if (IsAssignment (astNode) || IsEnumeration (astNode)) { + references.Add (astNode); + var statement = (Statement)parent; + refStatements.Add (statement); + } + } } - void CollectIssues (AstNode variableDecl, LocalResolveResult resolveResult) + void CollectIssues (AstNode variableDecl, AstNode rootNode, LocalResolveResult resolveResult) { if (resolveResult == null) return; @@ -246,7 +238,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring typeDef.KnownTypeCode != KnownTypeCode.IEnumerableOfT)) return; - FindReferences (variableDecl, resolveResult.Variable); + FindReferences (variableDecl, rootNode, resolveResult.Variable); var statements = AnalysisStatementCollector.Collect (variableDecl); foreach (var statement in statements) {