Skip to content

Commit f499deb

Browse files
retailcoderretailcoder
authored
Merge pull request #2817 from BZngr/next
Relocate accessible declarations tests to DeclarationFinderTests
2 parents c6cede0 + 4a36e12 commit f499deb

File tree

8 files changed

+451
-441
lines changed

8 files changed

+451
-441
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ ClientBin/
130130
*.[Pp]ublish.xml
131131
*.pfx
132132
*.publishsettings
133+
*.playlist
133134

134135
# Monodevelop detritus
135136
*.userprefs

‎RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterMakeLocalCopyQuickFix.cs

Lines changed: 2 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ public AssignedByValParameterMakeLocalCopyQuickFix(Declaration target, Qualified
2727
_target = target;
2828
_dialogFactory = dialogFactory;
2929
_parserState = parserState;
30-
_forbiddenNames = GetIdentifierNamesAccessibleToProcedureContext();
31-
_localCopyVariableName = ComputeSuggestedName();
30+
_forbiddenNames = parserState.DeclarationFinder.GetDeclarationsWithIdentifiersToAvoid(target).Select(n => n.IdentifierName);
31+
_localCopyVariableName = ComputeSuggestedName();
3232
}
3333

3434
public override bool CanFixInModule { get { return false; } }
@@ -113,61 +113,5 @@ private string BuildLocalCopyAssignment()
113113
return (_target.AsTypeDeclaration is ClassModuleDeclaration ? Tokens.Set + " " : string.Empty)
114114
+ _localCopyVariableName + " = " + _target.IdentifierName;
115115
}
116-
117-
private IEnumerable<string> GetIdentifierNamesAccessibleToProcedureContext()
118-
{
119-
return _parserState.AllUserDeclarations
120-
.Where(candidateDeclaration =>
121-
(
122-
IsDeclarationInTheSameProcedure(candidateDeclaration, _target)
123-
|| IsDeclarationInTheSameModule(candidateDeclaration, _target)
124-
|| IsProjectGlobalDeclaration(candidateDeclaration, _target))
125-
).Select(declaration => declaration.IdentifierName).Distinct();
126-
}
127-
128-
private bool IsDeclarationInTheSameProcedure(Declaration candidateDeclaration, Declaration scopingDeclaration)
129-
{
130-
return candidateDeclaration.ParentScope == scopingDeclaration.ParentScope;
131-
}
132-
133-
private bool IsDeclarationInTheSameModule(Declaration candidateDeclaration, Declaration scopingDeclaration)
134-
{
135-
return candidateDeclaration.ComponentName == scopingDeclaration.ComponentName
136-
&& !IsDeclaredInMethodOrProperty(candidateDeclaration.ParentDeclaration.Context);
137-
}
138-
139-
private bool IsProjectGlobalDeclaration(Declaration candidateDeclaration, Declaration scopingDeclaration)
140-
{
141-
return candidateDeclaration.ProjectName == scopingDeclaration.ProjectName
142-
&& !(candidateDeclaration.ParentScopeDeclaration is ClassModuleDeclaration)
143-
&& (candidateDeclaration.Accessibility == Accessibility.Public
144-
|| ((candidateDeclaration.Accessibility == Accessibility.Implicit)
145-
&& (candidateDeclaration.ParentScopeDeclaration is ProceduralModuleDeclaration)));
146-
}
147-
148-
private bool IsDeclaredInMethodOrProperty(RuleContext procedureContext)
149-
{
150-
if (procedureContext is VBAParser.SubStmtContext)
151-
{
152-
return true;
153-
}
154-
else if (procedureContext is VBAParser.FunctionStmtContext)
155-
{
156-
return true;
157-
}
158-
else if (procedureContext is VBAParser.PropertyLetStmtContext)
159-
{
160-
return true;
161-
}
162-
else if (procedureContext is VBAParser.PropertyGetStmtContext)
163-
{
164-
return true;
165-
}
166-
else if (procedureContext is VBAParser.PropertySetStmtContext)
167-
{
168-
return true;
169-
}
170-
return false;
171-
}
172116
}
173117
}

‎RetailCoder.VBE/Refactorings/Rename/RenameRefactoring.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,12 @@ public void Refactor(Declaration target)
101101

102102
private void Rename()
103103
{
104-
var declaration = _state.DeclarationFinder.GetDeclarationsAccessibleToScope(_model.Target, _model.Declarations)
104+
var declaration = _state.DeclarationFinder.GetDeclarationsWithIdentifiersToAvoid(_model.Target)
105105
.Where(d => d.IdentifierName.Equals(_model.NewName, StringComparison.InvariantCultureIgnoreCase)).FirstOrDefault();
106106
if (declaration != null)
107107
{
108108
var message = string.Format(RubberduckUI.RenameDialog_ConflictingNames, _model.NewName,
109-
declaration.IdentifierName);
109+
declaration);
110110
var rename = _messageBox.Show(message, RubberduckUI.RenameDialog_Caption, MessageBoxButtons.YesNo,
111111
MessageBoxIcon.Exclamation);
112112

‎Rubberduck.Parsing/Symbols/DeclarationFinder.cs

Lines changed: 109 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -816,58 +816,138 @@ private ConcurrentBag<Declaration> FindEventHandlers(IEnumerable<Declaration> de
816816
return new ConcurrentBag<Declaration>(handlers);
817817
}
818818

819-
public IEnumerable<Declaration> GetDeclarationsAccessibleToScope(Declaration target, IEnumerable<Declaration> declarations)
819+
820+
public IEnumerable<Declaration> GetAccessibleDeclarations(Declaration target)
820821
{
821822
if (target == null) { return Enumerable.Empty<Declaration>(); }
822823

824+
var declarations = GetAllDeclarations();
825+
826+
//declarations based on Accessibility
827+
var projectDeclaration = RetrieveDeclarationType(target, DeclarationType.Project);
828+
var moduleDeclaration = GetModuleDeclaration(target);
823829
return declarations
824-
.Where(candidateDeclaration =>
825-
(
826-
IsDeclarationInTheSameProcedure(candidateDeclaration, target)
827-
|| IsDeclarationChildOfTheScope(candidateDeclaration, target)
828-
|| IsModuleLevelDeclarationOfTheScope(candidateDeclaration, target)
829-
|| IsProjectGlobalDeclaration(candidateDeclaration, target)
830-
)).Distinct();
830+
.Where(callee => AccessibilityCheck.IsAccessible(projectDeclaration, moduleDeclaration, target.ParentDeclaration, callee)).ToList();
831+
}
832+
833+
public IEnumerable<Declaration> GetDeclarationsWithIdentifiersToAvoid(Declaration target)
834+
{
835+
if (target == null) { return Enumerable.Empty<Declaration>(); }
836+
837+
var accessibleDeclarations = GetAccessibleDeclarations(target);
838+
839+
//todo: handle case where target is in a consuming module rather than the exposing module
840+
841+
//Filter accessible declarations to those that would result in name collisions or hiding
842+
var possibleConflictDeclarations = accessibleDeclarations.Where(dec =>
843+
IsInProceduralModule(dec)
844+
|| IsDeclarationInSameModuleScope(dec, target)
845+
|| IsDeclarationInSameProcedureScope(dec, target)
846+
|| dec.DeclarationType == DeclarationType.Project
847+
|| dec.DeclarationType == DeclarationType.ClassModule
848+
|| dec.DeclarationType == DeclarationType.ProceduralModule
849+
|| dec.DeclarationType == DeclarationType.UserForm
850+
).ToList();
851+
852+
//Add local variables when the target is a method or property
853+
if (IsMethodOrProperty(target))
854+
{
855+
var declarations = GetAllDeclarations();
856+
var localVariableDeclarations = declarations.Where(dec => target == dec.ParentDeclaration).ToList();
857+
possibleConflictDeclarations.AddRange(localVariableDeclarations.ToList());
858+
}
859+
860+
return possibleConflictDeclarations;
861+
}
862+
863+
private IEnumerable<Declaration> GetAllDeclarations()
864+
{
865+
List<Declaration> declarations = new List<Declaration>();
866+
foreach (var Key in _declarationsByName.Keys)
867+
{
868+
ConcurrentBag<Declaration> theDeclarations;
869+
_declarationsByName.TryGetValue(Key, out theDeclarations);
870+
declarations.AddRange(theDeclarations);
871+
}
872+
return declarations;
873+
}
874+
875+
private bool IsInProceduralModule(Declaration candidateDeclaration)
876+
{
877+
var candidateModuleDeclaration = GetModuleDeclaration(candidateDeclaration);
878+
if (null == candidateModuleDeclaration) { return false; }
879+
880+
return (candidateModuleDeclaration.DeclarationType == DeclarationType.ProceduralModule);
831881
}
832882

833-
private bool IsDeclarationInTheSameProcedure(Declaration candidateDeclaration, Declaration scopingDeclaration)
883+
private bool IsDeclarationInSameProcedureScope(Declaration candidateDeclaration, Declaration scopingDeclaration)
834884
{
835885
return candidateDeclaration.ParentScope == scopingDeclaration.ParentScope;
836886
}
837887

838-
private bool IsDeclarationChildOfTheScope(Declaration candidateDeclaration, Declaration scopingDeclaration)
888+
private bool IsChildOfScopeMethodOrProperty(Declaration candidateDeclaration, Declaration scopingDeclaration)
839889
{
840-
return scopingDeclaration == candidateDeclaration.ParentDeclaration;
890+
if (IsMethodOrProperty(scopingDeclaration))
891+
{
892+
return scopingDeclaration == candidateDeclaration.ParentDeclaration;
893+
}
894+
return false;
841895
}
842896

843-
private bool IsModuleLevelDeclarationOfTheScope(Declaration candidateDeclaration, Declaration scopingDeclaration)
897+
private bool IsDeclarationInSameModuleScope(Declaration candidateDeclaration, Declaration scopingDeclaration)
844898
{
845-
if (candidateDeclaration.ParentDeclaration == null)
899+
if (candidateDeclaration.ParentDeclaration != null)
846900
{
847-
return false;
901+
return candidateDeclaration.ComponentName == scopingDeclaration.ComponentName
902+
&& (candidateDeclaration.ParentDeclaration.DeclarationType == DeclarationType.ClassModule
903+
|| candidateDeclaration.ParentDeclaration.DeclarationType == DeclarationType.ProceduralModule);
848904
}
849-
return candidateDeclaration.ComponentName == scopingDeclaration.ComponentName
850-
&& !IsDeclaredWithinMethodOrProperty(candidateDeclaration.ParentDeclaration.Context);
905+
else
906+
return false;
851907
}
852908

853-
private bool IsProjectGlobalDeclaration(Declaration candidateDeclaration, Declaration scopingDeclaration)
909+
private bool IsMethodOrProperty(Declaration declaration)
854910
{
855-
return candidateDeclaration.ProjectName == scopingDeclaration.ProjectName
856-
&& !(candidateDeclaration.ParentScopeDeclaration is ClassModuleDeclaration)
857-
&& (candidateDeclaration.Accessibility == Accessibility.Public
858-
|| ((candidateDeclaration.Accessibility == Accessibility.Implicit)
859-
&& (candidateDeclaration.ParentScopeDeclaration is ProceduralModuleDeclaration)));
911+
if (declaration == null) { return false; }
912+
913+
return (declaration.DeclarationType == DeclarationType.PropertyGet)
914+
|| (declaration.DeclarationType == DeclarationType.PropertySet)
915+
|| (declaration.DeclarationType == DeclarationType.PropertyLet)
916+
|| (declaration.DeclarationType == DeclarationType.Procedure)
917+
|| (declaration.DeclarationType == DeclarationType.Function);
860918
}
861919

862-
private bool IsDeclaredWithinMethodOrProperty(RuleContext procedureContextCandidate)
920+
private Declaration GetModuleDeclaration(Declaration declaration)
863921
{
864-
if (procedureContextCandidate == null) { return false; }
922+
var classDeclaration = RetrieveDeclarationType(declaration, DeclarationType.ClassModule);
923+
if (null != classDeclaration)
924+
{
925+
return classDeclaration;
926+
}
927+
var moduleDeclaration = RetrieveDeclarationType(declaration, DeclarationType.ProceduralModule);
928+
if (null != moduleDeclaration)
929+
{
930+
return moduleDeclaration;
931+
}
932+
return null;
933+
}
934+
935+
private Declaration RetrieveDeclarationType(Declaration start, DeclarationType goalType)
936+
{
937+
if (start.DeclarationType == goalType) { return start; }
938+
939+
var next = start.ParentDeclaration;
940+
for (var idx = 0; idx < 10; idx++)
941+
{
942+
if (next == null) { return null; }
865943

866-
return (procedureContextCandidate is VBAParser.SubStmtContext)
867-
|| (procedureContextCandidate is VBAParser.FunctionStmtContext)
868-
|| (procedureContextCandidate is VBAParser.PropertyLetStmtContext)
869-
|| (procedureContextCandidate is VBAParser.PropertyGetStmtContext)
870-
|| (procedureContextCandidate is VBAParser.PropertySetStmtContext);
944+
if (next.DeclarationType == goalType)
945+
{
946+
return next;
947+
}
948+
next = next.ParentDeclaration;
949+
}
950+
return null;
871951
}
872952
}
873953
}

0 commit comments

Comments
 (0)