Skip to content

Commit defda47

Browse files
committed
Add ArgumentWithIncompatibleSetTypeInspection
Also adds missing German translations for two other inspections and fixes the contexts on argument references.
1 parent 1a6df5f commit defda47

15 files changed

+421
-14
lines changed
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
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.Grammar;
7+
using Rubberduck.Parsing.Inspections;
8+
using Rubberduck.Parsing.Inspections.Abstract;
9+
using Rubberduck.Parsing.Symbols;
10+
using Rubberduck.Parsing.TypeResolvers;
11+
using Rubberduck.Parsing.VBA;
12+
using Rubberduck.Parsing.VBA.DeclarationCaching;
13+
using Rubberduck.Resources.Inspections;
14+
using Rubberduck.VBEditor;
15+
16+
namespace Rubberduck.CodeAnalysis.Inspections.Concrete
17+
{
18+
public class ArgumentWithIncompatibleObjectTypeInspection : InspectionBase
19+
{
20+
private readonly IDeclarationFinderProvider _declarationFinderProvider;
21+
private readonly ISetTypeResolver _setTypeResolver;
22+
23+
/// <summary>
24+
/// Locates arguments passed to functions or procedures for object parameters which the do not have a compatible declared type.
25+
/// </summary>
26+
/// <why>
27+
/// The VBA compiler does not check whether different object types are compatible. Instead there is a runtime error whenever the types are incompatible.
28+
/// </why>
29+
/// <example hasResult="true">
30+
/// <![CDATA[
31+
/// IInterface:
32+
///
33+
/// Public Sub DoSomething()
34+
/// End Sub
35+
///
36+
/// ------------------------------
37+
/// Class1:
38+
///
39+
///'No Implements IInterface
40+
///
41+
/// Public Sub DoSomething()
42+
/// End Sub
43+
///
44+
/// ------------------------------
45+
/// Module1:
46+
///
47+
/// Public Sub DoIt()
48+
/// Dim cls As Class1
49+
/// Set cls = New Class1
50+
/// Foo cls
51+
/// End Sub
52+
///
53+
/// Public Sub Foo(cls As IInterface)
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+
/// Set cls = New Class1
78+
/// Foo cls
79+
/// End Sub
80+
///
81+
/// Public Sub Foo(cls As IInterface)
82+
/// End Sub
83+
/// ]]>
84+
/// </example>
85+
public ArgumentWithIncompatibleObjectTypeInspection(RubberduckParserState state, ISetTypeResolver setTypeResolver)
86+
: base(state)
87+
{
88+
_declarationFinderProvider = state;
89+
_setTypeResolver = setTypeResolver;
90+
91+
//This will most likely cause a runtime error. The exceptions are rare and should be refactored or made explicit with an @Ignore annotation.
92+
Severity = CodeInspectionSeverity.Error;
93+
}
94+
95+
protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
96+
{
97+
var finder = _declarationFinderProvider.DeclarationFinder;
98+
99+
var strictlyTypedObjectParameters = finder.DeclarationsWithType(DeclarationType.Parameter)
100+
.Where(ToBeConsidered)
101+
.OfType<ParameterDeclaration>();
102+
103+
var offendingArguments = strictlyTypedObjectParameters
104+
.SelectMany(param => param.ArgumentReferences)
105+
.Select(argumentReference => ArgumentReferenceWithArgumentTypeName(argumentReference, finder))
106+
.Where(argumentReferenceWithTypeName => argumentReferenceWithTypeName.argumentTypeName != null
107+
&& !ArgumentPossiblyLegal(
108+
argumentReferenceWithTypeName.argumentReference.Declaration,
109+
argumentReferenceWithTypeName.argumentTypeName));
110+
111+
return offendingArguments
112+
.Where(argumentReferenceWithTypeName => !IsIgnored(argumentReferenceWithTypeName.Item1))
113+
.Select(argumentReference => InspectionResult(argumentReference, _declarationFinderProvider));
114+
}
115+
116+
private static bool ToBeConsidered(Declaration declaration)
117+
{
118+
return declaration != null
119+
&& declaration.AsTypeDeclaration != null
120+
&& declaration.IsObject;
121+
}
122+
123+
private (IdentifierReference argumentReference, string argumentTypeName) ArgumentReferenceWithArgumentTypeName(IdentifierReference argumentReference, DeclarationFinder finder)
124+
{
125+
return (argumentReference, ArgumentSetTypeName(argumentReference, finder));
126+
}
127+
128+
private string ArgumentSetTypeName(IdentifierReference argumentReference, DeclarationFinder finder)
129+
{
130+
return SetTypeNameOfExpression((VBAParser.ExpressionContext)argumentReference.Context, argumentReference.QualifiedModuleName, finder);
131+
}
132+
133+
private string SetTypeNameOfExpression(VBAParser.ExpressionContext expression, QualifiedModuleName containingModule, DeclarationFinder finder)
134+
{
135+
return _setTypeResolver.SetTypeName(expression, containingModule);
136+
}
137+
138+
private bool ArgumentPossiblyLegal(Declaration parameterDeclaration , string assignedTypeName)
139+
{
140+
return assignedTypeName == parameterDeclaration.FullAsTypeName
141+
|| assignedTypeName == Tokens.Variant
142+
|| assignedTypeName == Tokens.Object
143+
|| HasBaseType(parameterDeclaration, assignedTypeName)
144+
|| HasSubType(parameterDeclaration, assignedTypeName);
145+
}
146+
147+
private bool HasBaseType(Declaration declaration, string typeName)
148+
{
149+
var ownType = declaration.AsTypeDeclaration;
150+
if (ownType == null || !(ownType is ClassModuleDeclaration classType))
151+
{
152+
return false;
153+
}
154+
155+
return classType.Subtypes.Select(subtype => subtype.QualifiedModuleName.ToString()).Contains(typeName);
156+
}
157+
158+
private bool HasSubType(Declaration declaration, string typeName)
159+
{
160+
var ownType = declaration.AsTypeDeclaration;
161+
if (ownType == null || !(ownType is ClassModuleDeclaration classType))
162+
{
163+
return false;
164+
}
165+
166+
return classType.Supertypes.Select(supertype => supertype.QualifiedModuleName.ToString()).Contains(typeName);
167+
}
168+
169+
private bool IsIgnored(IdentifierReference assignment)
170+
{
171+
return assignment.IsIgnoringInspectionResultFor(AnnotationName)
172+
// Ignoring the Declaration disqualifies all assignments
173+
|| assignment.Declaration.IsIgnoringInspectionResultFor(AnnotationName);
174+
}
175+
176+
private IInspectionResult InspectionResult((IdentifierReference argumentReference, string argumentTypeName) argumentReferenceWithTypeName, IDeclarationFinderProvider declarationFinderProvider)
177+
{
178+
var (argumentReference, argumentTypeName) = argumentReferenceWithTypeName;
179+
return new IdentifierReferenceInspectionResult(this,
180+
ResultDescription(argumentReference, argumentTypeName),
181+
declarationFinderProvider,
182+
argumentReference);
183+
}
184+
185+
private string ResultDescription(IdentifierReference argumentReference, string argumentTypeName)
186+
{
187+
var parameterName = argumentReference.Declaration.IdentifierName;
188+
var parameterTypeName = argumentReference.Declaration.FullAsTypeName;
189+
var argumentExpression = argumentReference.Context.GetText();
190+
return string.Format(InspectionResults.SetAssignmentWithIncompatibleObjectTypeInspection, parameterName, parameterTypeName, argumentExpression, argumentTypeName);
191+
}
192+
}
193+
}

Rubberduck.CodeAnalysis/Inspections/Concrete/ImplementedInterfaceMemberInspection.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
4343
&& !member.IsIgnoringInspectionResultFor(AnnotationName)))
4444
.Select(result => new DeclarationInspectionResult(this,
4545
string.Format(InspectionResults.ImplementedInterfaceMemberInspection,
46+
result.QualifiedModuleName.ToString(),
4647
Resources.RubberduckUI.ResourceManager
4748
.GetString("DeclarationType_" + result.DeclarationType)
4849
.Capitalize(),

Rubberduck.Parsing/Binding/ArgumentListArgument.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ namespace Rubberduck.Parsing.Binding
99
public sealed class ArgumentListArgument
1010
{
1111
private readonly IExpressionBinding _binding;
12-
private readonly ParserRuleContext _context;
1312
private readonly Func<Declaration, IBoundExpression> _namedArgumentExpressionCreator;
1413
private readonly bool _isAddressOfArgument;
1514

@@ -21,7 +20,7 @@ public ArgumentListArgument(IExpressionBinding binding, ParserRuleContext contex
2120
public ArgumentListArgument(IExpressionBinding binding, ParserRuleContext context, ArgumentListArgumentType argumentType, Func<Declaration, IBoundExpression> namedArgumentExpressionCreator, bool isAddressOfArgument = false)
2221
{
2322
_binding = binding;
24-
_context = context;
23+
Context = context;
2524
ArgumentType = argumentType;
2625
_namedArgumentExpressionCreator = namedArgumentExpressionCreator;
2726
_isAddressOfArgument = isAddressOfArgument;
@@ -32,6 +31,7 @@ public ArgumentListArgument(IExpressionBinding binding, ParserRuleContext contex
3231
public IBoundExpression NamedArgumentExpression { get; private set; }
3332
public IBoundExpression Expression { get; private set; }
3433
public ParameterDeclaration ReferencedParameter { get; private set; }
34+
public ParserRuleContext Context { get; }
3535

3636
public void Resolve(Declaration calledProcedure, int parameterIndex, bool isArrayAccess = false)
3737
{
@@ -46,7 +46,7 @@ public void Resolve(Declaration calledProcedure, int parameterIndex, bool isArra
4646
|| ReferencedParameter != null
4747
&& !CanBeObject(ReferencedParameter)))
4848
{
49-
binding = new LetCoercionDefaultBinding(_context, binding);
49+
binding = new LetCoercionDefaultBinding(Context, binding);
5050
}
5151
}
5252

Rubberduck.Parsing/VBA/ReferenceManagement/BoundExpressionVisitor.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,7 @@ private void AddArgumentReference(
240240
Declaration parent
241241
)
242242
{
243-
var expression = argument.Expression;
244-
var callSiteContext = expression.Context;
243+
var callSiteContext = argument.Context;
245244
var identifier = callSiteContext.GetText();
246245
var selection = callSiteContext.GetSelection();
247246
var callee = argument.ReferencedParameter;

Rubberduck.Resources/Inspections/InspectionInfo.Designer.cs

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Rubberduck.Resources/Inspections/InspectionInfo.de.resx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,15 @@ Falls der Parameter 'null' sein kann, bitte dieses Auftreten ignorieren. 'null'
383383
<value>'While...Wend'-Schleifen sind aus Gründen der Abwärtskompatibilität vorhanden und wurden durch die Einführung von 'Do While...Loop'-Blöcken ersetzt, die die 'Exit Do'-Exit-Anweisung unterstützen. 'While...Wend'-Schleifen können nur beendet werden, wenn die 'While'-Bedingung erfüllt ist.</value>
384384
</data>
385385
<data name="SetAssignmentWithIncompatibleObjectTypeInspection" xml:space="preserve">
386-
<value>Der VBA-Compiler gibt keinen Fehler aus, wenn ein Objekt einer Variables Set-zugewiesen wird mit einem inkompatiblen Objecttype, d.h. deren Type weder identisch, ein Subtyp noch ein Supertyp ist. In fast allen Fällen führt dies zu einem Laufzeitfehler, der schwerer zu entdecken ist und auf einen Programmierfehler hinweist. In allen anderen Fällen kann der Quallcode so geändert werden, dass er ausschließlich Zuweisungen zwischen kompatiblen deklarierten Typen verwendet.</value>
386+
<value>Der VBA-Compiler gibt keinen Fehler aus, wenn ein Objekt einer Variables Set-zugewiesen wird mit einem inkompatiblen Objekttype, d.h. deren Typ weder identisch, ein Subtyp noch ein Supertyp ist. In fast allen Fällen führt dies zu einem Laufzeitfehler, der schwerer zu entdecken ist und auf einen Programmierfehler hinweist. In allen anderen Fällen kann der Quallcode so geändert werden, dass er ausschließlich Zuweisungen zwischen kompatiblen deklarierten Typen verwendet.</value>
387+
</data>
388+
<data name="ArgumentWithIncompatibleObjectTypeInspection" xml:space="preserve">
389+
<value>Der VBA-Compiler gibt keinen Fehler aus, wenn ein Objekt als Argument für einen Parameter übergeben wird mit einem inkompatiblen Objekttype, d.h. dessen Typ weder identisch, ein Subtyp noch ein Supertyp ist. In fast allen Fällen führt dies zu einem Laufzeitfehler, der schwerer zu entdecken ist und auf einen Programmierfehler hinweist. In allen anderen Fällen kann der Quallcode so geändert werden, dass er ausschließlich Zuweisungen zwischen kompatiblen deklarierten Typen verwendet.</value>
390+
</data>
391+
<data name="EmptyMethodInspection" xml:space="preserve">
392+
<value>Methoden ohne ausführbare Anweisungen können den Eindruck erwecken, dass sie etwas tun, was sie eigentlich nicht tun. Dies kann zu unerwartetem Verhalten führen.</value>
393+
</data>
394+
<data name="ImplementedInterfaceMemberInspection" xml:space="preserve">
395+
<value>Eine Klasse, die dafür gedacht ist von anderen Klassen als Interface genutzt zu werden, sollte gewöhnlicher Weise keine Implementierungen enthalten. Falls die Intention ist diese Klasse direkt als konkrete Klasse zu verwenden, kann dieses Inspektionsresultat ignoriert werden.</value>
387396
</data>
388397
</root>

Rubberduck.Resources/Inspections/InspectionInfo.resx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,9 @@ If the parameter can be null, ignore this inspection result; passing a null valu
389389
<value>Methods without executable statements may appear to be doing something which they actually don't, and therefore causing unexpected behaviour.</value>
390390
</data>
391391
<data name="ImplementedInterfaceMemberInspection" xml:space="preserve">
392-
<value>A class module that is meant to be used as interface for concrete classes should gnerally be abstracted of any implementations. If it is your intention to use this class module as a concrete, you can safely ignore this inspection result.</value>
392+
<value>A class module that is meant to be used as interface for concrete classes should generally be abstracted of any implementations. If it is your intention to use this class module as a concrete, you can safely ignore this inspection result.</value>
393+
</data>
394+
<data name="ArgumentWithIncompatibleObjectTypeInspection" xml:space="preserve">
395+
<value>The VBA compiler does not raise an error if an object is passed as an argument for a parameter with an incompatible declared object type, i.e. with an object type that is neither the same type, a supertype nor a subtype. Under almost all circumstances passing such an argument leads to a run-time error, which is harder to detect and indicates a bug. In all other situations the code can be changed to use only assignments between compatible declared types.</value>
393396
</data>
394397
</root>

Rubberduck.Resources/Inspections/InspectionNames.Designer.cs

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

Rubberduck.Resources/Inspections/InspectionNames.de.resx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,4 +369,13 @@
369369
<data name="ObsoleteWhileWendStatementInspection" xml:space="preserve">
370370
<value>Verwendung der veralteten 'While...Wend'-Anweisung</value>
371371
</data>
372+
<data name="ArgumentWithIncompatibleObjectTypeInspection" xml:space="preserve">
373+
<value>Argument mit nicht kompatiblem Objekttyp</value>
374+
</data>
375+
<data name="EmptyMethodInspection" xml:space="preserve">
376+
<value>Lehre Methode</value>
377+
</data>
378+
<data name="ImplementedInterfaceMemberInspection" xml:space="preserve">
379+
<value>Implementierte Methode einer Interface-Klasse</value>
380+
</data>
372381
</root>

Rubberduck.Resources/Inspections/InspectionNames.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,4 +395,7 @@
395395
<data name="ImplementedInterfaceMemberInspection" xml:space="preserve">
396396
<value>Implemented member of interface class</value>
397397
</data>
398+
<data name="ArgumentWithIncompatibleObjectTypeInspection" xml:space="preserve">
399+
<value>Argument with incompatible object type</value>
400+
</data>
398401
</root>

0 commit comments

Comments
 (0)