Skip to content

Commit 4ee7f82

Browse files
Hosch250Hosch250
authored
Merge pull request #2124 from Hosch250/Issue2097
Fix memory leak in Parameter Not Used inspection
2 parents fd88129 + 5d320de commit 4ee7f82

File tree

3 files changed

+29
-25
lines changed

3 files changed

+29
-25
lines changed

‎RetailCoder.VBE/Inspections/ParameterNotUsedInspection.cs‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,19 @@
11
using System.Collections.Generic;
22
using System.Linq;
3-
using Microsoft.Vbe.Interop;
43
using Rubberduck.Common;
54
using Rubberduck.Parsing.Symbols;
65
using Rubberduck.Parsing.VBA;
7-
using Rubberduck.Refactorings.RemoveParameters;
86
using Rubberduck.UI;
9-
using Rubberduck.UI.Refactorings;
107

118
namespace Rubberduck.Inspections
129
{
1310
public sealed class ParameterNotUsedInspection : InspectionBase
1411
{
15-
private readonly VBE _vbe;
1612
private readonly IMessageBox _messageBox;
1713

18-
public ParameterNotUsedInspection(VBE vbe, RubberduckParserState state, IMessageBox messageBox)
14+
public ParameterNotUsedInspection(RubberduckParserState state, IMessageBox messageBox)
1915
: base(state)
2016
{
21-
_vbe = vbe;
2217
_messageBox = messageBox;
2318
}
2419

@@ -41,16 +36,14 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
4136
&& parameter.ParentDeclaration.DeclarationType != DeclarationType.LibraryProcedure);
4237

4338
var unused = parameters.Where(parameter => !parameter.References.Any()).ToList();
44-
var quickFixRefactoring =
45-
new RemoveParametersRefactoring(_vbe, new RemoveParametersPresenterFactory(_vbe, new RemoveParametersDialog(), State, _messageBox));
4639

4740
var issues = from issue in unused.Where(parameter =>
4841
!IsInterfaceMemberParameter(parameter, interfaceMemberScopes)
4942
&& !builtInHandlers.Contains(parameter.ParentDeclaration))
5043
let isInterfaceImplementationMember = IsInterfaceMemberImplementationParameter(issue, interfaceImplementationMemberScopes)
5144
select new ParameterNotUsedInspectionResult(this, issue,
5245
((dynamic) issue.Context).unrestrictedIdentifier(), issue.QualifiedName,
53-
isInterfaceImplementationMember, quickFixRefactoring, State);
46+
isInterfaceImplementationMember, issue.Project.VBE, State, _messageBox);
5447

5548
return issues.ToList();
5649
}

‎RetailCoder.VBE/Inspections/ParameterNotUsedInspectionResult.cs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
using Rubberduck.Parsing.VBA;
55
using Rubberduck.Refactorings.RemoveParameters;
66
using Rubberduck.VBEditor;
7+
using Microsoft.Vbe.Interop;
8+
using Rubberduck.UI;
9+
using Rubberduck.UI.Refactorings;
710

811
namespace Rubberduck.Inspections
912
{
@@ -13,12 +16,12 @@ public class ParameterNotUsedInspectionResult : InspectionResultBase
1316

1417
public ParameterNotUsedInspectionResult(IInspection inspection, Declaration target,
1518
ParserRuleContext context, QualifiedMemberName qualifiedName, bool isInterfaceImplementation,
16-
RemoveParametersRefactoring refactoring, RubberduckParserState state)
19+
VBE vbe, RubberduckParserState state, IMessageBox messageBox)
1720
: base(inspection, qualifiedName.QualifiedModuleName, context, target)
1821
{
1922
_quickFixes = isInterfaceImplementation ? new CodeInspectionQuickFix[] {} : new CodeInspectionQuickFix[]
2023
{
21-
new RemoveUnusedParameterQuickFix(Context, QualifiedSelection, refactoring, state),
24+
new RemoveUnusedParameterQuickFix(Context, QualifiedSelection, vbe, state, messageBox),
2225
new IgnoreOnceQuickFix(Context, QualifiedSelection, Inspection.AnnotationName),
2326
};
2427
}
@@ -33,20 +36,28 @@ public override string Description
3336

3437
public class RemoveUnusedParameterQuickFix : CodeInspectionQuickFix
3538
{
36-
private readonly RemoveParametersRefactoring _quickFixRefactoring;
39+
private readonly VBE _vbe;
3740
private readonly RubberduckParserState _state;
41+
private readonly IMessageBox _messageBox;
3842

3943
public RemoveUnusedParameterQuickFix(ParserRuleContext context, QualifiedSelection selection,
40-
RemoveParametersRefactoring quickFixRefactoring, RubberduckParserState state)
44+
VBE vbe, RubberduckParserState state, IMessageBox messageBox)
4145
: base(context, selection, InspectionsUI.RemoveUnusedParameterQuickFix)
4246
{
43-
_quickFixRefactoring = quickFixRefactoring;
47+
_vbe = vbe;
4448
_state = state;
49+
_messageBox = messageBox;
4550
}
4651

4752
public override void Fix()
4853
{
49-
_quickFixRefactoring.QuickFix(_state, Selection);
54+
using (var dialog = new RemoveParametersDialog())
55+
{
56+
var refactoring = new RemoveParametersRefactoring(_vbe,
57+
new RemoveParametersPresenterFactory(_vbe, dialog, _state, _messageBox));
58+
59+
refactoring.QuickFix(_state, Selection);
60+
}
5061
}
5162
}
5263
}

‎RubberduckTests/Inspections/ParameterNotUsedInspectionTests.cs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public void ParameterNotUsed_ReturnsResult()
3434
parser.Parse(new CancellationTokenSource());
3535
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
3636

37-
var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
37+
var inspection = new ParameterNotUsedInspection(parser.State, null);
3838
var inspectionResults = inspection.GetInspectionResults();
3939

4040
Assert.AreEqual(1, inspectionResults.Count());
@@ -62,7 +62,7 @@ Private Sub Goo(ByVal arg1 as Integer)
6262
parser.Parse(new CancellationTokenSource());
6363
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
6464

65-
var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
65+
var inspection = new ParameterNotUsedInspection(parser.State, null);
6666
var inspectionResults = inspection.GetInspectionResults();
6767

6868
Assert.AreEqual(2, inspectionResults.Count());
@@ -88,7 +88,7 @@ public void ParameterUsed_DoesNotReturnResult()
8888
parser.Parse(new CancellationTokenSource());
8989
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
9090

91-
var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
91+
var inspection = new ParameterNotUsedInspection(parser.State, null);
9292
var inspectionResults = inspection.GetInspectionResults();
9393

9494
Assert.AreEqual(0, inspectionResults.Count());
@@ -114,7 +114,7 @@ public void ParameterNotUsed_ReturnsResult_SomeParamsUsed()
114114
parser.Parse(new CancellationTokenSource());
115115
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
116116

117-
var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
117+
var inspection = new ParameterNotUsedInspection(parser.State, null);
118118
var inspectionResults = inspection.GetInspectionResults();
119119

120120
Assert.AreEqual(1, inspectionResults.Count());
@@ -149,7 +149,7 @@ Private Sub IClass1_DoSomething(ByVal a As Integer)
149149
parser.Parse(new CancellationTokenSource());
150150
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
151151

152-
var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
152+
var inspection = new ParameterNotUsedInspection(parser.State, null);
153153
var inspectionResults = inspection.GetInspectionResults().ToList();
154154

155155
Assert.AreEqual(1, inspectionResults.Count);
@@ -176,7 +176,7 @@ Private Sub Foo(ByVal arg1 as Integer)
176176
parser.Parse(new CancellationTokenSource());
177177
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
178178

179-
var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
179+
var inspection = new ParameterNotUsedInspection(parser.State, null);
180180
var inspectionResults = inspection.GetInspectionResults();
181181

182182
Assert.IsFalse(inspectionResults.Any());
@@ -207,7 +207,7 @@ public void GivenPrivateSub_DefaultQuickFixRemovesParameter()
207207
parser.Parse(new CancellationTokenSource());
208208
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
209209

210-
var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
210+
var inspection = new ParameterNotUsedInspection(parser.State, null);
211211
var inspectionResults = inspection.GetInspectionResults();
212212

213213
inspectionResults.First().QuickFixes.First().Fix();
@@ -241,7 +241,7 @@ Private Sub Foo(ByVal arg1 as Integer)
241241
parser.Parse(new CancellationTokenSource());
242242
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
243243

244-
var inspection = new ParameterNotUsedInspection(vbe.Object, parser.State, null);
244+
var inspection = new ParameterNotUsedInspection(parser.State, null);
245245
var inspectionResults = inspection.GetInspectionResults();
246246

247247
inspectionResults.First().QuickFixes.Single(s => s is IgnoreOnceQuickFix).Fix();
@@ -253,7 +253,7 @@ Private Sub Foo(ByVal arg1 as Integer)
253253
[TestCategory("Inspections")]
254254
public void InspectionType()
255255
{
256-
var inspection = new ParameterNotUsedInspection(null, null, null);
256+
var inspection = new ParameterNotUsedInspection(null, null);
257257
Assert.AreEqual(CodeInspectionType.CodeQualityIssues, inspection.InspectionType);
258258
}
259259

@@ -262,7 +262,7 @@ public void InspectionType()
262262
public void InspectionName()
263263
{
264264
const string inspectionName = "ParameterNotUsedInspection";
265-
var inspection = new ParameterNotUsedInspection(null, null, null);
265+
var inspection = new ParameterNotUsedInspection(null, null);
266266

267267
Assert.AreEqual(inspectionName, inspection.Name);
268268
}

0 commit comments

Comments
 (0)