Skip to content

Commit ff42e3f

Browse files
MDoernerMDoerner
committed
Merge branch 'next' into HackFor2773
Merging recent changes.
2 parents 57098fb + 817509e commit ff42e3f

File tree

9 files changed

+320
-103
lines changed

9 files changed

+320
-103
lines changed

‎RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,53 +23,61 @@ public ParameterCanBeByValInspection(RubberduckParserState state)
2323

2424
public override IEnumerable<InspectionResultBase> GetInspectionResults()
2525
{
26-
var declarations = UserDeclarations.ToList();
26+
var declarations = UserDeclarations.ToArray();
2727
var issues = new List<ParameterCanBeByValInspectionResult>();
2828

29-
var interfaceDeclarationMembers = declarations.FindInterfaceMembers().ToList();
30-
var interfaceScopes = declarations.FindInterfaceImplementationMembers().Concat(interfaceDeclarationMembers).Select(s => s.Scope);
29+
var interfaceDeclarationMembers = declarations.FindInterfaceMembers().ToArray();
30+
var interfaceScopes = declarations.FindInterfaceImplementationMembers().Concat(interfaceDeclarationMembers).Select(s => s.Scope).ToArray();
3131

3232
issues.AddRange(GetResults(declarations, interfaceDeclarationMembers));
3333

34-
var eventMembers = declarations.Where(item => !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event).ToList();
35-
var formEventHandlerScopes = State.FindFormEventHandlers().Select(handler => handler.Scope);
36-
var eventHandlerScopes = State.DeclarationFinder.FindEventHandlers().Concat(declarations.FindUserEventHandlers()).Select(e => e.Scope);
34+
var eventMembers = declarations.Where(item => !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event).ToArray();
35+
var formEventHandlerScopes = State.FindFormEventHandlers().Select(handler => handler.Scope).ToArray();
36+
var eventHandlerScopes = State.DeclarationFinder.FindEventHandlers().Concat(declarations.FindUserEventHandlers()).Select(e => e.Scope).ToArray();
3737
var eventScopes = eventMembers.Select(s => s.Scope)
3838
.Concat(formEventHandlerScopes)
39-
.Concat(eventHandlerScopes);
39+
.Concat(eventHandlerScopes)
40+
.ToArray();
4041

4142
issues.AddRange(GetResults(declarations, eventMembers));
4243

4344
var declareScopes = declarations.Where(item =>
4445
item.DeclarationType == DeclarationType.LibraryFunction
4546
|| item.DeclarationType == DeclarationType.LibraryProcedure)
46-
.Select(e => e.Scope);
47+
.Select(e => e.Scope)
48+
.ToArray();
4749

48-
issues.AddRange(declarations.Where(declaration =>
50+
issues.AddRange(declarations.OfType<ParameterDeclaration>()
51+
.Where(declaration => IsIssue(declaration, declarations, declareScopes, eventScopes, interfaceScopes))
52+
.Select(issue => new ParameterCanBeByValInspectionResult(this, State, issue, issue.Context, issue.QualifiedName)));
53+
54+
return issues;
55+
}
56+
57+
private bool IsIssue(ParameterDeclaration declaration, Declaration[] userDeclarations, string[] declareScopes, string[] eventScopes, string[] interfaceScopes)
58+
{
59+
var isIssue =
4960
!declaration.IsArray
50-
&& (declaration.AsTypeDeclaration == null || declaration.AsTypeDeclaration.DeclarationType != DeclarationType.UserDefinedType)
61+
&& !declaration.IsParamArray
62+
&& (declaration.IsByRef || declaration.IsImplicitByRef)
63+
&& (declaration.AsTypeDeclaration == null || declaration.AsTypeDeclaration.DeclarationType != DeclarationType.ClassModule && declaration.AsTypeDeclaration.DeclarationType != DeclarationType.UserDefinedType && declaration.AsTypeDeclaration.DeclarationType != DeclarationType.Enumeration)
5164
&& !declareScopes.Contains(declaration.ParentScope)
5265
&& !eventScopes.Contains(declaration.ParentScope)
5366
&& !interfaceScopes.Contains(declaration.ParentScope)
54-
&& declaration.DeclarationType == DeclarationType.Parameter
55-
&& ((VBAParser.ArgContext)declaration.Context).BYVAL() == null
56-
&& !IsUsedAsByRefParam(declarations, declaration)
57-
&& !declaration.References.Any(reference => reference.IsAssignment))
58-
.Select(issue => new ParameterCanBeByValInspectionResult(this, State, issue, issue.Context, issue.QualifiedName)));
59-
60-
return issues;
67+
&& !IsUsedAsByRefParam(userDeclarations, declaration)
68+
&& (!declaration.References.Any() || !declaration.References.Any(reference => reference.IsAssignment));
69+
return isIssue;
6170
}
6271

63-
private IEnumerable<ParameterCanBeByValInspectionResult> GetResults(List<Declaration> declarations, List<Declaration> declarationMembers)
72+
private IEnumerable<ParameterCanBeByValInspectionResult> GetResults(Declaration[] declarations, Declaration[] declarationMembers)
6473
{
6574
foreach (var declaration in declarationMembers)
6675
{
67-
var declarationParameters =
68-
declarations.Where(d => d.DeclarationType == DeclarationType.Parameter &&
69-
Equals(d.ParentDeclaration, declaration))
70-
.OrderBy(o => o.Selection.StartLine)
71-
.ThenBy(t => t.Selection.StartColumn)
72-
.ToList();
76+
var declarationParameters = declarations.OfType<ParameterDeclaration>()
77+
.Where(d => Equals(d.ParentDeclaration, declaration))
78+
.OrderBy(o => o.Selection.StartLine)
79+
.ThenBy(t => t.Selection.StartColumn)
80+
.ToList();
7381

7482
if (!declarationParameters.Any()) { continue; }
7583
var parametersAreByRef = declarationParameters.Select(s => true).ToList();
@@ -80,12 +88,11 @@ private IEnumerable<ParameterCanBeByValInspectionResult> GetResults(List<Declara
8088

8189
foreach (var member in members)
8290
{
83-
var parameters =
84-
declarations.Where(d => d.DeclarationType == DeclarationType.Parameter &&
85-
Equals(d.ParentDeclaration, member))
86-
.OrderBy(o => o.Selection.StartLine)
87-
.ThenBy(t => t.Selection.StartColumn)
88-
.ToList();
91+
var parameters = declarations.OfType<ParameterDeclaration>()
92+
.Where(d => Equals(d.ParentDeclaration, member))
93+
.OrderBy(o => o.Selection.StartLine)
94+
.ThenBy(t => t.Selection.StartColumn)
95+
.ToList();
8996

9097
for (var i = 0; i < parameters.Count; i++)
9198
{

‎Rubberduck.Parsing/Symbols/DeclarationFinder.cs

Lines changed: 36 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,11 @@ public class DeclarationFinder
5252
private readonly Lazy<ConcurrentDictionary<Declaration, Declaration[]>> _handlersByWithEventsField;
5353
private readonly Lazy<ConcurrentDictionary<VBAParser.ImplementsStmtContext, Declaration[]>> _membersByImplementsContext;
5454
private readonly Lazy<ConcurrentDictionary<Declaration, Declaration[]>> _interfaceMembers;
55+
private Lazy<List<Declaration>> _nonBaseAsType;
56+
private readonly Lazy<ConcurrentBag<Declaration>> _eventHandlers;
57+
private readonly Lazy<ConcurrentBag<Declaration>> _classes;
5558

56-
private static readonly object ThreadLock = new object();
59+
private readonly object threadLock = new object();
5760

5861
public DeclarationFinder(IReadOnlyList<Declaration> declarations, IEnumerable<IAnnotation> annotations, IReadOnlyList<UnboundMemberDeclaration> unresolvedMemberDeclarations, IHostApplication hostApp = null)
5962
{
@@ -133,8 +136,16 @@ public DeclarationFinder(IReadOnlyList<Declaration> declarations, IEnumerable<IA
133136
});
134137

135138
_membersByImplementsContext = new Lazy<ConcurrentDictionary<VBAParser.ImplementsStmtContext, Declaration[]>>(() =>
136-
new ConcurrentDictionary<VBAParser.ImplementsStmtContext, Declaration[]>(
137-
implementableMembers.ToDictionary(item => item.Context, item => item.Members)), true);
139+
new ConcurrentDictionary<VBAParser.ImplementsStmtContext, Declaration[]>(
140+
implementableMembers.ToDictionary(item => item.Context, item => item.Members)), true);
141+
142+
_nonBaseAsType = new Lazy<List<Declaration>>(() =>
143+
_declarations.AllValues().Where(d =>
144+
!string.IsNullOrWhiteSpace(d.AsTypeName)
145+
&& !d.AsTypeIsBaseType
146+
&& d.DeclarationType != DeclarationType.Project
147+
&& d.DeclarationType != DeclarationType.ProceduralModule).ToList()
148+
,true);
138149
}
139150

140151
public IEnumerable<Declaration> FreshUndeclared
@@ -152,38 +163,22 @@ public IEnumerable<Declaration> Members(QualifiedModuleName module)
152163
return _declarations[module];
153164
}
154165

155-
private IEnumerable<Declaration> _nonBaseAsType;
156166
public IEnumerable<Declaration> FindDeclarationsWithNonBaseAsType()
157167
{
158-
lock (ThreadLock)
159-
{
160-
return _nonBaseAsType ?? (_nonBaseAsType = _declarations.AllValues().Where(d =>
161-
!string.IsNullOrWhiteSpace(d.AsTypeName)
162-
&& !d.AsTypeIsBaseType
163-
&& d.DeclarationType != DeclarationType.Project
164-
&& d.DeclarationType != DeclarationType.ProceduralModule).ToList());
165-
}
166-
}
168+
return _nonBaseAsType.Value;
167169

168-
private readonly Lazy<ConcurrentBag<Declaration>> _eventHandlers;
170+
}
171+
169172
public IEnumerable<Declaration> FindEventHandlers()
170173
{
171-
lock (ThreadLock)
172-
{
173-
return _eventHandlers.Value;
174-
}
174+
return _eventHandlers.Value;
175175
}
176176

177-
private readonly Lazy<ConcurrentBag<Declaration>> _classes;
178-
179177
public IEnumerable<Declaration> Classes
180178
{
181179
get
182180
{
183-
lock (ThreadLock)
184-
{
185-
return _classes.Value;
186-
}
181+
return _classes.Value;
187182
}
188183
}
189184

@@ -193,10 +188,7 @@ public IEnumerable<Declaration> Projects
193188
{
194189
get
195190
{
196-
lock (ThreadLock)
197-
{
198-
return _projects.Value;
199-
}
191+
return _projects.Value;
200192
}
201193
}
202194

@@ -214,10 +206,7 @@ public IEnumerable<Declaration> UserDeclarations(DeclarationType type)
214206

215207
public IEnumerable<UnboundMemberDeclaration> FreshUnresolvedMemberDeclarations()
216208
{
217-
lock (ThreadLock)
218-
{
219-
return _newUnresolved.ToArray();
220-
}
209+
return _newUnresolved.ToArray(); //This does not need a lock because enumerators over a ConcurrentBag uses a snapshot.
221210
}
222211

223212
public IEnumerable<UnboundMemberDeclaration> UnresolvedMemberDeclarations()
@@ -253,25 +242,27 @@ public IEnumerable<Declaration> FindAllInterfaceImplementingMembers()
253242

254243
public Declaration FindParameter(Declaration procedure, string parameterName)
255244
{
256-
return _parametersByParent[procedure].SingleOrDefault(parameter => parameter.IdentifierName == parameterName);
245+
ConcurrentBag<Declaration> parameters;
246+
return _parametersByParent.TryGetValue(procedure, out parameters)
247+
? parameters.SingleOrDefault(parameter => parameter.IdentifierName == parameterName)
248+
: null;
257249
}
258250

259251
public IEnumerable<Declaration> FindMemberMatches(Declaration parent, string memberName)
260252
{
261253
ConcurrentBag<Declaration> children;
262-
if (_declarations.TryGetValue(parent.QualifiedName.QualifiedModuleName, out children))
263-
{
264-
return children.Where(item => item.DeclarationType.HasFlag(DeclarationType.Member)
265-
&& item.IdentifierName == memberName).ToList();
266-
}
267-
268-
return Enumerable.Empty<Declaration>();
254+
return _declarations.TryGetValue(parent.QualifiedName.QualifiedModuleName, out children)
255+
? children.Where(item => item.DeclarationType.HasFlag(DeclarationType.Member)
256+
&& item.IdentifierName == memberName).ToList()
257+
: Enumerable.Empty<Declaration>();
269258
}
270259

271260
public IEnumerable<IAnnotation> FindAnnotations(QualifiedModuleName module)
272261
{
273262
ConcurrentBag<IAnnotation> result;
274-
return _annotations.TryGetValue(module, out result) ? result : Enumerable.Empty<IAnnotation>();
263+
return _annotations.TryGetValue(module, out result)
264+
? result
265+
: Enumerable.Empty<IAnnotation>();
275266
}
276267

277268
public bool IsMatch(string declarationName, string potentialMatchName)
@@ -326,7 +317,8 @@ public Declaration FindProject(string name, Declaration currentScope = null)
326317
Declaration result = null;
327318
try
328319
{
329-
result = MatchName(name).SingleOrDefault(project => project.DeclarationType.HasFlag(DeclarationType.Project)
320+
result = MatchName(name).SingleOrDefault(project =>
321+
project.DeclarationType.HasFlag(DeclarationType.Project)
330322
&& (currentScope == null || project.ProjectId == currentScope.ProjectId));
331323
}
332324
catch (InvalidOperationException exception)
@@ -337,7 +329,7 @@ public Declaration FindProject(string name, Declaration currentScope = null)
337329
return result;
338330
}
339331

340-
public Declaration FindStdModule(string name, Declaration parent = null, bool includeBuiltIn = false)
332+
public Declaration FindStdModule(string name, Declaration parent, bool includeBuiltIn = false)
341333
{
342334
Debug.Assert(parent != null);
343335
Declaration result = null;
@@ -356,7 +348,7 @@ public Declaration FindStdModule(string name, Declaration parent = null, bool in
356348
return result;
357349
}
358350

359-
public Declaration FindClassModule(string name, Declaration parent = null, bool includeBuiltIn = false)
351+
public Declaration FindClassModule(string name, Declaration parent, bool includeBuiltIn = false)
360352
{
361353
Debug.Assert(parent != null);
362354
Declaration result = null;

‎Rubberduck.Parsing/VBA/ComponentParseTask.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public void Start(CancellationToken token)
9292
}
9393
catch (SyntaxErrorException exception)
9494
{
95-
//System.Diagnostics.Debug.Assert(false, "A RecognitionException should be notified of, not thrown as a SyntaxErrorException. This lets the parser recover from parse errors.");
95+
Logger.Warn("Syntax error; offending token '{0}' at line {1}, column {2} in module {3}.", exception.OffendingSymbol.Text, exception.LineNumber, exception.Position, _qualifiedName);
9696
Logger.Error(exception, "Exception thrown in thread {0}, ParseTaskID {1}.", Thread.CurrentThread.ManagedThreadId, _taskId);
9797
var failedHandler = ParseFailure;
9898
if (failedHandler != null)

0 commit comments

Comments
 (0)