Skip to content

Commit 296a7e9

Browse files
committed
all tests pass
1 parent 49916b1 commit 296a7e9

File tree

6 files changed

+93
-68
lines changed

6 files changed

+93
-68
lines changed

Rubberduck.Parsing/Symbols/Declaration.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public Declaration(QualifiedMemberName qualifiedName, Declaration parentDeclarat
5252
_context = context;
5353
_isBuiltIn = isBuiltIn;
5454
_annotations = annotations;
55-
_attributes = attributes;
55+
_attributes = attributes ?? new Attributes();
5656

5757
_projectName = _qualifiedName.QualifiedModuleName.ProjectName;
5858

Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs

Lines changed: 69 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void EnterWithBlock(VBAParser.WithStmtContext context)
112112
}
113113
else
114114
{
115-
//qualifier = ResolveType(typeContext.complexType());
115+
qualifier = ResolveType(typeContext.complexType());
116116
}
117117
}
118118

@@ -160,11 +160,11 @@ private void ResolveType(VBAParser.ICS_S_MembersCallContext context)
160160
ResolveType(identifiers);
161161
}
162162

163-
private void ResolveType(VBAParser.ComplexTypeContext context)
163+
private Declaration ResolveType(VBAParser.ComplexTypeContext context)
164164
{
165165
if (context == null)
166166
{
167-
return;
167+
return null;
168168
}
169169

170170
var identifiers = context.ambiguousIdentifier()
@@ -175,19 +175,19 @@ private void ResolveType(VBAParser.ComplexTypeContext context)
175175
if (identifiers.Count == 1)
176176
{
177177
var type = ResolveInScopeType(identifiers.Single().GetText(), _currentScope);
178-
if (type != null)
178+
if (type != null && !_alreadyResolved.Contains(context))
179179
{
180180
type.AddReference(CreateReference(context, type));
181181
_alreadyResolved.Add(context);
182182
}
183-
return;
183+
return type;
184184
}
185185

186186
// if there's 2 or more identifiers, resolve to the deepest path:
187-
ResolveType(identifiers);
187+
return ResolveType(identifiers);
188188
}
189189

190-
private void ResolveType(IList<VBAParser.AmbiguousIdentifierContext> identifiers)
190+
private Declaration ResolveType(IList<VBAParser.AmbiguousIdentifierContext> identifiers)
191191
{
192192
var first = identifiers[0].GetText();
193193
var projectMatch = _declarationFinder.FindProject(_currentScope, first);
@@ -214,38 +214,56 @@ private void ResolveType(IList<VBAParser.AmbiguousIdentifierContext> identifiers
214214
{
215215
var udtReference = CreateReference(identifiers[2], udtMatch);
216216

217-
projectMatch.AddReference(projectReference);
218-
_alreadyResolved.Add(projectReference.Context);
219-
220-
moduleMatch.AddReference(moduleReference);
221-
_alreadyResolved.Add(moduleReference.Context);
222-
223-
udtMatch.AddReference(udtReference);
224-
_alreadyResolved.Add(udtReference.Context);
225-
226-
return;
217+
if (!_alreadyResolved.Contains(projectReference.Context))
218+
{
219+
projectMatch.AddReference(projectReference);
220+
_alreadyResolved.Add(projectReference.Context);
221+
}
222+
223+
if (!_alreadyResolved.Contains(moduleReference.Context))
224+
{
225+
moduleMatch.AddReference(moduleReference);
226+
_alreadyResolved.Add(moduleReference.Context);
227+
}
228+
229+
if (!_alreadyResolved.Contains(udtReference.Context))
230+
{
231+
udtMatch.AddReference(udtReference);
232+
_alreadyResolved.Add(udtReference.Context);
233+
}
234+
235+
return udtMatch;
227236
}
228237
var enumMatch = _declarationFinder.FindEnum(moduleMatch, identifiers[2].GetText());
229238
if (enumMatch != null)
230239
{
231240
var enumReference = CreateReference(identifiers[2], enumMatch);
232241

233-
projectMatch.AddReference(projectReference);
234-
_alreadyResolved.Add(projectReference.Context);
235-
236-
moduleMatch.AddReference(moduleReference);
237-
_alreadyResolved.Add(moduleReference.Context);
238-
239-
enumMatch.AddReference(enumReference);
240-
_alreadyResolved.Add(enumReference.Context);
241-
242-
return;
242+
if (!_alreadyResolved.Contains(projectReference.Context))
243+
{
244+
projectMatch.AddReference(projectReference);
245+
_alreadyResolved.Add(projectReference.Context);
246+
}
247+
248+
if (!_alreadyResolved.Contains(moduleReference.Context))
249+
{
250+
moduleMatch.AddReference(moduleReference);
251+
_alreadyResolved.Add(moduleReference.Context);
252+
}
253+
254+
if (!_alreadyResolved.Contains(enumReference.Context))
255+
{
256+
enumMatch.AddReference(enumReference);
257+
_alreadyResolved.Add(enumReference.Context);
258+
}
259+
260+
return enumMatch;
243261
}
244262
}
245263
}
246264
else
247265
{
248-
if (projectReference != null)
266+
if (projectReference != null && !_alreadyResolved.Contains(projectReference.Context))
249267
{
250268
projectMatch.AddReference(projectReference);
251269
_alreadyResolved.Add(projectReference.Context);
@@ -257,12 +275,12 @@ private void ResolveType(IList<VBAParser.AmbiguousIdentifierContext> identifiers
257275
if (match != null)
258276
{
259277
var reference = CreateReference(identifiers[1], match);
260-
if (reference != null)
278+
if (reference != null && !_alreadyResolved.Contains(reference.Context))
261279
{
262280
match.AddReference(reference);
263281
_alreadyResolved.Add(reference.Context);
264-
return;
265282
}
283+
return match;
266284
}
267285
}
268286
}
@@ -284,14 +302,24 @@ private void ResolveType(IList<VBAParser.AmbiguousIdentifierContext> identifiers
284302
{
285303
var reference = CreateReference(identifiers[1], match);
286304

287-
moduleMatch.AddReference(moduleReference);
288-
_alreadyResolved.Add(moduleReference.Context);
305+
if (!_alreadyResolved.Contains(moduleReference.Context))
306+
{
307+
moduleMatch.AddReference(moduleReference);
308+
_alreadyResolved.Add(moduleReference.Context);
309+
}
310+
311+
if (!_alreadyResolved.Contains(reference.Context))
312+
{
313+
match.AddReference(reference);
314+
_alreadyResolved.Add(reference.Context);
315+
}
289316

290-
match.AddReference(reference);
291-
_alreadyResolved.Add(reference.Context);
317+
return match;
292318
}
293319
}
294320
}
321+
322+
return null;
295323
}
296324

297325
private Declaration ResolveInScopeType(string identifier, Declaration scope)
@@ -379,11 +407,6 @@ private Declaration ResolveInternal(ParserRuleContext callSiteContext, Declarati
379407
return null;
380408
}
381409

382-
if (_alreadyResolved.Contains(callSiteContext))
383-
{
384-
return null;
385-
}
386-
387410
if (!IdentifierContexts.Contains(callSiteContext.GetType()))
388411
{
389412
throw new ArgumentException("'" + callSiteContext.GetType().Name + "' is not an identifier context.", "callSiteContext");
@@ -440,7 +463,7 @@ private Declaration ResolveInternal(ParserRuleContext callSiteContext, Declarati
440463
}
441464

442465
var reference = CreateReference(callSiteContext, callee, isAssignmentTarget, hasExplicitLetStatement);
443-
if (reference != null)
466+
if (reference != null && !_alreadyResolved.Contains(reference.Context))
444467
{
445468
callee.AddReference(reference);
446469
_alreadyResolved.Add(reference.Context);
@@ -989,7 +1012,9 @@ private Declaration FindLocalScopeDeclaration(string identifierName, Declaration
9891012
var matches = _declarationFinder.MatchName(identifierName);
9901013

9911014
var results = matches.Where(item =>
992-
(localScope.Equals(item.ParentScopeDeclaration) || (isAssignmentTarget && item.Scope == localScope.Scope))
1015+
((localScope.Equals(item.ParentDeclaration)
1016+
|| (item.DeclarationType == DeclarationType.Parameter && localScope.Equals(item.ParentScopeDeclaration)))
1017+
|| (isAssignmentTarget && item.Scope == localScope.Scope))
9931018
&& localScope.Context.GetSelection().Contains(item.Selection)
9941019
&& !_moduleTypes.Contains(item.DeclarationType))
9951020
.ToList();
@@ -1068,10 +1093,9 @@ private Declaration FindProjectScopeDeclaration(string identifierName, Declarati
10681093
// the "$" in e.g. "UCase$" isn't picked up as part of the identifierName, so we need to add it manually:
10691094
var matches = _declarationFinder.MatchName(identifierName).Where(item =>
10701095
(!item.IsBuiltIn || item.IdentifierName == identifierName + (hasStringQualifier ? "$" : string.Empty))
1071-
/*&& item.ParentDeclaration.DeclarationType == DeclarationType.Module*/).ToList();
1072-
1073-
// note: we cannot be sure that a class has no PredeclaredId until we can read attributes.
1074-
// for this reason we cannot limit the scope of public members to DeclarationType.Module.
1096+
&& ((item.ParentDeclaration.DeclarationType == DeclarationType.Module
1097+
|| item.ParentDeclaration.HasPredeclaredId))
1098+
|| item.ParentScopeDeclaration.Equals(localScope)).ToList();
10751099

10761100
if (matches.Count == 1)
10771101
{

RubberduckTests/Grammar/ResolverTests.cs

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -192,36 +192,35 @@ Public foo As Integer
192192
}
193193

194194
[TestMethod]
195-
public void PublicInstanceVariable_DoesNotRequireInstance()
195+
public void PublicVariableAssignment_IsReferenceToVariableDeclaration()
196196
{
197-
// todo: reverse this test condition when we can read module attributes
198-
199197
// arrange
200198
var code_class1 = @"
201199
Public Sub DoSomething()
202-
Debug.Print foo
200+
foo = 42
203201
End Sub
204202
";
205-
var code_class2 = @"
203+
var code_module1 = @"
206204
Option Explicit
207205
Public foo As Integer
208206
";
207+
var class1 = Tuple.Create(code_class1, vbext_ComponentType.vbext_ct_ClassModule);
208+
var module1 = Tuple.Create(code_module1, vbext_ComponentType.vbext_ct_StdModule);
209+
209210
// act
210-
var state = Resolve(code_class1, code_class2);
211+
var state = Resolve(class1, module1);
211212

212213
// assert
213214
var declaration = state.AllUserDeclarations.Single(item =>
214215
item.DeclarationType == DeclarationType.Variable && item.IdentifierName == "foo");
215216

216-
var reference = declaration.References.SingleOrDefault(item => !item.IsAssignment
217-
&& item.ParentScoping.IdentifierName == "DoSomething"
218-
&& item.ParentScoping.DeclarationType == DeclarationType.Procedure);
219-
217+
var reference = declaration.References.SingleOrDefault(item => item.IsAssignment);
220218
Assert.IsNotNull(reference);
219+
Assert.AreEqual("DoSomething", reference.ParentScoping.IdentifierName);
221220
}
222221

223222
[TestMethod]
224-
public void PublicVariableAssignment_IsReferenceToVariableDeclaration()
223+
public void EncapsulatedVariableAssignment_DoesNotResolve()
225224
{
226225
// arrange
227226
var code_class1 = @"
@@ -233,16 +232,18 @@ End Sub
233232
Option Explicit
234233
Public foo As Integer
235234
";
235+
var class1 = Tuple.Create(code_class1, vbext_ComponentType.vbext_ct_ClassModule);
236+
var class2 = Tuple.Create(code_class2, vbext_ComponentType.vbext_ct_ClassModule);
237+
236238
// act
237-
var state = Resolve(code_class1, code_class2);
239+
var state = Resolve(class1, class2);
238240

239241
// assert
240242
var declaration = state.AllUserDeclarations.Single(item =>
241243
item.DeclarationType == DeclarationType.Variable && item.IdentifierName == "foo");
242244

243245
var reference = declaration.References.SingleOrDefault(item => item.IsAssignment);
244-
Assert.IsNotNull(reference);
245-
Assert.AreEqual("DoSomething", reference.ParentScoping.IdentifierName);
246+
Assert.IsNull(reference);
246247
}
247248

248249
[TestMethod]

RubberduckTests/Inspections/ConstantNotUsedInspectionTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public void ConstantNotUsed_ReturnsResult_MultipleConsts()
6565
}
6666

6767
[TestMethod]
68-
public void ConstantNotUsed_DoesNotReturnResult()
68+
public void GivenReferencedConstant_ReturnsNoInspectionResult()
6969
{
7070
const string inputCode =
7171
@"Public Sub Foo()
@@ -94,7 +94,7 @@ Public Sub Goo(ByVal arg1 As Integer)
9494
}
9595

9696
[TestMethod]
97-
public void ConstantNotUsed_ReturnsResult_SomeConstantsUsed()
97+
public void GivenConstantNotUsed_ReturnsResultForUnusedConstant()
9898
{
9999
const string inputCode =
100100
@"Public Sub Foo()

RubberduckTests/Inspections/FunctionReturnValueNotUsedInspectionTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ public void FunctionReturnValueNotUsed_DoesNotReturnResult_IgnoresBuiltInFunctio
355355
}
356356

357357
[TestMethod]
358-
public void FunctionReturnValueNotUsed_DoesNotReturnResult_InterfaceImplementationMember()
358+
public void GivenInterfaceImplementationMember_ReturnsNoResult()
359359
{
360360
const string interfaceCode =
361361
@"Public Function Test() As Integer

RubberduckTests/Refactoring/EncapsulateFieldTests.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -693,16 +693,16 @@ Sub Bar(ByVal name As Integer)
693693
}
694694

695695
[TestMethod]
696-
public void EncapsulatePublicField_FieldHasReferencesInMultipleClasses()
696+
public void GivenReferencedPublicField_UpdatesReferenceToNewProperty()
697697
{
698698
//Input
699-
const string inputCode1 =
699+
const string code_class1 =
700700
@"Public fizz As Integer
701701
702702
Sub Foo()
703703
fizz = 1
704704
End Sub";
705-
const string inputCode2 =
705+
const string code_class2 =
706706
@"Sub Foo()
707707
Dim c As Class1
708708
c.fizz = 0
@@ -743,8 +743,8 @@ Sub Bar(ByVal v As Integer)
743743
//Arrange
744744
var builder = new MockVbeBuilder();
745745
var project = builder.ProjectBuilder("TestProject1", vbext_ProjectProtection.vbext_pp_none)
746-
.AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, inputCode1)
747-
.AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, inputCode2)
746+
.AddComponent("Class1", vbext_ComponentType.vbext_ct_ClassModule, code_class1)
747+
.AddComponent("Class2", vbext_ComponentType.vbext_ct_ClassModule, code_class2)
748748
.Build();
749749
var vbe = builder.AddProject(project).Build();
750750
var component = project.Object.VBComponents.Item(0);

0 commit comments

Comments
 (0)