Skip to content

Commit d59c3e2

Browse files
retailcoderretailcoder
authored
Merge pull request #5248 from MDoerner/ResolveExpressionInNext
Resolve expression in Next of For loop
2 parents e86108c + 9b88aad commit d59c3e2

File tree

8 files changed

+456
-76
lines changed

8 files changed

+456
-76
lines changed

‎Rubberduck.CodeAnalysis/QuickFixes/ExpandBangNotationQuickFix.cs

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
using System.Linq;
1+
using System.Collections.Generic;
2+
using System.Linq;
23
using Antlr4.Runtime;
34
using Rubberduck.Inspections.Abstract;
45
using Rubberduck.Inspections.Concrete;
56
using Rubberduck.Parsing.Grammar;
67
using Rubberduck.Parsing.Inspections.Abstract;
78
using Rubberduck.Parsing.Rewriter;
9+
using Rubberduck.Parsing.Symbols;
810
using Rubberduck.Parsing.VBA;
911
using Rubberduck.Parsing.VBA.DeclarationCaching;
1012
using Rubberduck.VBEditor;
@@ -59,10 +61,34 @@ private void InsertDefaultMember(VBAParser.DictionaryAccessContext dictionaryAcc
5961
private string DefaultMemberAccessCode(QualifiedSelection selection, DeclarationFinder finder)
6062
{
6163
var defaultMemberAccesses = finder.IdentifierReferences(selection);
62-
var defaultMemberNames = defaultMemberAccesses.Select(reference => reference.Declaration.IdentifierName);
64+
var defaultMemberNames = defaultMemberAccesses
65+
.Select(DefaultMemberName)
66+
.Select(declarationName => IsNotLegalIdentifierName(declarationName)
67+
? $"[{declarationName}]"
68+
: declarationName);
6369
return $".{string.Join("().", defaultMemberNames)}";
6470
}
6571

72+
private static string DefaultMemberName(IdentifierReference defaultMemberReference)
73+
{
74+
var defaultMemberMemberName = defaultMemberReference.Declaration.QualifiedName;
75+
var fullDefaultMemberName = $"{defaultMemberMemberName.QualifiedModuleName.ProjectName}.{defaultMemberMemberName.QualifiedModuleName.ComponentName}.{defaultMemberMemberName.MemberName}";
76+
77+
if (DefaultMemberOverrides.TryGetValue(fullDefaultMemberName, out var defaultMemberOverride))
78+
{
79+
return defaultMemberOverride;
80+
}
81+
82+
return defaultMemberMemberName.MemberName;
83+
}
84+
85+
private bool IsNotLegalIdentifierName(string declarationName)
86+
{
87+
return string.IsNullOrEmpty(declarationName)
88+
|| NonIdentifierCharacters.Any(character => declarationName.Contains(character))
89+
|| AdditionalNonFirstIdentifierCharacters.Contains(declarationName[0]); ;
90+
}
91+
6692
public override string Description(IInspectionResult result)
6793
{
6894
return Resources.Inspections.QuickFixes.ExpandBangNotationQuickFix;
@@ -71,5 +97,14 @@ public override string Description(IInspectionResult result)
7197
public override bool CanFixInProcedure => true;
7298
public override bool CanFixInModule => true;
7399
public override bool CanFixInProject => true;
100+
101+
private string NonIdentifierCharacters = "[](){}\r\n\t.,'\"\\ |!@#$%^&*-+:=; ";
102+
private string AdditionalNonFirstIdentifierCharacters = "0123456789_";
103+
104+
private static readonly Dictionary<string, string> DefaultMemberOverrides = new Dictionary<string, string>
105+
{
106+
["Excel.Range._Default"] = "Item"
107+
};
108+
74109
}
75110
}

‎Rubberduck.CodeAnalysis/QuickFixes/ExpandDefaultMemberQuickFix.cs

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
using System.Linq;
1+
using System.Collections.Generic;
2+
using System.Linq;
23
using Antlr4.Runtime;
34
using Rubberduck.Inspections.Abstract;
45
using Rubberduck.Inspections.Concrete;
6+
using Rubberduck.Parsing.Grammar;
57
using Rubberduck.Parsing.Inspections.Abstract;
68
using Rubberduck.Parsing.Rewriter;
79
using Rubberduck.Parsing.Symbols;
@@ -54,10 +56,74 @@ private void InsertDefaultMember(ParserRuleContext lExpressionContext, Qualified
5456
private string DefaultMemberAccessCode(QualifiedSelection selection, DeclarationFinder finder)
5557
{
5658
var defaultMemberAccesses = finder.IdentifierReferences(selection).Where(reference => reference.DefaultMemberRecursionDepth > 0);
57-
var defaultMemberNames = defaultMemberAccesses.Select(reference => reference.Declaration.IdentifierName);
59+
var defaultMemberNames = defaultMemberAccesses.Select(DefaultMemberName)
60+
.Select(declarationName => IsNotLegalIdentifierName(declarationName)
61+
? $"[{declarationName}]"
62+
: declarationName);
5863
return $".{string.Join("().", defaultMemberNames)}";
5964
}
6065

66+
private bool IsNotLegalIdentifierName(string declarationName)
67+
{
68+
return string.IsNullOrEmpty(declarationName)
69+
|| NonIdentifierCharacters.Any(character => declarationName.Contains(character))
70+
|| AdditionalNonFirstIdentifierCharacters.Contains(declarationName[0]); ;
71+
}
72+
73+
private static string DefaultMemberName(IdentifierReference defaultMemberReference)
74+
{
75+
var defaultMemberMemberName = defaultMemberReference.Declaration.QualifiedName;
76+
var fullDefaultMemberName = $"{defaultMemberMemberName.QualifiedModuleName.ProjectName}.{defaultMemberMemberName.QualifiedModuleName.ComponentName}.{defaultMemberMemberName.MemberName}";
77+
78+
if (DefaultMemberBaseOverrides.TryGetValue(fullDefaultMemberName, out var baseOverride))
79+
{
80+
if (DefaultMemberArgumentNumberOverrides.TryGetValue(fullDefaultMemberName, out var argumentNumberOverrides))
81+
{
82+
var numberOfArguments = NumberOfArguments(defaultMemberReference);
83+
if (argumentNumberOverrides.TryGetValue(numberOfArguments, out var numberOfArgumentsOverride))
84+
{
85+
return numberOfArgumentsOverride;
86+
}
87+
}
88+
89+
return baseOverride;
90+
}
91+
92+
return defaultMemberMemberName.MemberName;
93+
}
94+
95+
private static int NumberOfArguments(IdentifierReference defaultMemberReference)
96+
{
97+
if (defaultMemberReference.IsNonIndexedDefaultMemberAccess)
98+
{
99+
return 0;
100+
}
101+
102+
var argumentList = ArgumentList(defaultMemberReference);
103+
if (argumentList == null)
104+
{
105+
return -1;
106+
}
107+
108+
var arguments = argumentList.argument();
109+
110+
return arguments?.Length ?? 0;
111+
}
112+
113+
private static VBAParser.ArgumentListContext ArgumentList(IdentifierReference indexedDefaultMemberReference)
114+
{
115+
var defaultMemberReferenceContextWithArguments = indexedDefaultMemberReference.Context.Parent;
116+
switch (defaultMemberReferenceContextWithArguments)
117+
{
118+
case VBAParser.IndexExprContext indexExpression:
119+
return indexExpression.argumentList();
120+
case VBAParser.WhitespaceIndexExprContext whiteSpaceIndexExpression:
121+
return whiteSpaceIndexExpression.argumentList();
122+
default:
123+
return null;
124+
}
125+
}
126+
61127
public override string Description(IInspectionResult result)
62128
{
63129
return Resources.Inspections.QuickFixes.ExpandDefaultMemberQuickFix;
@@ -66,5 +132,18 @@ public override string Description(IInspectionResult result)
66132
public override bool CanFixInProcedure => true;
67133
public override bool CanFixInModule => true;
68134
public override bool CanFixInProject => true;
135+
136+
private string NonIdentifierCharacters = "[](){}\r\n\t.,'\"\\ |!@#$%^&*-+:=; ";
137+
private string AdditionalNonFirstIdentifierCharacters = "0123456789_";
138+
139+
private static readonly Dictionary<string, string> DefaultMemberBaseOverrides = new Dictionary<string, string>
140+
{
141+
["Excel.Range._Default"] = "Item"
142+
};
143+
144+
private static readonly Dictionary<string, Dictionary<int, string>> DefaultMemberArgumentNumberOverrides = new Dictionary<string, Dictionary<int, string>>
145+
{
146+
["Excel.Range._Default"] = new Dictionary<int, string>{[0] = "Value"}
147+
};
69148
}
70149
}

‎Rubberduck.Parsing/VBA/ReferenceManagement/IdentifierReferenceResolver.cs

Lines changed: 36 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -614,51 +614,40 @@ public void Resolve(VBAParser.AsTypeClauseContext context)
614614

615615
public void Resolve(VBAParser.ForNextStmtContext context)
616616
{
617+
var expressions = context.expression();
618+
617619
// In "For expr1 = expr2" the "expr1 = expr2" part is treated as a single expression.
618-
var assignmentExpr = ((VBAParser.RelationalOpContext)context.expression()[0]);
619-
var lExpr = assignmentExpr.expression()[0];
620-
var withExpression = GetInnerMostWithExpression();
621-
var firstExpression = _bindingService.ResolveDefault(
622-
_moduleDeclaration,
623-
_currentParent,
624-
lExpr,
625-
withExpression,
626-
StatementResolutionContext.Undefined,
627-
true);
628-
if (firstExpression.Classification != ExpressionClassification.ResolutionFailed)
629-
{
630-
// each iteration counts as an assignment
620+
var assignmentExpr = ((VBAParser.RelationalOpContext)expressions[0]);
621+
ResolveStartValueAssignmentOfForNext(assignmentExpr);
631622

632-
_failedResolutionVisitor.CollectUnresolved(firstExpression, _currentParent, withExpression);
633-
_boundExpressionVisitor.AddIdentifierReferences(
634-
firstExpression,
635-
_qualifiedModuleName,
636-
_currentScope,
637-
_currentParent,
638-
true);
639-
}
640-
var rExpr = assignmentExpr.expression()[1];
641-
var secondExpression = _bindingService.ResolveDefault(
642-
_moduleDeclaration,
643-
_currentParent,
644-
rExpr,
645-
withExpression,
646-
StatementResolutionContext.Undefined,
647-
true);
648-
_failedResolutionVisitor.CollectUnresolved(secondExpression, _currentParent, withExpression);
649-
_boundExpressionVisitor.AddIdentifierReferences(
650-
secondExpression,
651-
_qualifiedModuleName,
652-
_currentScope,
653-
_currentParent);
654-
655-
ResolveDefault(context.expression()[1], true);
623+
ResolveToValueOfForNext(expressions[1]);
656624

657625
var stepStatement = context.stepStmt();
658626
if (stepStatement != null)
659627
{
660628
Resolve(stepStatement);
661629
}
630+
631+
const int firstNextExpressionIndex = 2;
632+
for (var exprIndex = firstNextExpressionIndex; exprIndex < expressions.Length; exprIndex++)
633+
{
634+
ResolveDefault(expressions[exprIndex]);
635+
}
636+
}
637+
638+
private void ResolveStartValueAssignmentOfForNext(VBAParser.RelationalOpContext expression)
639+
{
640+
var expressions = expression.expression();
641+
var elementVariableExpression = expressions[0];
642+
ResolveDefault(elementVariableExpression, requiresLetCoercion: true, isAssignmentTarget: true);
643+
644+
var startValueExpression = expressions[1];
645+
ResolveDefault(startValueExpression, requiresLetCoercion: true);
646+
}
647+
648+
private void ResolveToValueOfForNext(ParserRuleContext expression)
649+
{
650+
ResolveDefault(expression, requiresLetCoercion: true);
662651
}
663652

664653
private void Resolve(VBAParser.StepStmtContext context)
@@ -668,38 +657,18 @@ private void Resolve(VBAParser.StepStmtContext context)
668657

669658
public void Resolve(VBAParser.ForEachStmtContext context)
670659
{
671-
var withExpression = GetInnerMostWithExpression();
672-
var firstExpression = _bindingService.ResolveDefault(
673-
_moduleDeclaration,
674-
_currentParent,
675-
context.expression()[0],
676-
withExpression,
677-
StatementResolutionContext.Undefined,
678-
false);
679-
if (firstExpression.Classification == ExpressionClassification.ResolutionFailed)
680-
{
660+
var expressions = context.expression();
681661

682-
_failedResolutionVisitor.CollectUnresolved(firstExpression, _currentParent, withExpression);
683-
_boundExpressionVisitor.AddIdentifierReferences(
684-
firstExpression,
685-
_qualifiedModuleName,
686-
_currentScope,
687-
_currentParent);
688-
}
689-
else
690-
{
691-
// each iteration counts as an assignment
692-
_failedResolutionVisitor.CollectUnresolved(firstExpression, _currentParent, withExpression);
693-
_boundExpressionVisitor.AddIdentifierReferences(
694-
firstExpression,
695-
_qualifiedModuleName,
696-
_currentScope,
697-
_currentParent,
698-
true);
699-
}
700-
for (var exprIndex = 1; exprIndex < context.expression().Length; exprIndex++)
662+
var elementVariableExpression = expressions[0];
663+
ResolveDefault(elementVariableExpression, isAssignmentTarget: true);
664+
665+
var collectionExpression = expressions[1];
666+
ResolveDefault(collectionExpression);
667+
668+
const int firstNextExpressionIndex = 2;
669+
for (var exprIndex = firstNextExpressionIndex; exprIndex < context.expression().Length; exprIndex++)
701670
{
702-
ResolveDefault(context.expression()[exprIndex], false);
671+
ResolveDefault(expressions[exprIndex]);
703672
}
704673
}
705674

0 commit comments

Comments
 (0)