Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SetAssignmentWithIncompatibleObjectTypeInspection #5003

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a993dfd
Add initial SetAssignmentWithIncompatibleObjectTypeInspection
MDoerner Jun 2, 2019
de99bcf
Merge branch 'next' into IncompatibleObjectTypeInspection
MDoerner Jun 10, 2019
e369382
Extract set type resolution from SetAssignmentWithIncompatibleObjectT…
MDoerner Jun 13, 2019
c6635a7
Merge branch 'next' into IncompatibleObjectTypeInspection
MDoerner Aug 4, 2019
47076a6
Add support for (with) member access expressions to SetTypeResolver
MDoerner Aug 4, 2019
242cb42
Explicitly return NotAnObject from SetTypeReolver.SetTypeName for non…
MDoerner Aug 4, 2019
fd0a701
Add support for all non lExpressions to SetTypeResolver
MDoerner Aug 5, 2019
8dba19e
Prefer builtInType over lExpr in expression in grammar
MDoerner Aug 5, 2019
ae99ed2
Attach references to the called default member to the exclamation mar…
MDoerner Aug 7, 2019
d9e6679
Fix #5069
MDoerner Aug 7, 2019
57fd753
Fix resolution of dictionary access expression for default members wi…
MDoerner Aug 8, 2019
44d53c2
Add support for (with) dictionary access expressions to the Set type …
MDoerner Aug 8, 2019
3e4176e
Merge branch 'next' into IncompatibleObjectTypeInspection
MDoerner Aug 8, 2019
8148307
Add IsDefaultMemberAccess to IdentifierReference
MDoerner Aug 10, 2019
d46f190
Add support for index expressions to SetTypeResolver
MDoerner Aug 10, 2019
d024143
Introduce references for array accesses
MDoerner Aug 12, 2019
6348fff
Move band-aid for AssignmentToByValParameterInspection from resolver …
MDoerner Aug 13, 2019
39402e8
Reorder alternatives in annotationArgList in grammar
MDoerner Aug 13, 2019
02cad44
Fix resolution of default member and array accesses on dictionary acc…
MDoerner Aug 14, 2019
c5ab99e
Clarify todos and add comments
MDoerner Aug 14, 2019
426dc00
Fix named argument resolution on default member accesses
MDoerner Aug 14, 2019
7a7f2e7
Set Severity of SetAssignmentWithIncompatibleObjectTypeInspection to …
MDoerner Aug 14, 2019
8668b13
Unignore tests passing now
MDoerner Aug 14, 2019
56165d1
Merge branch 'next' into IncompatibleObjectTypeInspection
MDoerner Aug 15, 2019
2fe9ab9
Fix comments
MDoerner Aug 15, 2019
0ba0c90
Remove restriction to variables on SetAssignmentWithIncompatibleObjec…
MDoerner Aug 15, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -48,12 +48,43 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
.Cast<ParameterDeclaration>()
.Where(item => !item.IsByRef
&& !item.IsIgnoringInspectionResultFor(AnnotationName)
&& item.References.Any(reference => reference.IsAssignment));
&& item.References.Any(IsAssignmentToDeclaration));

return parameters
.Select(param => new DeclarationInspectionResult(this,
string.Format(InspectionResults.AssignedByValParameterInspection, param.IdentifierName),
param));
}

private static bool IsAssignmentToDeclaration(IdentifierReference reference)
{
//Todo: Review whether this is still needed once parameterless default member assignments are resolved correctly.

if (!reference.IsAssignment)
{
return false;
}

if (reference.IsSetAssignment)
{
return true;
}

var declaration = reference.Declaration;
if (declaration == null)
{
return false;
}

if (declaration.IsObject)
{
//This can only be legal with a default member access.
return false;
MDoerner marked this conversation as resolved.
Show resolved Hide resolved
}

//This is not perfect in case the referenced declaration is an unbound Variant.
//In that case, a default member access might occur after the run-time resolution.
return true;
}
}
}
@@ -1,6 +1,7 @@
using System.Collections.Generic;
using System.Linq;
using Rubberduck.Inspections.Abstract;
using Rubberduck.Inspections.Inspections.Extensions;
using Rubberduck.Inspections.Results;
using Rubberduck.Parsing;
using Rubberduck.Parsing.Annotations;
Expand Down Expand Up @@ -49,7 +50,8 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
var declarationsWithAttributeAnnotations = State.DeclarationFinder.AllUserDeclarations
.Where(declaration => declaration.Annotations.Any(annotation => annotation.AnnotationType.HasFlag(AnnotationType.Attribute)));
var results = new List<DeclarationInspectionResult>();
foreach (var declaration in declarationsWithAttributeAnnotations.Where(decl => decl.QualifiedModuleName.ComponentType != ComponentType.Document))
foreach (var declaration in declarationsWithAttributeAnnotations.Where(decl => decl.QualifiedModuleName.ComponentType != ComponentType.Document
&& !decl.IsIgnoringInspectionResultFor(AnnotationName)))
{
foreach(var annotation in declaration.Annotations.OfType<IAttributeAnnotation>())
{
Expand Down
@@ -1,13 +1,15 @@
using System.Collections.Generic;
using System.Linq;
using Rubberduck.Inspections.Abstract;
using Rubberduck.Inspections.Inspections.Extensions;
using Rubberduck.Inspections.Results;
using Rubberduck.Parsing;
using Rubberduck.Parsing.Grammar;
using Rubberduck.Parsing.Inspections.Abstract;
using Rubberduck.Resources.Inspections;
using Rubberduck.Parsing.Symbols;
using Rubberduck.Parsing.VBA;
using Rubberduck.Parsing.VBA.Extensions;

namespace Rubberduck.Inspections.Concrete
{
Expand Down Expand Up @@ -48,30 +50,31 @@ public NonReturningFunctionInspection(RubberduckParserState state)

protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
{
var declarations = UserDeclarations.ToList();
var interfaceMembers = State.DeclarationFinder.FindAllInterfaceMembers().ToHashSet();

var interfaceMembers = State.DeclarationFinder.FindAllInterfaceMembers();
var functions = State.DeclarationFinder.UserDeclarations(DeclarationType.Function)
.Where(declaration => !interfaceMembers.Contains(declaration));

var functions = declarations
.Where(declaration => ReturningMemberTypes.Contains(declaration.DeclarationType)
&& !interfaceMembers.Contains(declaration)).ToList();

var unassigned = (from function in functions
let isUdt = IsReturningUserDefinedType(function)
let inScopeRefs = function.References.Where(r => r.ParentScoping.Equals(function))
where (!isUdt && (!inScopeRefs.Any(r => r.IsAssignment) &&
!inScopeRefs.Any(reference => IsAssignedByRefArgument(function, reference))))
|| (isUdt && !IsUserDefinedTypeAssigned(function))
select function)
.ToList();
var unassigned = functions.Where(function => IsReturningUserDefinedType(function)
&& !IsUserDefinedTypeAssigned(function)
|| !IsReturningUserDefinedType(function)
&& !IsAssigned(function));

return unassigned
.Where(declaration => !declaration.IsIgnoringInspectionResultFor(AnnotationName))
.Select(issue =>
new DeclarationInspectionResult(this,
string.Format(InspectionResults.NonReturningFunctionInspection, issue.IdentifierName),
issue));
}

private bool IsAssigned(Declaration function)
{
var inScopeIdentifierReferences = function.References.Where(r => r.ParentScoping.Equals(function));
return inScopeIdentifierReferences.Any(reference => reference.IsAssignment
|| IsAssignedByRefArgument(function, reference));
}

private bool IsAssignedByRefArgument(Declaration enclosingProcedure, IdentifierReference reference)
{
var argExpression = reference.Context.GetAncestor<VBAParser.ArgumentExpressionContext>();
Expand All @@ -83,7 +86,7 @@ private bool IsAssignedByRefArgument(Declaration enclosingProcedure, IdentifierR
&& parameter.References.Any(r => r.IsAssignment);
}

private bool IsReturningUserDefinedType(Declaration member)
private static bool IsReturningUserDefinedType(Declaration member)
{
return member.AsTypeDeclaration != null &&
member.AsTypeDeclaration.DeclarationType == DeclarationType.UserDefinedType;
Expand Down
@@ -0,0 +1,195 @@
using System.Collections.Generic;
using System.Linq;
using Rubberduck.Inspections.Abstract;
using Rubberduck.Inspections.Inspections.Extensions;
using Rubberduck.Inspections.Results;
using Rubberduck.Parsing;
using Rubberduck.Parsing.Grammar;
using Rubberduck.Parsing.Inspections;
using Rubberduck.Parsing.Inspections.Abstract;
using Rubberduck.Parsing.Symbols;
using Rubberduck.Parsing.TypeResolvers;
using Rubberduck.Parsing.VBA;
using Rubberduck.Parsing.VBA.DeclarationCaching;
using Rubberduck.Resources.Inspections;
using Rubberduck.VBEditor;

namespace Rubberduck.CodeAnalysis.Inspections.Concrete
{
public class SetAssignmentWithIncompatibleObjectTypeInspection : InspectionBase
{
private readonly IDeclarationFinderProvider _declarationFinderProvider;
private readonly ISetTypeResolver _setTypeResolver;

/// <summary>
/// Locates assignments to object variables for which the RHS does not have a compatible declared type.
/// </summary>
/// <why>
/// The VBA compiler does not check whether different object types are compatible. Instead there is a runtime error whenever the types are incompatible.
/// </why>
/// <example hasResult="true">
/// <![CDATA[
/// IInterface:
///
/// Public Sub DoSomething()
/// End Sub
///
/// ------------------------------
/// Class1:
///
///'No Implements IInterface
///
/// Public Sub DoSomething()
/// End Sub
///
/// ------------------------------
/// Module1:
///
/// Public Sub DoIt()
/// Dim cls As Class1
/// Dim intrfc As IInterface
///
/// Set cls = New Class1
/// Set intrfc = cls
/// End Sub
/// ]]>
/// </example>
/// <example hasResult="false">
/// <![CDATA[
/// IInterface:
///
/// Public Sub DoSomething()
/// End Sub
///
/// ------------------------------
/// Class1:
///
/// Implements IInterface
///
/// Private Sub IInterface_DoSomething()
/// End Sub
///
/// ------------------------------
/// Module1:
///
/// Public Sub DoIt()
/// Dim cls As Class1
/// Dim intrfc As IInterface
///
/// Set cls = New Class1
/// Set intrfc = cls
/// End Sub
/// ]]>
/// </example>
public SetAssignmentWithIncompatibleObjectTypeInspection(RubberduckParserState state, ISetTypeResolver setTypeResolver)
: base(state)
{
_declarationFinderProvider = state;
MDoerner marked this conversation as resolved.
Show resolved Hide resolved
_setTypeResolver = setTypeResolver;

//This will most likely cause a runtime error. The exceptions are rare and should be refactored or made explicit with an @Ignore annotation.
Severity = CodeInspectionSeverity.Error;
}

protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
{
var finder = _declarationFinderProvider.DeclarationFinder;

var setAssignments = finder.AllIdentifierReferences().Where(reference => reference.IsSetAssignment);

var offendingAssignments = setAssignments
.Where(ToBeConsidered)
.Select(setAssignment => SetAssignmentWithAssignedTypeName(setAssignment, finder))
.Where(setAssignmentWithAssignedTypeName => setAssignmentWithAssignedTypeName.assignedTypeName != null
&& !SetAssignmentPossiblyLegal(setAssignmentWithAssignedTypeName));

return offendingAssignments
.Where(setAssignmentWithAssignedTypeName => !IsIgnored(setAssignmentWithAssignedTypeName.setAssignment))
.Select(setAssignmentWithAssignedTypeName => InspectionResult(setAssignmentWithAssignedTypeName, _declarationFinderProvider));
}

private static bool ToBeConsidered(IdentifierReference reference)
{
var declaration = reference.Declaration;
return declaration != null
&& declaration.AsTypeDeclaration != null
&& declaration.IsObject;
}

private (IdentifierReference setAssignment, string assignedTypeName) SetAssignmentWithAssignedTypeName(IdentifierReference setAssignment, DeclarationFinder finder)
{
return (setAssignment, SetTypeNameOfExpression(RHS(setAssignment), setAssignment.QualifiedModuleName, finder));
}

private VBAParser.ExpressionContext RHS(IdentifierReference setAssignment)
{
return setAssignment.Context.GetAncestor<VBAParser.SetStmtContext>().expression();
}

private string SetTypeNameOfExpression(VBAParser.ExpressionContext expression, QualifiedModuleName containingModule, DeclarationFinder finder)
{
return _setTypeResolver.SetTypeName(expression, containingModule);
}

private bool SetAssignmentPossiblyLegal((IdentifierReference setAssignment, string assignedTypeName) setAssignmentWithAssignedType)
{
var (setAssignment, assignedTypeName) = setAssignmentWithAssignedType;

return SetAssignmentPossiblyLegal(setAssignment.Declaration, assignedTypeName);
}

private bool SetAssignmentPossiblyLegal(Declaration declaration, string assignedTypeName)
{
return assignedTypeName == declaration.FullAsTypeName
|| assignedTypeName == Tokens.Variant
|| assignedTypeName == Tokens.Object
|| HasBaseType(declaration, assignedTypeName)
|| HasSubType(declaration, assignedTypeName);
}

private bool HasBaseType(Declaration declaration, string typeName)
{
var ownType = declaration.AsTypeDeclaration;
if (ownType == null || !(ownType is ClassModuleDeclaration classType))
{
return false;
}

return classType.Subtypes.Select(subtype => subtype.QualifiedModuleName.ToString()).Contains(typeName);
}

private bool HasSubType(Declaration declaration, string typeName)
{
var ownType = declaration.AsTypeDeclaration;
if (ownType == null || !(ownType is ClassModuleDeclaration classType))
{
return false;
}

return classType.Supertypes.Select(supertype => supertype.QualifiedModuleName.ToString()).Contains(typeName);
}

private bool IsIgnored(IdentifierReference assignment)
{
return assignment.IsIgnoringInspectionResultFor(AnnotationName)
// Ignoring the Declaration disqualifies all assignments
|| assignment.Declaration.IsIgnoringInspectionResultFor(AnnotationName);
}

private IInspectionResult InspectionResult((IdentifierReference setAssignment, string assignedTypeName) setAssignmentWithAssignedType, IDeclarationFinderProvider declarationFinderProvider)
{
var (setAssignment, assignedTypeName) = setAssignmentWithAssignedType;
return new IdentifierReferenceInspectionResult(this,
ResultDescription(setAssignment, assignedTypeName),
declarationFinderProvider,
setAssignment);
}

private string ResultDescription(IdentifierReference setAssignment, string assignedTypeName)
{
var declarationName = setAssignment.Declaration.IdentifierName;
var variableTypeName = setAssignment.Declaration.FullAsTypeName;
return string.Format(InspectionResults.SetAssignmentWithIncompatibleObjectTypeInspection, declarationName, variableTypeName, assignedTypeName);
}
}
}
62 changes: 62 additions & 0 deletions Rubberduck.CodeAnalysis/Rubberduck.CodeAnalysis.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.