Skip to content

Commit 0b86dd3

Browse files
retailcoderretailcoder
authored
Merge pull request #5647 from BZngr/5646_ImplicitTypeArrayFix
Fix "Variable is implicitly Variant" quickfix results for array declarations
2 parents 23c7767 + 01de2f1 commit 0b86dd3

File tree

7 files changed

+234
-77
lines changed

7 files changed

+234
-77
lines changed

‎Rubberduck.CodeAnalysis/QuickFixes/Concrete/DeclareAsExplicitVariantQuickFix.cs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
using Rubberduck.CodeAnalysis.QuickFixes.Abstract;
55
using Rubberduck.Parsing.Grammar;
66
using Rubberduck.Parsing.Rewriter;
7+
using Rubberduck.Refactorings;
8+
using Rubberduck.Refactorings.ImplicitTypeToExplicit;
79

810
namespace Rubberduck.CodeAnalysis.QuickFixes.Concrete
911
{
@@ -36,19 +38,21 @@ namespace Rubberduck.CodeAnalysis.QuickFixes.Concrete
3638
/// </example>
3739
internal sealed class DeclareAsExplicitVariantQuickFix : QuickFixBase
3840
{
39-
public DeclareAsExplicitVariantQuickFix()
41+
private readonly ICodeOnlyRefactoringAction<ImplicitTypeToExplicitModel> _refactoring;
42+
public DeclareAsExplicitVariantQuickFix(ImplicitTypeToExplicitRefactoringAction refactoringAction)
4043
: base(typeof(VariableTypeNotDeclaredInspection))
41-
{}
44+
{
45+
_refactoring = refactoringAction;
46+
}
4247

4348
public override void Fix(IInspectionResult result, IRewriteSession rewriteSession)
4449
{
45-
var rewriter = rewriteSession.CheckOutModuleRewriter(result.Target.QualifiedModuleName);
50+
var model = new ImplicitTypeToExplicitModel(result.Target)
51+
{
52+
ForceVariantAsType = true
53+
};
4654

47-
ParserRuleContext identifierNode =
48-
result.Context is VBAParser.VariableSubStmtContext || result.Context is VBAParser.ConstSubStmtContext
49-
? result.Context.children[0]
50-
: ((dynamic) result.Context).unrestrictedIdentifier();
51-
rewriter.InsertAfter(identifierNode.Stop.TokenIndex, " As Variant");
55+
_refactoring.Refactor(model, rewriteSession);
5256
}
5357

5458
public override string Description(IInspectionResult result) => Resources.Inspections.QuickFixes.DeclareAsExplicitVariantQuickFix;

‎Rubberduck.Refactorings/ImplicitTypeToExplicit/ImplicitTypeToExplicitModel.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,7 @@ public ImplicitTypeToExplicitModel(Declaration target)
1010
}
1111

1212
public Declaration Target { get; }
13+
14+
public bool ForceVariantAsType { set; get; }
1315
}
1416
}

‎Rubberduck.Refactorings/ImplicitTypeToExplicit/ImplicitTypeToExplicitRefactoringAction.cs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System.Collections.Generic;
1+
using System;
2+
using System.Collections.Generic;
23
using System.Linq;
34
using Antlr4.Runtime;
45
using Rubberduck.Parsing;
@@ -26,13 +27,29 @@ public ImplicitTypeToExplicitRefactoringAction(
2627

2728
public override void Refactor(ImplicitTypeToExplicitModel model, IRewriteSession rewriteSession)
2829
{
29-
var identifierContext = model.Target.Context.GetDescendent<VBAParser.IdentifierContext>();
30+
if (!(model.Target.Context is VBAParser.VariableSubStmtContext
31+
|| model.Target.Context is VBAParser.ConstSubStmtContext
32+
|| model.Target.Context is VBAParser.ArgContext))
33+
{
34+
throw new ArgumentException($"Invalid target {model.Target.IdentifierName}");
35+
}
36+
37+
var identifierNode = model.Target.Context.GetChild<VBAParser.IdentifierContext>()
38+
?? model.Target.Context.GetChild<VBAParser.UnrestrictedIdentifierContext>() as ParserRuleContext;
3039

31-
var resolver = new ImplicitAsTypeNameResolver(_declarationFinderProvider, _parseTreeValueFactory, model.Target);
32-
var asTypeName = InferAsTypeNameForInspectionResult(model.Target, resolver, new AsTypeNamesResultsHandler());
40+
var insertAfterTarget = model.Target.IsArray
41+
? model.Target.Context.Stop.TokenIndex
42+
: identifierNode.Stop.TokenIndex;
43+
44+
var asTypeName = Tokens.Variant;
45+
if (!model.ForceVariantAsType)
46+
{
47+
var resolver = new ImplicitAsTypeNameResolver(_declarationFinderProvider, _parseTreeValueFactory, model.Target);
48+
asTypeName = InferAsTypeNameForInspectionResult(model.Target, resolver, new AsTypeNamesResultsHandler());
49+
}
3350

3451
var rewriter = rewriteSession.CheckOutModuleRewriter(model.Target.QualifiedModuleName);
35-
rewriter.InsertAfter(identifierContext.Stop.TokenIndex, $" {Tokens.As} {asTypeName}");
52+
rewriter.InsertAfter(insertAfterTarget, $" {Tokens.As} {asTypeName}");
3653
}
3754

3855
private static string InferAsTypeNameForInspectionResult(Declaration target, ImplicitAsTypeNameResolver resolver, AsTypeNamesResultsHandler resultsHandler)

‎RubberduckTests/QuickFixes/DeclareAsExplicitTypeQuickFixTests.cs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
using Moq;
2+
using NUnit.Framework;
3+
using Rubberduck.CodeAnalysis.Inspections;
4+
using Rubberduck.CodeAnalysis.Inspections.Concrete;
5+
using Rubberduck.CodeAnalysis.QuickFixes;
6+
using Rubberduck.CodeAnalysis.QuickFixes.Concrete;
7+
using Rubberduck.Parsing.Rewriter;
8+
using Rubberduck.Parsing.VBA;
9+
using Rubberduck.Parsing.VBA.Parsing;
10+
using Rubberduck.Refactoring.ParseTreeValue;
11+
using Rubberduck.Refactorings.ImplicitTypeToExplicit;
12+
using Rubberduck.VBEditor;
13+
using Rubberduck.VBEditor.Utility;
14+
using RubberduckTests.Mocks;
15+
using System;
16+
using System.Linq;
17+
using System.Threading;
18+
19+
namespace RubberduckTests.QuickFixes
20+
{
21+
[TestFixture]
22+
public class DeclareAsExplicitTypeQuickFixTests
23+
{
24+
[Test]
25+
[Category("QuickFixes")]
26+
public void VariableTypeNotDeclared_Variable()
27+
{
28+
const string inputCode =
29+
@"Sub Foo(arg As String)
30+
Dim var1
31+
var1 = arg
32+
End Sub";
33+
34+
const string expectedCode =
35+
@"Sub Foo(arg As String)
36+
Dim var1 As String
37+
var1 = arg
38+
End Sub";
39+
40+
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode);
41+
Assert.AreEqual(expectedCode, actualCode);
42+
}
43+
44+
[Test]
45+
[Category("QuickFixes")]
46+
public void VariableTypeNotDeclared_Parameter()
47+
{
48+
const string inputCode =
49+
@"
50+
Private mArg As Long
51+
52+
Sub Foo(arg1)
53+
arg1 = mArg
54+
End Sub";
55+
56+
const string expectedCode =
57+
@"
58+
Private mArg As Long
59+
60+
Sub Foo(arg1 As Long)
61+
arg1 = mArg
62+
End Sub";
63+
64+
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode);
65+
Assert.AreEqual(expectedCode, actualCode);
66+
}
67+
68+
private string ApplyQuickFixToFirstInspectionResult(string inputCode)
69+
{
70+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var component);
71+
var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);
72+
using (state)
73+
{
74+
var inspection = new VariableTypeNotDeclaredInspection(state);
75+
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
76+
var resultToFix = inspectionResults.First();
77+
78+
var refactoringAction = new ImplicitTypeToExplicitRefactoringAction(state, new ParseTreeValueFactory(), rewritingManager);
79+
var quickFix = new DeclareAsExplicitTypeQuickFix(refactoringAction);
80+
81+
var rewriteSession = rewritingManager.CheckOutCodePaneSession();
82+
quickFix.Fix(resultToFix, rewriteSession);
83+
rewriteSession.TryRewrite();
84+
85+
return component.CodeModule.Content();
86+
}
87+
}
88+
}
89+
}

‎RubberduckTests/QuickFixes/DeclareAsExplicitVariantQuickFixTests.cs

Lines changed: 41 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,101 +1,79 @@
1-
using NUnit.Framework;
1+
using Moq;
2+
using NUnit.Framework;
3+
using Rubberduck.CodeAnalysis.Inspections;
24
using Rubberduck.CodeAnalysis.Inspections.Concrete;
35
using Rubberduck.CodeAnalysis.QuickFixes;
46
using Rubberduck.CodeAnalysis.QuickFixes.Concrete;
7+
using Rubberduck.Parsing.Rewriter;
58
using Rubberduck.Parsing.VBA;
9+
using Rubberduck.Parsing.VBA.Parsing;
10+
using Rubberduck.Refactoring.ParseTreeValue;
11+
using Rubberduck.Refactorings.ImplicitTypeToExplicit;
12+
using Rubberduck.VBEditor;
13+
using Rubberduck.VBEditor.Utility;
14+
using RubberduckTests.Mocks;
15+
using System;
16+
using System.Linq;
17+
using System.Threading;
618

719
namespace RubberduckTests.QuickFixes
820
{
921
[TestFixture]
10-
public class DeclareAsExplicitVariantQuickFixTests : QuickFixTestBase
22+
public class DeclareAsExplicitVariantQuickFixTests
1123
{
12-
13-
[Test]
14-
[Category("QuickFixes")]
15-
public void VariableTypeNotDeclared_QuickFixWorks_Parameter()
16-
{
17-
const string inputCode =
18-
@"Sub Foo(arg1)
19-
End Sub";
20-
21-
const string expectedCode =
22-
@"Sub Foo(arg1 As Variant)
23-
End Sub";
24-
25-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new VariableTypeNotDeclaredInspection(state));
26-
Assert.AreEqual(expectedCode, actualCode);
27-
}
28-
2924
[Test]
3025
[Category("QuickFixes")]
31-
public void VariableTypeNotDeclared_QuickFixWorks_SubNameContainsParameterName()
26+
public void VariableTypeNotDeclared_Variable()
3227
{
3328
const string inputCode =
34-
@"Sub Foo(Foo)
35-
End Sub";
36-
37-
const string expectedCode =
38-
@"Sub Foo(Foo As Variant)
39-
End Sub";
40-
41-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new VariableTypeNotDeclaredInspection(state));
42-
Assert.AreEqual(expectedCode, actualCode);
43-
}
44-
45-
[Test]
46-
[Category("QuickFixes")]
47-
public void VariableTypeNotDeclared_QuickFixWorks_Variable()
48-
{
49-
const string inputCode =
50-
@"Sub Foo()
29+
@"Sub Foo()
5130
Dim var1
5231
End Sub";
5332

5433
const string expectedCode =
55-
@"Sub Foo()
34+
@"Sub Foo()
5635
Dim var1 As Variant
5736
End Sub";
5837

59-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new VariableTypeNotDeclaredInspection(state));
38+
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode);
6039
Assert.AreEqual(expectedCode, actualCode);
6140
}
6241

6342
[Test]
6443
[Category("QuickFixes")]
65-
public void VariableTypeNotDeclared_QuickFixWorks_ParameterWithoutDefaultValue()
44+
public void VariableTypeNotDeclared_Parameter()
6645
{
6746
const string inputCode =
68-
@"Sub Foo(ByVal Fizz)
47+
@"Sub Foo(arg1)
6948
End Sub";
7049

7150
const string expectedCode =
72-
@"Sub Foo(ByVal Fizz As Variant)
51+
@"Sub Foo(arg1 As Variant)
7352
End Sub";
7453

75-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new VariableTypeNotDeclaredInspection(state));
54+
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode);
7655
Assert.AreEqual(expectedCode, actualCode);
7756
}
78-
79-
[Test]
80-
[Category("QuickFixes")]
81-
public void VariableTypeNotDeclared_QuickFixWorks_ParameterWithDefaultValue()
82-
{
83-
const string inputCode =
84-
@"Sub Foo(ByVal Fizz = False)
85-
End Sub";
86-
87-
const string expectedCode =
88-
@"Sub Foo(ByVal Fizz As Variant = False)
89-
End Sub";
90-
91-
var actualCode = ApplyQuickFixToFirstInspectionResult(inputCode, state => new VariableTypeNotDeclaredInspection(state));
92-
Assert.AreEqual(expectedCode, actualCode);
93-
}
94-
95-
96-
protected override IQuickFix QuickFix(RubberduckParserState state)
57+
58+
private string ApplyQuickFixToFirstInspectionResult(string inputCode)
9759
{
98-
return new DeclareAsExplicitVariantQuickFix();
60+
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var component);
61+
var (state, rewritingManager) = MockParser.CreateAndParseWithRewritingManager(vbe.Object);
62+
using (state)
63+
{
64+
var inspection = new VariableTypeNotDeclaredInspection(state);
65+
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
66+
var resultToFix = inspectionResults.First();
67+
68+
var refactoringAction = new ImplicitTypeToExplicitRefactoringAction(state, new ParseTreeValueFactory(), rewritingManager);
69+
var quickFix = new DeclareAsExplicitVariantQuickFix(refactoringAction);
70+
71+
var rewriteSession = rewritingManager.CheckOutCodePaneSession();
72+
quickFix.Fix(resultToFix, rewriteSession);
73+
rewriteSession.TryRewrite();
74+
75+
return component.CodeModule.Content();
76+
}
9977
}
10078
}
10179
}

‎RubberduckTests/Refactoring/ImplicitTypeToExplicit/ImplicitTypeToExplicitRefactoringActionParameterTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,24 @@ End Function
231231
StringAssert.Contains($"{targetName} As {expectedType}", refactoredCode);
232232
}
233233

234+
//https://github.com/rubberduck-vba/Rubberduck/issues/5646
235+
[TestCase("argArray()", "argArray() As Variant")]
236+
[TestCase("arg1 As Long, arg2 As String, argArray()", "arg1 As Long, arg2 As String, argArray() As Variant")]
237+
[TestCase("arg1 As Long, argArray(), arg2 As String", "arg1 As Long, argArray() As Variant, arg2 As String")]
238+
[Category("Refactorings")]
239+
[Category(nameof(ImplicitTypeToExplicitRefactoringAction))]
240+
public void Parameter_Arrays(string argList, string expectedArgList)
241+
{
242+
var targetName = "argArray";
243+
244+
var inputCode =
245+
$@"Sub Foo({argList})
246+
End Sub";
247+
248+
var refactoredCode = RefactoredCode(inputCode, NameAndDeclarationTypeTuple(targetName));
249+
StringAssert.Contains(expectedArgList, refactoredCode);
250+
}
251+
234252
(string, DeclarationType) NameAndDeclarationTypeTuple(string name)
235253
=> (name, DeclarationType.Parameter);
236254
}

0 commit comments

Comments
 (0)