Skip to content

Commit a319741

Browse files
testingoutgith1testingoutgith1
committed
added proper version of new excessive member inspection and updated implemented members inspection
1 parent e56bb88 commit a319741

File tree

8 files changed

+176
-33
lines changed

8 files changed

+176
-33
lines changed

‎Rubberduck.CodeAnalysis/Inspections/Concrete/ExcessiveInterfaceMembersInspection.cs‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
using System.Linq;
2+
using System.Collections.Generic;
3+
using Rubberduck.CodeAnalysis.Inspections.Abstract;
4+
using Rubberduck.Parsing.VBA.DeclarationCaching;
5+
using Rubberduck.Parsing.VBA;
6+
using Rubberduck.Parsing.Symbols;
7+
using Rubberduck.Resources.Inspections;
8+
9+
namespace Rubberduck.CodeAnalysis.Inspections.Concrete
10+
{
11+
/// <summary>
12+
/// Identifies class modules that define an interface with an excessive number of public members and reminds users about Interface Segregation Principle.
13+
/// </summary>
14+
/// <why>
15+
/// Interfaces should not be designed to continually grow new members; we should be keeping them small, specific, and specialized.
16+
/// </why>
17+
/// <example hasResult="false">
18+
/// <module name="MyModule" type="Class Module">
19+
/// <![CDATA[
20+
/// Option Explicit
21+
/// '@Interface
22+
///
23+
/// Public Sub DoSomething()
24+
///
25+
/// End Sub
26+
/// ]]>
27+
/// </module>
28+
/// </example>
29+
/// <example hasResult="true">
30+
/// <module name="MyModule" type="Class Module">
31+
/// <![CDATA[
32+
/// Option Explicit
33+
/// '@Interface
34+
///
35+
/// Public Sub DoSomething1()
36+
///
37+
/// End Sub
38+
///
39+
/// Public Sub DoSomething2()
40+
///
41+
/// End Sub
42+
///
43+
/// '...
44+
///
45+
/// Public Sub DoSomethingNGreaterThanMaxPublicMemberCount()
46+
///
47+
/// End Sub
48+
/// ]]>
49+
/// </module>
50+
/// </example>
51+
///
52+
internal sealed class ExcessiveInterfaceMembersInspection : DeclarationInspectionBase<int>
53+
{
54+
private const int PublicMemberLimit = 10; //todo: make setting rather than constant
55+
56+
public ExcessiveInterfaceMembersInspection(IDeclarationFinderProvider declarationFinderProvider)
57+
: base(declarationFinderProvider, DeclarationType.ClassModule)
58+
{}
59+
60+
protected override (bool isResult, int properties) IsResultDeclarationWithAdditionalProperties(Declaration declaration, DeclarationFinder finder)
61+
{
62+
if (!(declaration is ClassModuleDeclaration classModule && classModule.IsInterface))
63+
{
64+
return (false, 0);
65+
}
66+
67+
return HasExcessiveMembers(classModule);
68+
}
69+
70+
private static (bool, int) HasExcessiveMembers(ClassModuleDeclaration declaration)
71+
{
72+
var _publicmembers = declaration.Members.Where(member =>
73+
{
74+
int acc = (int)member.Accessibility;
75+
return acc >= (int)Accessibility.Implicit && acc <= (int)Accessibility.Global;
76+
});
77+
78+
var count = _publicmembers.Where(member => member.DeclarationType != DeclarationType.Event)
79+
.Where(member => member.DeclarationType != DeclarationType.PropertyGet || NoMatchingSetter(member, _publicmembers))
80+
.Count();
81+
82+
return (count > PublicMemberLimit, count);
83+
}
84+
85+
private static bool NoMatchingSetter(Declaration property, IEnumerable<Declaration> members) =>
86+
!members.Any(member => (member.IdentifierName == property.IdentifierName) && (member != property));
87+
88+
protected override string ResultDescription(Declaration declaration, int memberCount)
89+
{
90+
var identifierName = declaration.IdentifierName;
91+
92+
return string.Format(
93+
InspectionResults.ExcessiveInterfaceMembersInspection,
94+
identifierName,
95+
memberCount);
96+
}
97+
}
98+
}

‎Rubberduck.CodeAnalysis/Inspections/Concrete/ImplementedInterfaceMemberInspection.cs‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
using System.Linq;
2+
using System.Collections.Generic;
23
using Rubberduck.CodeAnalysis.Inspections.Abstract;
34
using Rubberduck.CodeAnalysis.Inspections.Extensions;
45
using Rubberduck.Common;
5-
using Rubberduck.Parsing.Annotations;
66
using Rubberduck.Parsing.Symbols;
77
using Rubberduck.Parsing.VBA;
88
using Rubberduck.Parsing.VBA.DeclarationCaching;
99
using Rubberduck.Resources.Inspections;
1010

11-
namespace Rubberduck.CodeAnalysis.Inspections.Concrete
11+
namespace Rubberduck.CodeAnalysis.Inspections.Concrete
1212
{
1313
/// <summary>
1414
/// Identifies class modules that define an interface with one or more members containing a concrete implementation.
@@ -40,49 +40,57 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
4040
/// ]]>
4141
/// </module>
4242
/// </example>
43-
internal sealed class ImplementedInterfaceMemberInspection : DeclarationInspectionBase
43+
internal sealed class ImplementedInterfaceMemberInspection : DeclarationInspectionBase<IEnumerable<ModuleBodyElementDeclaration>>
4444
{
4545
public ImplementedInterfaceMemberInspection(IDeclarationFinderProvider declarationFinderProvider)
46-
: base(declarationFinderProvider, DeclarationType.ClassModule)
46+
: base(declarationFinderProvider, DeclarationType.ClassModule)
4747
{}
4848

49-
protected override bool IsResultDeclaration(Declaration declaration, DeclarationFinder finder)
49+
protected override (bool, IEnumerable<ModuleBodyElementDeclaration>) IsResultDeclarationWithAdditionalProperties(Declaration declaration, DeclarationFinder finder)
5050
{
51-
if (!IsInterfaceDeclaration(declaration))
51+
if (!(declaration is ClassModuleDeclaration classModule && classModule.IsInterface))
5252
{
53-
return false;
53+
return (false, null);
5454
}
5555

56+
var implementedMembers = HasImplementedMembers(declaration, finder);
57+
return (implementedMembers.Count() > 0, implementedMembers);
58+
}
59+
60+
private IEnumerable<ModuleBodyElementDeclaration> HasImplementedMembers(Declaration declaration, DeclarationFinder finder)
61+
{
5662
var moduleBodyElements = finder.Members(declaration, DeclarationType.Member)
5763
.OfType<ModuleBodyElementDeclaration>();
5864

5965
return moduleBodyElements
60-
.Any(member => member.Block.ContainsExecutableStatements(true));
61-
}
62-
63-
private static bool IsInterfaceDeclaration(Declaration declaration)
64-
{
65-
if (!(declaration is ClassModuleDeclaration classModule))
66-
{
67-
return false;
68-
}
69-
return classModule.IsInterface
70-
|| declaration.Annotations.Any(an => an.Annotation is InterfaceAnnotation);
66+
.Where(member => member.Block.ContainsExecutableStatements(true));
7167
}
7268

73-
protected override string ResultDescription(Declaration declaration)
69+
protected override string ResultDescription(Declaration declaration, IEnumerable<ModuleBodyElementDeclaration> results)
7470
{
75-
var qualifiedName = declaration.QualifiedModuleName.ToString();
76-
var declarationType = Resources.RubberduckUI.ResourceManager
77-
.GetString("DeclarationType_" + declaration.DeclarationType)
78-
.Capitalize();
7971
var identifierName = declaration.IdentifierName;
72+
var memberResultsString = FormatResults(results);
8073

8174
return string.Format(
8275
InspectionResults.ImplementedInterfaceMemberInspection,
83-
qualifiedName,
84-
declarationType,
85-
identifierName);
76+
identifierName,
77+
memberResultsString);
78+
}
79+
80+
private static string FormatResults(IEnumerable<ModuleBodyElementDeclaration> results)
81+
{
82+
List<string> items = new List<string>();
83+
84+
foreach (ModuleBodyElementDeclaration elem in results)
85+
{
86+
var memberType = Resources.RubberduckUI.ResourceManager
87+
.GetString("DeclarationType_" + elem.DeclarationType)
88+
.Capitalize();
89+
90+
items.Add($"{elem.IdentifierName} ({memberType})");
91+
}
92+
93+
return string.Join(", ", items);
8694
}
8795
}
88-
}
96+
}

‎Rubberduck.Resources/Inspections/InspectionInfo.Designer.cs‎

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rubberduck.Resources/Inspections/InspectionInfo.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,4 +442,7 @@ If the parameter can be null, ignore this inspection result; passing a null valu
442442
<data name="SuperfluousAnnotationArgumentInspection" xml:space="preserve">
443443
<value>An annotation has more arguments than allowed; superfluous arguments are ignored.</value>
444444
</data>
445+
<data name="ExcessiveInterfaceMembersInspection" xml:space="preserve">
446+
<value>Interfaces should be designed to be small, specific, and specialized. Reducing the number of public members makes a more straightforward API that is easier to work with, and adheres to the Interface Segregation Principle.</value>
447+
</data>
445448
</root>

‎Rubberduck.Resources/Inspections/InspectionNames.Designer.cs‎

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rubberduck.Resources/Inspections/InspectionNames.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,4 +446,7 @@
446446
<data name="SuperfluousAnnotationArgumentInspection" xml:space="preserve">
447447
<value>Superfluous annotation arguments</value>
448448
</data>
449+
<data name="ExcessiveInterfaceMembersInspection" xml:space="preserve">
450+
<value>Interface has too many public members.</value>
451+
</data>
449452
</root>

‎Rubberduck.Resources/Inspections/InspectionResults.Designer.cs‎

Lines changed: 11 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rubberduck.Resources/Inspections/InspectionResults.resx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,8 @@ In memoriam, 1972-2018</value>
435435
<comment>{0} Method kind, {1} Method name</comment>
436436
</data>
437437
<data name="ImplementedInterfaceMemberInspection" xml:space="preserve">
438-
<value>Interface class module '{2}' contains a concrete implementation for {0} '{1}'.</value>
439-
<comment>{0} Method kind, {1} Method name, {2} interface class module name</comment>
438+
<value>Interface '{0}' contains a concrete implementation for the following member(s): {1}</value>
439+
<comment>{0} interface class name; {1} formatted string of results</comment>
440440
</data>
441441
<data name="ArgumentWithIncompatibleObjectTypeInspection" xml:space="preserve">
442442
<value>The argument '{2}' of type '{3}' is passed to the parameter '{0}' of the incompatible type '{1}'.</value>
@@ -513,4 +513,8 @@ In memoriam, 1972-2018</value>
513513
<value>The annotation '{0}' was expected to have less arguments.</value>
514514
<comment>{0} annotation name</comment>
515515
</data>
516+
<data name="ExcessiveInterfaceMembersInspection" xml:space="preserve">
517+
<value>Class/interface '{0}' has too many public members ({1}).</value>
518+
<comment>{0} class name; {1} public member count</comment>
519+
</data>
516520
</root>

0 commit comments

Comments
 (0)