Skip to content

Commit 10a0572

Browse files
authored
Merge pull request #5003 from MDoerner/IncompatibleObjectTypeInspection
SetAssignmentWithIncompatibleObjectTypeInspection
2 parents aac8edc + 0ba0c90 commit 10a0572

File tree

44 files changed

+4283
-435
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+4283
-435
lines changed

Rubberduck.CodeAnalysis/Inspections/Concrete/AssignedByValParameterInspection.cs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,43 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
4848
.Cast<ParameterDeclaration>()
4949
.Where(item => !item.IsByRef
5050
&& !item.IsIgnoringInspectionResultFor(AnnotationName)
51-
&& item.References.Any(reference => reference.IsAssignment));
51+
&& item.References.Any(IsAssignmentToDeclaration));
5252

5353
return parameters
5454
.Select(param => new DeclarationInspectionResult(this,
5555
string.Format(InspectionResults.AssignedByValParameterInspection, param.IdentifierName),
5656
param));
5757
}
58+
59+
private static bool IsAssignmentToDeclaration(IdentifierReference reference)
60+
{
61+
//Todo: Review whether this is still needed once parameterless default member assignments are resolved correctly.
62+
63+
if (!reference.IsAssignment)
64+
{
65+
return false;
66+
}
67+
68+
if (reference.IsSetAssignment)
69+
{
70+
return true;
71+
}
72+
73+
var declaration = reference.Declaration;
74+
if (declaration == null)
75+
{
76+
return false;
77+
}
78+
79+
if (declaration.IsObject)
80+
{
81+
//This can only be legal with a default member access.
82+
return false;
83+
}
84+
85+
//This is not perfect in case the referenced declaration is an unbound Variant.
86+
//In that case, a default member access might occur after the run-time resolution.
87+
return true;
88+
}
5889
}
5990
}

Rubberduck.CodeAnalysis/Inspections/Concrete/MissingAttributeInspection.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Collections.Generic;
22
using System.Linq;
33
using Rubberduck.Inspections.Abstract;
4+
using Rubberduck.Inspections.Inspections.Extensions;
45
using Rubberduck.Inspections.Results;
56
using Rubberduck.Parsing;
67
using Rubberduck.Parsing.Annotations;
@@ -49,7 +50,8 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
4950
var declarationsWithAttributeAnnotations = State.DeclarationFinder.AllUserDeclarations
5051
.Where(declaration => declaration.Annotations.Any(annotation => annotation.AnnotationType.HasFlag(AnnotationType.Attribute)));
5152
var results = new List<DeclarationInspectionResult>();
52-
foreach (var declaration in declarationsWithAttributeAnnotations.Where(decl => decl.QualifiedModuleName.ComponentType != ComponentType.Document))
53+
foreach (var declaration in declarationsWithAttributeAnnotations.Where(decl => decl.QualifiedModuleName.ComponentType != ComponentType.Document
54+
&& !decl.IsIgnoringInspectionResultFor(AnnotationName)))
5355
{
5456
foreach(var annotation in declaration.Annotations.OfType<IAttributeAnnotation>())
5557
{

Rubberduck.CodeAnalysis/Inspections/Concrete/NonReturningFunctionInspection.cs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
using System.Collections.Generic;
22
using System.Linq;
33
using Rubberduck.Inspections.Abstract;
4+
using Rubberduck.Inspections.Inspections.Extensions;
45
using Rubberduck.Inspections.Results;
56
using Rubberduck.Parsing;
67
using Rubberduck.Parsing.Grammar;
78
using Rubberduck.Parsing.Inspections.Abstract;
89
using Rubberduck.Resources.Inspections;
910
using Rubberduck.Parsing.Symbols;
1011
using Rubberduck.Parsing.VBA;
12+
using Rubberduck.Parsing.VBA.Extensions;
1113

1214
namespace Rubberduck.Inspections.Concrete
1315
{
@@ -48,30 +50,31 @@ public NonReturningFunctionInspection(RubberduckParserState state)
4850

4951
protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
5052
{
51-
var declarations = UserDeclarations.ToList();
53+
var interfaceMembers = State.DeclarationFinder.FindAllInterfaceMembers().ToHashSet();
5254

53-
var interfaceMembers = State.DeclarationFinder.FindAllInterfaceMembers();
55+
var functions = State.DeclarationFinder.UserDeclarations(DeclarationType.Function)
56+
.Where(declaration => !interfaceMembers.Contains(declaration));
5457

55-
var functions = declarations
56-
.Where(declaration => ReturningMemberTypes.Contains(declaration.DeclarationType)
57-
&& !interfaceMembers.Contains(declaration)).ToList();
58-
59-
var unassigned = (from function in functions
60-
let isUdt = IsReturningUserDefinedType(function)
61-
let inScopeRefs = function.References.Where(r => r.ParentScoping.Equals(function))
62-
where (!isUdt && (!inScopeRefs.Any(r => r.IsAssignment) &&
63-
!inScopeRefs.Any(reference => IsAssignedByRefArgument(function, reference))))
64-
|| (isUdt && !IsUserDefinedTypeAssigned(function))
65-
select function)
66-
.ToList();
58+
var unassigned = functions.Where(function => IsReturningUserDefinedType(function)
59+
&& !IsUserDefinedTypeAssigned(function)
60+
|| !IsReturningUserDefinedType(function)
61+
&& !IsAssigned(function));
6762

6863
return unassigned
64+
.Where(declaration => !declaration.IsIgnoringInspectionResultFor(AnnotationName))
6965
.Select(issue =>
7066
new DeclarationInspectionResult(this,
7167
string.Format(InspectionResults.NonReturningFunctionInspection, issue.IdentifierName),
7268
issue));
7369
}
7470

71+
private bool IsAssigned(Declaration function)
72+
{
73+
var inScopeIdentifierReferences = function.References.Where(r => r.ParentScoping.Equals(function));
74+
return inScopeIdentifierReferences.Any(reference => reference.IsAssignment
75+
|| IsAssignedByRefArgument(function, reference));
76+
}
77+
7578
private bool IsAssignedByRefArgument(Declaration enclosingProcedure, IdentifierReference reference)
7679
{
7780
var argExpression = reference.Context.GetAncestor<VBAParser.ArgumentExpressionContext>();
@@ -83,7 +86,7 @@ private bool IsAssignedByRefArgument(Declaration enclosingProcedure, IdentifierR
8386
&& parameter.References.Any(r => r.IsAssignment);
8487
}
8588

86-
private bool IsReturningUserDefinedType(Declaration member)
89+
private static bool IsReturningUserDefinedType(Declaration member)
8790
{
8891
return member.AsTypeDeclaration != null &&
8992
member.AsTypeDeclaration.DeclarationType == DeclarationType.UserDefinedType;
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using Rubberduck.Inspections.Abstract;
4+
using Rubberduck.Inspections.Inspections.Extensions;
5+
using Rubberduck.Inspections.Results;
6+
using Rubberduck.Parsing;
7+
using Rubberduck.Parsing.Grammar;
8+
using Rubberduck.Parsing.Inspections;
9+
using Rubberduck.Parsing.Inspections.Abstract;
10+
using Rubberduck.Parsing.Symbols;
11+
using Rubberduck.Parsing.TypeResolvers;
12+
using Rubberduck.Parsing.VBA;
13+
using Rubberduck.Parsing.VBA.DeclarationCaching;
14+
using Rubberduck.Resources.Inspections;
15+
using Rubberduck.VBEditor;
16+
17+
namespace Rubberduck.CodeAnalysis.Inspections.Concrete
18+
{
19+
public class SetAssignmentWithIncompatibleObjectTypeInspection : InspectionBase
20+
{
21+
private readonly IDeclarationFinderProvider _declarationFinderProvider;
22+
private readonly ISetTypeResolver _setTypeResolver;
23+
24+
/// <summary>
25+
/// Locates assignments to object variables for which the RHS does not have a compatible declared type.
26+
/// </summary>
27+
/// <why>
28+
/// The VBA compiler does not check whether different object types are compatible. Instead there is a runtime error whenever the types are incompatible.
29+
/// </why>
30+
/// <example hasResult="true">
31+
/// <![CDATA[
32+
/// IInterface:
33+
///
34+
/// Public Sub DoSomething()
35+
/// End Sub
36+
///
37+
/// ------------------------------
38+
/// Class1:
39+
///
40+
///'No Implements IInterface
41+
///
42+
/// Public Sub DoSomething()
43+
/// End Sub
44+
///
45+
/// ------------------------------
46+
/// Module1:
47+
///
48+
/// Public Sub DoIt()
49+
/// Dim cls As Class1
50+
/// Dim intrfc As IInterface
51+
///
52+
/// Set cls = New Class1
53+
/// Set intrfc = cls
54+
/// End Sub
55+
/// ]]>
56+
/// </example>
57+
/// <example hasResult="false">
58+
/// <![CDATA[
59+
/// IInterface:
60+
///
61+
/// Public Sub DoSomething()
62+
/// End Sub
63+
///
64+
/// ------------------------------
65+
/// Class1:
66+
///
67+
/// Implements IInterface
68+
///
69+
/// Private Sub IInterface_DoSomething()
70+
/// End Sub
71+
///
72+
/// ------------------------------
73+
/// Module1:
74+
///
75+
/// Public Sub DoIt()
76+
/// Dim cls As Class1
77+
/// Dim intrfc As IInterface
78+
///
79+
/// Set cls = New Class1
80+
/// Set intrfc = cls
81+
/// End Sub
82+
/// ]]>
83+
/// </example>
84+
public SetAssignmentWithIncompatibleObjectTypeInspection(RubberduckParserState state, ISetTypeResolver setTypeResolver)
85+
: base(state)
86+
{
87+
_declarationFinderProvider = state;
88+
_setTypeResolver = setTypeResolver;
89+
90+
//This will most likely cause a runtime error. The exceptions are rare and should be refactored or made explicit with an @Ignore annotation.
91+
Severity = CodeInspectionSeverity.Error;
92+
}
93+
94+
protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
95+
{
96+
var finder = _declarationFinderProvider.DeclarationFinder;
97+
98+
var setAssignments = finder.AllIdentifierReferences().Where(reference => reference.IsSetAssignment);
99+
100+
var offendingAssignments = setAssignments
101+
.Where(ToBeConsidered)
102+
.Select(setAssignment => SetAssignmentWithAssignedTypeName(setAssignment, finder))
103+
.Where(setAssignmentWithAssignedTypeName => setAssignmentWithAssignedTypeName.assignedTypeName != null
104+
&& !SetAssignmentPossiblyLegal(setAssignmentWithAssignedTypeName));
105+
106+
return offendingAssignments
107+
.Where(setAssignmentWithAssignedTypeName => !IsIgnored(setAssignmentWithAssignedTypeName.setAssignment))
108+
.Select(setAssignmentWithAssignedTypeName => InspectionResult(setAssignmentWithAssignedTypeName, _declarationFinderProvider));
109+
}
110+
111+
private static bool ToBeConsidered(IdentifierReference reference)
112+
{
113+
var declaration = reference.Declaration;
114+
return declaration != null
115+
&& declaration.AsTypeDeclaration != null
116+
&& declaration.IsObject;
117+
}
118+
119+
private (IdentifierReference setAssignment, string assignedTypeName) SetAssignmentWithAssignedTypeName(IdentifierReference setAssignment, DeclarationFinder finder)
120+
{
121+
return (setAssignment, SetTypeNameOfExpression(RHS(setAssignment), setAssignment.QualifiedModuleName, finder));
122+
}
123+
124+
private VBAParser.ExpressionContext RHS(IdentifierReference setAssignment)
125+
{
126+
return setAssignment.Context.GetAncestor<VBAParser.SetStmtContext>().expression();
127+
}
128+
129+
private string SetTypeNameOfExpression(VBAParser.ExpressionContext expression, QualifiedModuleName containingModule, DeclarationFinder finder)
130+
{
131+
return _setTypeResolver.SetTypeName(expression, containingModule);
132+
}
133+
134+
private bool SetAssignmentPossiblyLegal((IdentifierReference setAssignment, string assignedTypeName) setAssignmentWithAssignedType)
135+
{
136+
var (setAssignment, assignedTypeName) = setAssignmentWithAssignedType;
137+
138+
return SetAssignmentPossiblyLegal(setAssignment.Declaration, assignedTypeName);
139+
}
140+
141+
private bool SetAssignmentPossiblyLegal(Declaration declaration, string assignedTypeName)
142+
{
143+
return assignedTypeName == declaration.FullAsTypeName
144+
|| assignedTypeName == Tokens.Variant
145+
|| assignedTypeName == Tokens.Object
146+
|| HasBaseType(declaration, assignedTypeName)
147+
|| HasSubType(declaration, assignedTypeName);
148+
}
149+
150+
private bool HasBaseType(Declaration declaration, string typeName)
151+
{
152+
var ownType = declaration.AsTypeDeclaration;
153+
if (ownType == null || !(ownType is ClassModuleDeclaration classType))
154+
{
155+
return false;
156+
}
157+
158+
return classType.Subtypes.Select(subtype => subtype.QualifiedModuleName.ToString()).Contains(typeName);
159+
}
160+
161+
private bool HasSubType(Declaration declaration, string typeName)
162+
{
163+
var ownType = declaration.AsTypeDeclaration;
164+
if (ownType == null || !(ownType is ClassModuleDeclaration classType))
165+
{
166+
return false;
167+
}
168+
169+
return classType.Supertypes.Select(supertype => supertype.QualifiedModuleName.ToString()).Contains(typeName);
170+
}
171+
172+
private bool IsIgnored(IdentifierReference assignment)
173+
{
174+
return assignment.IsIgnoringInspectionResultFor(AnnotationName)
175+
// Ignoring the Declaration disqualifies all assignments
176+
|| assignment.Declaration.IsIgnoringInspectionResultFor(AnnotationName);
177+
}
178+
179+
private IInspectionResult InspectionResult((IdentifierReference setAssignment, string assignedTypeName) setAssignmentWithAssignedType, IDeclarationFinderProvider declarationFinderProvider)
180+
{
181+
var (setAssignment, assignedTypeName) = setAssignmentWithAssignedType;
182+
return new IdentifierReferenceInspectionResult(this,
183+
ResultDescription(setAssignment, assignedTypeName),
184+
declarationFinderProvider,
185+
setAssignment);
186+
}
187+
188+
private string ResultDescription(IdentifierReference setAssignment, string assignedTypeName)
189+
{
190+
var declarationName = setAssignment.Declaration.IdentifierName;
191+
var variableTypeName = setAssignment.Declaration.FullAsTypeName;
192+
return string.Format(InspectionResults.SetAssignmentWithIncompatibleObjectTypeInspection, declarationName, variableTypeName, assignedTypeName);
193+
}
194+
}
195+
}

Rubberduck.CodeAnalysis/Rubberduck.CodeAnalysis.xml

Lines changed: 62 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)