Skip to content

Commit

Permalink
Merge pull request #5363 from MDoerner/FixUndeclaredVariableScopingIn…
Browse files Browse the repository at this point in the history
…Resolver

Generate distinct unresolved variable declarations in Get Set and Let
  • Loading branch information
retailcoder committed Jan 19, 2020
2 parents 5d7369a + 58e8e15 commit f27427f
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 8 deletions.
19 changes: 11 additions & 8 deletions Rubberduck.Parsing/VBA/DeclarationCaching/DeclarationFinder.cs
Expand Up @@ -30,7 +30,7 @@ public class DeclarationFinder

private readonly IReadOnlyDictionary<QualifiedModuleName, IFailedResolutionStore> _failedResolutionStores;
private readonly ConcurrentDictionary<QualifiedModuleName, IMutableFailedResolutionStore> _newFailedResolutionStores;
private readonly ConcurrentDictionary<QualifiedMemberName, ConcurrentBag<Declaration>> _newUndeclared;
private readonly ConcurrentDictionary<(QualifiedMemberName memberName, DeclarationType declarationType), ConcurrentBag<Declaration>> _newUndeclared;

private IDictionary<(QualifiedModuleName module, int annotatedLine), List<IParseTreeAnnotation>> _annotations;
private IDictionary<Declaration, List<ParameterDeclaration>> _parametersByParent;
Expand Down Expand Up @@ -78,7 +78,7 @@ private static QualifiedSelection GetGroupingKey(Declaration declaration)
_failedResolutionStores = failedResolutionStores;

_newFailedResolutionStores = new ConcurrentDictionary<QualifiedModuleName, IMutableFailedResolutionStore>();
_newUndeclared = new ConcurrentDictionary<QualifiedMemberName, ConcurrentBag<Declaration>>();
_newUndeclared = new ConcurrentDictionary<(QualifiedMemberName memberName, DeclarationType declarationType), ConcurrentBag<Declaration>>();

var collectionConstructionActions = CollectionConstructionActions(declarations, annotations);
ExecuteCollectionConstructionActions(collectionConstructionActions);
Expand Down Expand Up @@ -986,13 +986,14 @@ public Declaration OnUndeclaredVariable(Declaration enclosingProcedure, string i
null,
!isReDimVariable);

var hasUndeclared = _newUndeclared.ContainsKey(enclosingProcedure.QualifiedName);
var enclosingScope = (enclosingProcedure.QualifiedName, enclosingProcedure.DeclarationType);
var hasUndeclared = _newUndeclared.ContainsKey(enclosingScope);
if (hasUndeclared)
{
ConcurrentBag<Declaration> undeclared;
while (!_newUndeclared.TryGetValue(enclosingProcedure.QualifiedName, out undeclared))
while (!_newUndeclared.TryGetValue(enclosingScope, out undeclared))
{
_newUndeclared.TryGetValue(enclosingProcedure.QualifiedName, out undeclared);
_newUndeclared.TryGetValue(enclosingScope, out undeclared);
}
var inScopeUndeclared = undeclared.FirstOrDefault(d => d.IdentifierName == identifierName);
if (inScopeUndeclared != null)
Expand All @@ -1003,7 +1004,7 @@ public Declaration OnUndeclaredVariable(Declaration enclosingProcedure, string i
}
else
{
_newUndeclared.TryAdd(enclosingProcedure.QualifiedName, new ConcurrentBag<Declaration> { undeclaredLocal });
_newUndeclared.TryAdd(enclosingScope, new ConcurrentBag<Declaration> { undeclaredLocal });
}
return undeclaredLocal;
}
Expand Down Expand Up @@ -1067,14 +1068,16 @@ public Declaration OnBracketedExpression(string expression, ParserRuleContext co
Debug.Assert(hostApp != null, "Host application project can't be null. Make sure VBA standard library is included if host is unknown.");

var qualifiedName = hostApp.QualifiedName.QualifiedModuleName.QualifyMemberName(expression);
var declarationType = DeclarationType.BracketedExpression;
var undeclaredScope = (qualifiedName, declarationType);

if (_newUndeclared.TryGetValue(qualifiedName, out var undeclared))
if (_newUndeclared.TryGetValue(undeclaredScope, out var undeclared))
{
return undeclared.SingleOrDefault();
}

var item = new Declaration(qualifiedName, hostApp, hostApp, Tokens.Variant, string.Empty, false, false, Accessibility.Global, DeclarationType.BracketedExpression, context, null, context.GetSelection(), true, null);
_newUndeclared.TryAdd(qualifiedName, new ConcurrentBag<Declaration> { item });
_newUndeclared.TryAdd(undeclaredScope, new ConcurrentBag<Declaration> { item });
return item;
}

Expand Down
72 changes: 72 additions & 0 deletions RubberduckTests/Grammar/ResolverTests.cs
Expand Up @@ -7300,5 +7300,77 @@ End Function
Assert.AreEqual(expectedReferenceText, enumMemberReference.IdentifierName);
}
}

[Category("Grammar")]
[Category("Resolver")]
[Test]
public void OneUndeclaredVariablePerMemberAndUndeclaredIdentifier_DifferentMemberName()
{
var moduleCode = @"
Private Sub Foo
bar = 42 + 23
bar = bar + bar
bar = bar * bar
fooBar = 42
End Sub
Private Sub DoSomething
bar = 42 + 23
bar = bar + bar
bar = bar * bar
fooBaz = 42
End Sub
";

var vbe = MockVbeBuilder.BuildFromSingleStandardModule(moduleCode, out _);

using (var state = Resolve(vbe.Object))
{
var finder = state.DeclarationFinder;
var module = finder.UserDeclarations(DeclarationType.ProceduralModule)
.Single();
var undeclared = finder.Members(module.QualifiedModuleName)
.Where(declaration => declaration.IsUndeclared)
.ToList();

Assert.AreEqual(4, undeclared.Count);
}
}

[Category("Grammar")]
[Category("Resolver")]
[Test]
public void OneUndeclaredVariablePerMemberAndUndeclaredIdentifier_DifferentDeclarationType()
{
var moduleCode = @"
Private Property Get Foo() As Variant
bar = 42 + 23
bar = bar + bar
bar = bar * bar
fooBar = 42
End Property
Private Property Let Foo(arg As Variant)
bar = 42 + 23
bar = bar + bar
bar = bar * bar
fooBaz = 42
End Property
";

var vbe = MockVbeBuilder.BuildFromSingleStandardModule(moduleCode, out _);

using (var state = Resolve(vbe.Object))
{
var finder = state.DeclarationFinder;
var module = finder.UserDeclarations(DeclarationType.ProceduralModule)
.Single();
var undeclared = finder.Members(module.QualifiedModuleName)
.Where(declaration => declaration.IsUndeclared)
.ToList();

Assert.AreEqual(4, undeclared.Count);
}
}
}
}

0 comments on commit f27427f

Please sign in to comment.