Skip to content

Commit 47ffe0b

Browse files
retailcoderretailcoder
committed
3 more green, 45 red left
1 parent 86519ee commit 47ffe0b

File tree

3 files changed

+101
-123
lines changed

3 files changed

+101
-123
lines changed

‎Rubberduck.Inspections/Concrete/ObjectVariableNotSetInspection.cs

Lines changed: 2 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -44,120 +44,10 @@ private IEnumerable<IdentifierReference> InterestingReferences()
4444

4545
foreach (var reference in State.DeclarationFinder.IdentifierReferences(qmn))
4646
{
47-
if (IsIgnoringInspectionResultFor(reference, AnnotationName))
47+
if (!IsIgnoringInspectionResultFor(reference, AnnotationName)
48+
&& VariableRequiresSetAssignmentEvaluator.RequiresSetAssignment(reference, State))
4849
{
49-
// inspection is ignored at reference level
50-
continue;
51-
}
52-
53-
if (!reference.IsAssignment)
54-
{
55-
// reference isn't assigning its declaration; not interesting
56-
continue;
57-
}
58-
59-
var setStmtContext = reference.Context.GetAncestor<VBAParser.SetStmtContext>();
60-
if (setStmtContext != null)
61-
{
62-
// assignment already has a Set keyword
63-
// (but is it misplaced? ...hmmm... beyond the scope of *this* inspection though)
64-
// if we're only ever assigning to 'Nothing', might as well flag it though
65-
if (reference.Declaration.References.Where(r => r.IsAssignment).All(r =>
66-
r.Context.GetAncestor<VBAParser.SetStmtContext>().expression().GetText() == Tokens.Nothing))
67-
{
68-
result.Add(reference);
69-
continue;
70-
}
71-
}
72-
73-
var letStmtContext = reference.Context.GetAncestor<VBAParser.LetStmtContext>();
74-
if (letStmtContext == null)
75-
{
76-
// we're probably in a For Each loop
77-
continue;
78-
}
79-
80-
var declaration = reference.Declaration;
81-
if (declaration.IsArray)
82-
{
83-
// arrays don't need a Set statement... todo figure out if array items are objects
84-
continue;
85-
}
86-
87-
var isObjectVariable = declaration.IsObject();
88-
var isVariant = declaration.IsUndeclared || declaration.AsTypeName == Tokens.Variant;
89-
if (!isObjectVariable && !isVariant)
90-
{
91-
continue;
92-
}
93-
94-
if (isObjectVariable)
95-
{
96-
// get the members of the returning type, a default member could make us lie otherwise
97-
var classModule = declaration.AsTypeDeclaration as ClassModuleDeclaration;
98-
if (classModule?.DefaultMember != null)
99-
{
100-
var parameters = (classModule.DefaultMember as IParameterizedDeclaration)?.Parameters.ToArray() ?? Enumerable.Empty<ParameterDeclaration>().ToArray();
101-
if (!parameters.Any() || parameters.All(p => p.IsOptional))
102-
{
103-
// assigned declaration has a default parameterless member, which is legally being assigned here.
104-
// might be a good idea to flag that default member assignment though...
105-
continue;
106-
}
107-
}
108-
109-
// assign declaration is an object without a default parameterless (or with all parameters optional) member - LHS needs a 'Set' keyword.
11050
result.Add(reference);
111-
continue;
112-
}
113-
114-
// assigned declaration is a variant. we need to know about the RHS of the assignment.
115-
116-
var expression = letStmtContext.expression();
117-
if (expression == null)
118-
{
119-
Debug.Assert(false, "RHS expression is empty? What's going on here?");
120-
}
121-
122-
if (expression is VBAParser.NewExprContext)
123-
{
124-
// RHS expression is newing up an object reference - LHS needs a 'Set' keyword:
125-
result.Add(reference);
126-
continue;
127-
}
128-
129-
var literalExpression = expression as VBAParser.LiteralExprContext;
130-
if (literalExpression?.literalExpression()?.literalIdentifier()?.objectLiteralIdentifier() != null)
131-
{
132-
// RHS is a 'Nothing' token - LHS needs a 'Set' keyword:
133-
result.Add(reference);
134-
continue;
135-
}
136-
137-
// todo resolve expression return type
138-
139-
var memberRefs = State.DeclarationFinder.IdentifierReferences(reference.ParentScoping.QualifiedName);
140-
var lastRef = memberRefs.LastOrDefault(r => !Equals(r, reference) && r.Context.GetAncestor<VBAParser.LetStmtContext>() == letStmtContext);
141-
if (lastRef?.Declaration.AsTypeDeclaration?.DeclarationType.HasFlag(DeclarationType.ClassModule) ?? false)
142-
{
143-
// the last reference in the expression is referring to an object type
144-
result.Add(reference);
145-
continue;
146-
}
147-
if (lastRef?.Declaration.AsTypeName == Tokens.Object)
148-
{
149-
result.Add(reference);
150-
continue;
151-
}
152-
153-
var accessibleDeclarations = State.DeclarationFinder.GetAccessibleDeclarations(reference.ParentScoping);
154-
foreach (var accessibleDeclaration in accessibleDeclarations.Where(d => d.IdentifierName == expression.GetText()))
155-
{
156-
if (accessibleDeclaration.DeclarationType.HasFlag(DeclarationType.ClassModule) || accessibleDeclaration.AsTypeName == Tokens.Object)
157-
{
158-
result.Add(reference);
159-
break;
160-
}
16151
}
16252
}
16353
}

‎Rubberduck.Inspections/VariableRequiresSetAssignmentEvaluator.cs

Lines changed: 93 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
using Rubberduck.Parsing.Symbols;
44
using Rubberduck.Parsing.VBA;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.Linq;
8+
using System.Windows.Forms;
79

810
namespace Rubberduck.Inspections
911
{
@@ -21,28 +23,110 @@ public static IEnumerable<Declaration> GetDeclarationsPotentiallyRequiringSetAss
2123

2224
public static bool RequiresSetAssignment(IdentifierReference reference, RubberduckParserState state)
2325
{
24-
//Not an assignment...definitely does not require a 'Set' assignment
2526
if (!reference.IsAssignment)
2627
{
28+
// reference isn't assigning its declaration; not interesting
2729
return false;
2830
}
29-
30-
//We know for sure it DOES NOT use 'Set'
31-
if (!MayRequireAssignmentUsingSet(reference.Declaration))
31+
32+
var setStmtContext = reference.Context.GetAncestor<VBAParser.SetStmtContext>();
33+
if (setStmtContext != null)
34+
{
35+
// assignment already has a Set keyword
36+
// (but is it misplaced? ...hmmm... beyond the scope of *this* inspection though)
37+
// if we're only ever assigning to 'Nothing', might as well flag it though
38+
if (reference.Declaration.References.Where(r => r.IsAssignment).All(r =>
39+
r.Context.GetAncestor<VBAParser.SetStmtContext>().expression().GetText() == Tokens.Nothing))
40+
{
41+
return true;
42+
}
43+
}
44+
45+
var letStmtContext = reference.Context.GetAncestor<VBAParser.LetStmtContext>();
46+
if (letStmtContext == null)
47+
{
48+
// we're probably in a For Each loop
49+
return false;
50+
}
51+
52+
var declaration = reference.Declaration;
53+
if (declaration.IsArray)
3254
{
55+
// arrays don't need a Set statement... todo figure out if array items are objects
3356
return false;
3457
}
3558

36-
//We know for sure that it DOES use 'Set'
37-
if (RequiresAssignmentUsingSet(reference.Declaration))
59+
var isObjectVariable = declaration.IsObject();
60+
var isVariant = declaration.IsUndeclared || declaration.AsTypeName == Tokens.Variant;
61+
if (!isObjectVariable && !isVariant)
62+
{
63+
return false;
64+
}
65+
66+
if (isObjectVariable)
67+
{
68+
// get the members of the returning type, a default member could make us lie otherwise
69+
var classModule = declaration.AsTypeDeclaration as ClassModuleDeclaration;
70+
if (classModule?.DefaultMember != null)
71+
{
72+
var parameters = (classModule.DefaultMember as IParameterizedDeclaration)?.Parameters.ToArray() ?? Enumerable.Empty<ParameterDeclaration>().ToArray();
73+
if (!parameters.Any() || parameters.All(p => p.IsOptional))
74+
{
75+
// assigned declaration has a default parameterless member, which is legally being assigned here.
76+
// might be a good idea to flag that default member assignment though...
77+
return false;
78+
}
79+
}
80+
81+
// assign declaration is an object without a default parameterless (or with all parameters optional) member - LHS needs a 'Set' keyword.
82+
return true;
83+
}
84+
85+
// assigned declaration is a variant. we need to know about the RHS of the assignment.
86+
87+
var expression = letStmtContext.expression();
88+
if (expression == null)
89+
{
90+
Debug.Assert(false, "RHS expression is empty? What's going on here?");
91+
}
92+
93+
if (expression is VBAParser.NewExprContext)
94+
{
95+
// RHS expression is newing up an object reference - LHS needs a 'Set' keyword:
96+
return true;
97+
}
98+
99+
var literalExpression = expression as VBAParser.LiteralExprContext;
100+
if (literalExpression?.literalExpression()?.literalIdentifier()?.objectLiteralIdentifier() != null)
38101
{
102+
// RHS is a 'Nothing' token - LHS needs a 'Set' keyword:
39103
return true;
40104
}
41105

42-
//We need to look everything to understand the RHS - the assigned reference is probably a Variant
43-
var allInterestingDeclarations = GetDeclarationsPotentiallyRequiringSetAssignment(state.AllUserDeclarations);
106+
// todo resolve expression return type
44107

45-
return ObjectOrVariantRequiresSetAssignment(reference, allInterestingDeclarations);
108+
var memberRefs = state.DeclarationFinder.IdentifierReferences(reference.ParentScoping.QualifiedName);
109+
var lastRef = memberRefs.LastOrDefault(r => !Equals(r, reference) && r.Context.GetAncestor<VBAParser.LetStmtContext>() == letStmtContext);
110+
if (lastRef?.Declaration.AsTypeDeclaration?.DeclarationType.HasFlag(DeclarationType.ClassModule) ?? false)
111+
{
112+
// the last reference in the expression is referring to an object type
113+
return true;
114+
}
115+
if (lastRef?.Declaration.AsTypeName == Tokens.Object)
116+
{
117+
return true;
118+
}
119+
120+
var accessibleDeclarations = state.DeclarationFinder.GetAccessibleDeclarations(reference.ParentScoping);
121+
foreach (var accessibleDeclaration in accessibleDeclarations.Where(d => d.IdentifierName == expression.GetText()))
122+
{
123+
if (accessibleDeclaration.DeclarationType.HasFlag(DeclarationType.ClassModule) || accessibleDeclaration.AsTypeName == Tokens.Object)
124+
{
125+
return true;
126+
}
127+
}
128+
129+
return false;
46130
}
47131

48132
private static bool MayRequireAssignmentUsingSet(Declaration declaration)

‎Rubberduck.Parsing/Symbols/DeclarationFinder.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,12 +1061,16 @@ private static bool IsSubroutineOrProperty(Declaration declaration)
10611061

10621062
public IEnumerable<IdentifierReference> IdentifierReferences(QualifiedModuleName module)
10631063
{
1064-
return _referencesByModule[module];
1064+
return _referencesByModule.TryGetValue(module, out List<IdentifierReference> value)
1065+
? value
1066+
: Enumerable.Empty<IdentifierReference>();
10651067
}
10661068

10671069
public IEnumerable<IdentifierReference> IdentifierReferences(QualifiedMemberName member)
10681070
{
1069-
return _referencesByMember[member];
1071+
return _referencesByMember.TryGetValue(member, out List<IdentifierReference> value)
1072+
? value
1073+
: Enumerable.Empty<IdentifierReference>();
10701074
}
10711075
}
10721076
}

0 commit comments

Comments
 (0)