Skip to content

Commit f4753ef

Browse files
retailcoderretailcoder
authored
Merge pull request #2398 from MDoerner/FixesAndRefactorings
Fix to AccessibilityCheck and refactorings
2 parents 5cf9cfa + 70dd8b7 commit f4753ef

File tree

8 files changed

+813
-128
lines changed

8 files changed

+813
-128
lines changed

‎Rubberduck.Parsing/Annotations/VBAParserAnnotationFactory.cs‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,29 @@ public VBAParserAnnotationFactory()
2828
public IAnnotation Create(VBAParser.AnnotationContext context, QualifiedSelection qualifiedSelection)
2929
{
3030
string annotationName = context.annotationName().GetText();
31-
List<string> parameters = new List<string>();
32-
var argList = context.annotationArgList();
33-
if (argList != null)
31+
List<string> parameters = AnnotationParametersFromContext(context);
32+
return CreateAnnotation(annotationName, parameters, qualifiedSelection);
33+
}
34+
35+
private static List<string> AnnotationParametersFromContext(VBAParser.AnnotationContext context)
3436
{
35-
parameters.AddRange(argList.annotationArg().Select(arg => arg.GetText()));
37+
List<string> parameters = new List<string>();
38+
var argList = context.annotationArgList();
39+
if (argList != null)
40+
{
41+
parameters.AddRange(argList.annotationArg().Select(arg => arg.GetText()));
42+
}
43+
return parameters;
3644
}
37-
Type annotationCLRType = null;
38-
if (_creators.TryGetValue(annotationName.ToUpperInvariant(), out annotationCLRType))
45+
46+
private IAnnotation CreateAnnotation(string annotationName, List<string> parameters, QualifiedSelection qualifiedSelection)
3947
{
40-
return (IAnnotation)Activator.CreateInstance(annotationCLRType, qualifiedSelection, parameters);
48+
Type annotationCLRType = null;
49+
if (_creators.TryGetValue(annotationName.ToUpperInvariant(), out annotationCLRType))
50+
{
51+
return (IAnnotation)Activator.CreateInstance(annotationCLRType, qualifiedSelection, parameters);
52+
}
53+
return null;
4154
}
42-
return null;
43-
}
4455
}
4556
}

‎Rubberduck.Parsing/Binding/DefaultBindingContext.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ public IBoundExpression Resolve(Declaration module, Declaration parent, ParserRu
3434
public IExpressionBinding BuildTree(Declaration module, Declaration parent, ParserRuleContext expression, IBoundExpression withBlockVariable, StatementResolutionContext statementContext)
3535
{
3636
dynamic dynamicExpression = expression;
37-
var type = expression.GetType();
3837
return Visit(module, parent, dynamicExpression, withBlockVariable, statementContext);
3938
}
4039

‎Rubberduck.Parsing/Binding/SimpleNameDefaultBinding.cs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -350,25 +350,34 @@ private bool IsValidMatch(Declaration match, string name)
350350
{
351351
return true;
352352
}
353-
var functionSubroutinePropertyGet = match.DeclarationType == DeclarationType.Function
354-
|| match.DeclarationType == DeclarationType.Procedure
355-
|| match.DeclarationType == DeclarationType.PropertyGet;
356-
if (!functionSubroutinePropertyGet)
353+
if (!IsFunctionSubroutinePropertyGet(match))
357354
{
358355
return true;
359356
}
360357
if (((IDeclarationWithParameter)match).Parameters.Count() > 0)
361358
{
362359
return true;
363360
}
364-
if (match.AsTypeName != null
365-
&& match.AsTypeName.ToUpperInvariant() != "VARIANT"
366-
&& match.AsTypeName.ToUpperInvariant() != "OBJECT"
367-
&& match.AsTypeIsBaseType)
361+
if (IsTypeDeclarationOfSpecificBaseType(match))
368362
{
369363
return false;
370364
}
371365
return true;
372366
}
367+
368+
private static bool IsFunctionSubroutinePropertyGet(Declaration match)
369+
{
370+
return match.DeclarationType == DeclarationType.Function
371+
|| match.DeclarationType == DeclarationType.Procedure
372+
|| match.DeclarationType == DeclarationType.PropertyGet;
373+
}
374+
375+
private static bool IsTypeDeclarationOfSpecificBaseType(Declaration match)
376+
{
377+
return match.AsTypeName != null
378+
&& match.AsTypeName.ToUpperInvariant() != "VARIANT"
379+
&& match.AsTypeName.ToUpperInvariant() != "OBJECT"
380+
&& match.AsTypeIsBaseType;
381+
}
373382
}
374383
}

‎Rubberduck.Parsing/Symbols/AccessibilityCheck.cs‎

Lines changed: 71 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -4,94 +4,104 @@ public static class AccessibilityCheck
44
{
55
public static bool IsAccessible(Declaration callingProject, Declaration callingModule, Declaration callingParent, Declaration callee)
66
{
7-
if (callee.DeclarationType.HasFlag(DeclarationType.Project))
8-
{
9-
return true;
10-
}
11-
if (callee.DeclarationType.HasFlag(DeclarationType.Module))
12-
{
13-
return IsModuleAccessible(callingProject, callingModule, callee);
14-
}
15-
return IsMemberAccessible(callingProject, callingModule, callingParent, callee);
7+
return callee != null
8+
&& (callee.DeclarationType.HasFlag(DeclarationType.Project)
9+
|| (callee.DeclarationType.HasFlag(DeclarationType.Module) && IsModuleAccessible(callingProject, callingModule, callee))
10+
|| (!callee.DeclarationType.HasFlag(DeclarationType.Module) && IsMemberAccessible(callingProject, callingModule, callingParent, callee)));
1611
}
1712

13+
1814
public static bool IsModuleAccessible(Declaration callingProject, Declaration callingModule, Declaration calleeModule)
1915
{
20-
bool validAccessibility = IsValidAccessibility(calleeModule);
21-
bool enclosingModule = callingModule.Equals(calleeModule);
22-
if (enclosingModule)
23-
{
24-
return true;
25-
}
26-
bool sameProject = callingModule.ParentScopeDeclaration.Equals(calleeModule.ParentScopeDeclaration);
27-
if (sameProject)
28-
{
29-
return validAccessibility;
30-
}
31-
if (calleeModule.DeclarationType.HasFlag(DeclarationType.ProceduralModule))
16+
return calleeModule != null
17+
&& (IsTheSameModule(callingModule, calleeModule)
18+
|| IsEnclosingProject(callingProject, calleeModule)
19+
|| (calleeModule.DeclarationType.HasFlag(DeclarationType.ProceduralModule) && !((ProceduralModuleDeclaration)calleeModule).IsPrivateModule)
20+
|| (!calleeModule.DeclarationType.HasFlag(DeclarationType.ProceduralModule) && ((ClassModuleDeclaration)calleeModule).IsExposed));
21+
}
22+
23+
private static bool IsTheSameModule(Declaration callingModule, Declaration calleeModule)
3224
{
33-
bool isPrivate = ((ProceduralModuleDeclaration)calleeModule).IsPrivateModule;
34-
return validAccessibility && !isPrivate;
25+
return calleeModule.Equals(callingModule);
3526
}
36-
else
27+
28+
private static bool IsEnclosingProject(Declaration callingProject, Declaration calleeModule)
3729
{
38-
bool isExposed = calleeModule != null && ((ClassModuleDeclaration)calleeModule).IsExposed;
39-
return validAccessibility && isExposed;
30+
return calleeModule.ParentScopeDeclaration.Equals(callingProject);
4031
}
41-
}
4232

43-
public static bool IsValidAccessibility(Declaration moduleOrMember)
44-
{
45-
return moduleOrMember != null
46-
&& (moduleOrMember.Accessibility == Accessibility.Global
47-
|| moduleOrMember.Accessibility == Accessibility.Public
48-
|| moduleOrMember.Accessibility == Accessibility.Friend
49-
|| moduleOrMember.Accessibility == Accessibility.Implicit);
50-
}
33+
5134

5235
public static bool IsMemberAccessible(Declaration callingProject, Declaration callingModule, Declaration callingParent, Declaration calleeMember)
5336
{
54-
if (IsEnclosingModule(callingModule, calleeMember))
37+
if (calleeMember == null)
5538
{
56-
return true;
57-
}
58-
var callerIsSubroutineOrProperty = callingParent.DeclarationType.HasFlag(DeclarationType.Property)
59-
|| callingParent.DeclarationType.HasFlag(DeclarationType.Function)
60-
|| callingParent.DeclarationType.HasFlag(DeclarationType.Procedure);
61-
var calleeHasSameParent = callingParent.Equals(callingParent.ParentScopeDeclaration);
62-
if (callerIsSubroutineOrProperty && calleeHasSameParent)
39+
return false;
40+
}
41+
if (IsEnclosingModuleOfInstanceMember(callingModule, calleeMember)
42+
|| IsLocalMemberOfTheCallingSubroutineOrProperty(callingParent, calleeMember))
6343
{
64-
return calleeHasSameParent;
44+
return true;
6545
}
6646
var memberModule = Declaration.GetModuleParent(calleeMember);
67-
if (IsModuleAccessible(callingProject, callingModule, memberModule))
47+
return IsModuleAccessible(callingProject, callingModule, memberModule)
48+
&& (calleeMember.DeclarationType.HasFlag(DeclarationType.EnumerationMember)
49+
|| calleeMember.DeclarationType.HasFlag(DeclarationType.UserDefinedTypeMember)
50+
|| HasPublicScope(calleeMember)
51+
|| (IsEnclosingProject(callingProject, memberModule) && IsAccessibleThroughoutTheSameProject(calleeMember)));
52+
}
53+
54+
private static bool IsEnclosingModuleOfInstanceMember(Declaration callingModule, Declaration calleeMember)
6855
{
69-
if (calleeMember.DeclarationType.HasFlag(DeclarationType.EnumerationMember) || calleeMember.DeclarationType.HasFlag(DeclarationType.UserDefinedTypeMember))
56+
if (callingModule.Equals(calleeMember.ParentScopeDeclaration))
7057
{
71-
return IsValidAccessibility(calleeMember.ParentDeclaration);
58+
return true;
7259
}
73-
else
60+
foreach (var supertype in ClassModuleDeclaration.GetSupertypes(callingModule))
7461
{
75-
return IsValidAccessibility(calleeMember);
62+
if (IsEnclosingModuleOfInstanceMember(supertype, calleeMember))
63+
{
64+
return true;
65+
}
7666
}
67+
return false;
7768
}
78-
return false;
79-
}
8069

81-
private static bool IsEnclosingModule(Declaration callingModule, Declaration calleeMember)
82-
{
83-
if (callingModule.Equals(calleeMember.ParentScopeDeclaration))
70+
private static bool IsLocalMemberOfTheCallingSubroutineOrProperty(Declaration callingParent, Declaration calleeMember)
8471
{
85-
return true;
72+
return IsSubroutineOrProperty(callingParent) && CaleeHasSameParentScopeAsCaller(callingParent, calleeMember);
8673
}
87-
foreach (var supertype in ClassModuleDeclaration.GetSupertypes(callingModule))
88-
{
89-
if (IsEnclosingModule(supertype, calleeMember))
74+
75+
private static bool IsSubroutineOrProperty(Declaration decl)
9076
{
91-
return true;
77+
return decl.DeclarationType.HasFlag(DeclarationType.Property)
78+
|| decl.DeclarationType.HasFlag(DeclarationType.Function)
79+
|| decl.DeclarationType.HasFlag(DeclarationType.Procedure);
80+
}
81+
82+
private static bool CaleeHasSameParentScopeAsCaller(Declaration callingParent, Declaration calleeMember)
83+
{
84+
return callingParent.Equals(calleeMember.ParentScopeDeclaration);
9285
}
86+
87+
private static bool HasPublicScope(Declaration member)
88+
{
89+
return member.Accessibility == Accessibility.Public
90+
|| member.Accessibility == Accessibility.Global
91+
|| (member.Accessibility == Accessibility.Implicit && IsPublicByDefault(member));
92+
}
93+
94+
private static bool IsPublicByDefault(Declaration member)
95+
{
96+
return IsSubroutineOrProperty(member)
97+
|| member.DeclarationType.HasFlag(DeclarationType.Enumeration)
98+
|| member.DeclarationType.HasFlag(DeclarationType.UserDefinedType);
99+
}
100+
101+
private static bool IsAccessibleThroughoutTheSameProject(Declaration member)
102+
{
103+
return HasPublicScope(member)
104+
|| member.Accessibility == Accessibility.Friend;
93105
}
94-
return false;
95-
}
96106
}
97107
}

‎Rubberduck.Parsing/Symbols/ClassModuleDeclaration.cs‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -70,23 +70,34 @@ public bool IsExposed
7070
{
7171
return _isExposed.Value;
7272
}
73-
// TODO: Find out if there's info about "being exposed" in type libraries.
74-
// We take the conservative approach of treating all type library modules as exposed.
7573
if (IsBuiltIn)
7674
{
77-
_isExposed = true;
75+
_isExposed = IsExposedForBuiltInModules();
7876
return _isExposed.Value;
7977
}
80-
var attributeIsExposed = false;
78+
_isExposed = HasAttribute("VB_Exposed");
79+
return _isExposed.Value;
80+
}
81+
}
82+
83+
// TODO: Find out if there's info about "being exposed" in type libraries.
84+
// We take the conservative approach of treating all type library modules as exposed.
85+
private static bool IsExposedForBuiltInModules()
86+
{
87+
return true;
88+
}
89+
90+
private bool HasAttribute(string attributeName)
91+
{
92+
var hasAttribute = false;
8193
IEnumerable<string> value;
82-
if (Attributes.TryGetValue("VB_Exposed", out value))
94+
if (Attributes.TryGetValue(attributeName, out value))
8395
{
84-
attributeIsExposed = value.Single() == "True";
96+
hasAttribute = value.Single() == "True";
8597
}
86-
_isExposed = attributeIsExposed;
87-
return _isExposed.Value;
98+
return hasAttribute;
8899
}
89-
}
100+
90101

91102
private bool? _isGlobal;
92103
public bool IsGlobalClassModule
@@ -97,30 +108,25 @@ public bool IsGlobalClassModule
97108
{
98109
return _isGlobal.Value;
99110
}
111+
_isGlobal = HasAttribute("VB_GlobalNamespace") || IsGlobalFromSubtypes();
112+
return _isGlobal.Value;
113+
}
114+
}
100115

101-
var attributeIsGlobalClassModule = false;
102-
IEnumerable<string> value;
103-
if (Attributes.TryGetValue("VB_GlobalNamespace", out value))
104-
{
105-
attributeIsGlobalClassModule = value.Single() == "True";
106-
}
107-
_isGlobal = attributeIsGlobalClassModule;
108-
109-
if (!_isGlobal.Value)
116+
private bool IsGlobalFromSubtypes()
117+
{
118+
var isGlobal = false;
119+
foreach (var type in Subtypes)
110120
{
111-
foreach (var type in Subtypes)
121+
if (type is ClassModuleDeclaration && ((ClassModuleDeclaration)type).IsGlobalClassModule)
112122
{
113-
if (type is ClassModuleDeclaration && ((ClassModuleDeclaration) type).IsGlobalClassModule)
114-
{
115-
_isGlobal = true;
116-
break;
117-
}
123+
isGlobal = true;
124+
break;
118125
}
119126
}
120-
121-
return _isGlobal.Value;
127+
return isGlobal;
122128
}
123-
}
129+
124130

125131
private bool? _hasPredeclaredId;
126132
/// <summary>
@@ -135,18 +141,12 @@ public bool HasPredeclaredId
135141
{
136142
return _hasPredeclaredId.Value;
137143
}
138-
139-
var attributeHasDefaultInstanceVariable = false;
140-
IEnumerable<string> value;
141-
if (Attributes.TryGetValue("VB_PredeclaredId", out value))
142-
{
143-
attributeHasDefaultInstanceVariable = value.Single() == "True";
144-
}
145-
_hasPredeclaredId = attributeHasDefaultInstanceVariable;
144+
_hasPredeclaredId = HasAttribute("VB_PredeclaredId");
146145
return _hasPredeclaredId.Value;
147146
}
148147
}
149148

149+
150150
public bool HasDefaultInstanceVariable
151151
{
152152
get

0 commit comments

Comments
 (0)