Skip to content

Commit 7167328

Browse files
retailcoderretailcoder
authored
Merge pull request #3097 from rkapka/next
Fixed implicit ByRef inspection quick fix interface impementation bug (#3096)
2 parents bccaf03 + f1df112 commit 7167328

File tree

10 files changed

+751
-467
lines changed

10 files changed

+751
-467
lines changed

‎Rubberduck.Inspections/Concrete/ImplicitByRefModifierInspection.cs‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using Antlr4.Runtime;
4+
using Rubberduck.Common;
5+
using Rubberduck.Inspections.Abstract;
6+
using Rubberduck.Inspections.Results;
7+
using Rubberduck.Parsing;
8+
using Rubberduck.Parsing.Grammar;
9+
using Rubberduck.Parsing.Inspections.Abstract;
10+
using Rubberduck.Parsing.Inspections.Resources;
11+
using Rubberduck.Parsing.VBA;
12+
using Rubberduck.VBEditor;
13+
14+
namespace Rubberduck.Inspections.Concrete
15+
{
16+
public sealed class ImplicitByRefModifierInspection : ParseTreeInspectionBase
17+
{
18+
public ImplicitByRefModifierInspection(RubberduckParserState state)
19+
: base(state, CodeInspectionSeverity.Hint)
20+
{
21+
}
22+
23+
public override CodeInspectionType InspectionType => CodeInspectionType.CodeQualityIssues;
24+
25+
public override IInspectionListener Listener { get; } = new ImplicitByRefModifierListener();
26+
27+
public override IEnumerable<IInspectionResult> GetInspectionResults()
28+
{
29+
var builtInEventHandlerContexts = State.DeclarationFinder.FindEventHandlers().Select(handler => handler.Context).ToHashSet();
30+
var interfaceImplementationMemberContexts = UserDeclarations.FindInterfaceImplementationMembers().Select(member => member.Context).ToHashSet();
31+
32+
var issues = Listener.Contexts.Where(context =>
33+
!IsIgnoringInspectionResultFor(context.ModuleName, context.Context.Start.Line) &&
34+
!builtInEventHandlerContexts.Contains(context.Context.Parent.Parent) &&
35+
!interfaceImplementationMemberContexts.Contains(context.Context.Parent.Parent));
36+
37+
return issues.Select(issue =>
38+
{
39+
var identifier = ((VBAParser.ArgContext)issue.Context)
40+
.unrestrictedIdentifier()
41+
.identifier();
42+
43+
return new QualifiedContextInspectionResult(this,
44+
string.Format(InspectionsUI.ImplicitByRefModifierInspectionResultFormat,
45+
identifier.untypedIdentifier() != null
46+
? identifier.untypedIdentifier().identifierValue().GetText()
47+
: identifier.typedIdentifier().untypedIdentifier().identifierValue().GetText()),
48+
State, issue);
49+
});
50+
}
51+
52+
public class ImplicitByRefModifierListener : VBAParserBaseListener, IInspectionListener
53+
{
54+
private readonly List<QualifiedContext<ParserRuleContext>> _contexts = new List<QualifiedContext<ParserRuleContext>>();
55+
56+
public IReadOnlyList<QualifiedContext<ParserRuleContext>> Contexts => _contexts;
57+
58+
public QualifiedModuleName CurrentModuleName { get; set; }
59+
60+
public void ClearContexts()
61+
{
62+
_contexts.Clear();
63+
}
64+
65+
public override void ExitArg(VBAParser.ArgContext context)
66+
{
67+
if (context.PARAMARRAY() == null && context.BYVAL() == null && context.BYREF() == null)
68+
{
69+
_contexts.Add(new QualifiedContext<ParserRuleContext>(CurrentModuleName, context));
70+
}
71+
}
72+
}
73+
}
74+
}

‎Rubberduck.Inspections/Concrete/ImplicitByRefParameterInspection.cs‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎Rubberduck.Inspections/QuickFixes/ChangeParameterByRefByValQuickFix.cs‎

Lines changed: 0 additions & 41 deletions
This file was deleted.

‎Rubberduck.Inspections/QuickFixes/SpecifyExplicitByRefModifierQuickFix.cs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
using System;
2+
using Rubberduck.Parsing.Grammar;
3+
using System.Collections.Generic;
4+
using System.Linq;
5+
using Rubberduck.Common;
6+
using Rubberduck.Inspections.Concrete;
7+
using Rubberduck.Parsing.Inspections.Abstract;
8+
using Rubberduck.Parsing.Inspections.Resources;
9+
using Rubberduck.Parsing.Rewriter;
10+
using Rubberduck.Parsing.Symbols;
11+
using Rubberduck.Parsing.VBA;
12+
13+
namespace Rubberduck.Inspections.QuickFixes
14+
{
15+
public class SpecifyExplicitByRefModifierQuickFix : IQuickFix
16+
{
17+
private readonly RubberduckParserState _state;
18+
19+
private static readonly HashSet<Type> _supportedInspections = new HashSet<Type>
20+
{
21+
typeof(ImplicitByRefModifierInspection)
22+
};
23+
24+
public SpecifyExplicitByRefModifierQuickFix(RubberduckParserState state)
25+
{
26+
_state = state;
27+
}
28+
29+
public IReadOnlyCollection<Type> SupportedInspections => _supportedInspections.ToList();
30+
31+
public void Fix(IInspectionResult result)
32+
{
33+
var context = (VBAParser.ArgContext)result.Context;
34+
35+
AddByRefIdentifier(_state.GetRewriter(result.QualifiedSelection.QualifiedName), context);
36+
37+
var interfaceMembers = _state.DeclarationFinder.FindAllInterfaceMembers().ToArray();
38+
39+
var matchingInterfaceMemberContext = interfaceMembers.Select(member => member.Context).FirstOrDefault(c => c == context.Parent.Parent);
40+
41+
if (matchingInterfaceMemberContext != null)
42+
{
43+
var interfaceParameterIndex = GetParameterIndex(context);
44+
45+
var implementationMembers =
46+
_state.AllUserDeclarations.FindInterfaceImplementationMembers(interfaceMembers.First(
47+
member => member.Context == matchingInterfaceMemberContext)).ToHashSet();
48+
49+
var parameters =
50+
_state.DeclarationFinder.UserDeclarations(DeclarationType.Parameter)
51+
.Where(p => implementationMembers.Contains(p.ParentDeclaration))
52+
.Cast<ParameterDeclaration>()
53+
.ToArray();
54+
55+
foreach (var parameter in parameters)
56+
{
57+
var parameterContext = (VBAParser.ArgContext)parameter.Context;
58+
var parameterIndex = GetParameterIndex(parameterContext);
59+
60+
if (parameterIndex == interfaceParameterIndex)
61+
{
62+
AddByRefIdentifier(_state.GetRewriter(parameter), parameterContext);
63+
}
64+
}
65+
}
66+
}
67+
68+
public string Description(IInspectionResult result)
69+
{
70+
return InspectionsUI.ImplicitByRefModifierQuickFix;
71+
}
72+
73+
public bool CanFixInProcedure => true;
74+
public bool CanFixInModule => true;
75+
public bool CanFixInProject => true;
76+
77+
private static int GetParameterIndex(VBAParser.ArgContext context)
78+
{
79+
return Array.IndexOf(((VBAParser.ArgListContext)context.Parent).arg().ToArray(), context);
80+
}
81+
82+
private static void AddByRefIdentifier(IModuleRewriter rewriter, VBAParser.ArgContext context)
83+
{
84+
if (context.BYREF() == null)
85+
{
86+
rewriter.InsertBefore(context.unrestrictedIdentifier().Start.TokenIndex, "ByRef ");
87+
}
88+
}
89+
}
90+
}

‎Rubberduck.Inspections/Rubberduck.Inspections.csproj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
<Compile Include="Concrete\IllegalAnnotationInspection.cs" />
7070
<Compile Include="Concrete\ImplicitActiveSheetReferenceInspection.cs" />
7171
<Compile Include="Concrete\ImplicitActiveWorkbookReferenceInspection.cs" />
72-
<Compile Include="Concrete\ImplicitByRefParameterInspection.cs" />
72+
<Compile Include="Concrete\ImplicitByRefModifierInspection.cs" />
7373
<Compile Include="Concrete\ImplicitDefaultMemberAssignmentInspection.cs" />
7474
<Compile Include="Concrete\ImplicitPublicMemberInspection.cs" />
7575
<Compile Include="Concrete\ImplicitVariantReturnTypeInspection.cs" />
@@ -103,7 +103,7 @@
103103
<Compile Include="QuickFixes\ApplicationWorksheetFunctionQuickFix.cs" />
104104
<Compile Include="QuickFixes\AssignedByValParameterMakeLocalCopyQuickFix.cs" />
105105
<Compile Include="QuickFixes\ChangeDimToPrivateQuickFix.cs" />
106-
<Compile Include="QuickFixes\ChangeParameterByRefByValQuickFix.cs" />
106+
<Compile Include="QuickFixes\SpecifyExplicitByRefModifierQuickFix.cs" />
107107
<Compile Include="QuickFixes\ChangeProcedureToFunctionQuickFix.cs" />
108108
<Compile Include="QuickFixes\ConvertToProcedureQuickFix.cs" />
109109
<Compile Include="QuickFixes\DeclareAsExplicitVariantQuickFix.cs" />

‎Rubberduck.Parsing/Inspections/Resources/InspectionsUI.resx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,10 @@
177177
<data name="ImplicitActiveWorkbookReferenceInspectionName" xml:space="preserve">
178178
<value>Implicit reference to ActiveWorkbook</value>
179179
</data>
180-
<data name="ImplicitByRefParameterInspectionMeta" xml:space="preserve">
180+
<data name="ImplicitByRefModifierInspectionMeta" xml:space="preserve">
181181
<value>Parameters are passed by reference unless specified otherwise, which can be confusing and bug-prone. Prefer passing parameters by value, and specify ByRef explicitly when passing parameters by reference.</value>
182182
</data>
183-
<data name="ImplicitByRefParameterInspectionName" xml:space="preserve">
183+
<data name="ImplicitByRefModifierInspectionName" xml:space="preserve">
184184
<value>Implicit ByRef parameter</value>
185185
</data>
186186
<data name="ImplicitPublicMemberInspectionMeta" xml:space="preserve">
@@ -378,7 +378,7 @@ If the parameter can be null, ignore this inspection result; passing a null valu
378378
<data name="IdentifierNotUsedInspectionResultFormat" xml:space="preserve">
379379
<value>{0} '{1}' is not used</value>
380380
</data>
381-
<data name="ImplicitByRefParameterInspectionResultFormat" xml:space="preserve">
381+
<data name="ImplicitByRefModifierInspectionResultFormat" xml:space="preserve">
382382
<value>Parameter '{0}' is implicitly passed by reference</value>
383383
</data>
384384
<data name="ImplicitPublicMemberInspectionResultFormat" xml:space="preserve">
@@ -417,7 +417,7 @@ If the parameter can be null, ignore this inspection result; passing a null valu
417417
<data name="DeclareAsExplicitVariantQuickFix" xml:space="preserve">
418418
<value>Declare as explicit Variant</value>
419419
</data>
420-
<data name="ImplicitByRefParameterQuickFix" xml:space="preserve">
420+
<data name="ImplicitByRefModifierQuickFix" xml:space="preserve">
421421
<value>Pass parameter by reference explicitly</value>
422422
</data>
423423
<data name="Inspections_Declaration" xml:space="preserve">

0 commit comments

Comments
 (0)