Skip to content

Commit f60b742

Browse files
fourlsfourls
committed
Implement TooManyDefaultParameters check
1 parent a3938fc commit f60b742

File tree

6 files changed

+324
-0
lines changed

6 files changed

+324
-0
lines changed

‎delphi-checks/src/main/java/au/com/integradev/delphi/checks/CheckList.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ public final class CheckList {
146146
TabulationCharacterCheck.class,
147147
TooLargeRoutineCheck.class,
148148
TooLongLineCheck.class,
149+
TooManyDefaultParametersCheck.class,
149150
TooManyNestedRoutinesCheck.class,
150151
TooManyParametersCheck.class,
151152
TooManyVariablesCheck.class,

‎delphi-checks/src/main/java/au/com/integradev/delphi/checks/TooManyDefaultParametersCheck.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Sonar Delphi Plugin
3+
* Copyright (C) 2019 Integrated Application Development
4+
*
5+
* This program is free software; you can redistribute it and/or
6+
* modify it under the terms of the GNU Lesser General Public
7+
* License as published by the Free Software Foundation; either
8+
* version 3 of the License, or (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
* Lesser General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Lesser General Public
16+
* License along with this program; if not, write to the Free Software
17+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
18+
*/
19+
package au.com.integradev.delphi.checks;
20+
21+
import org.sonar.check.Rule;
22+
import org.sonar.check.RuleProperty;
23+
import org.sonar.plugins.communitydelphi.api.ast.FormalParameterNode.FormalParameterData;
24+
import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode;
25+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheck;
26+
import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext;
27+
import org.sonar.plugins.communitydelphi.api.type.Parameter;
28+
29+
@Rule(key = "TooManyDefaultParameters")
30+
public class TooManyDefaultParametersCheck extends DelphiCheck {
31+
private static final int DEFAULT_MAXIMUM = 2;
32+
33+
@RuleProperty(
34+
key = "max",
35+
description = "Maximum authorized number of default parameters",
36+
defaultValue = "" + DEFAULT_MAXIMUM)
37+
public int max = DEFAULT_MAXIMUM;
38+
39+
@Override
40+
public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckContext context) {
41+
var count =
42+
routine.getParameters().stream().filter(FormalParameterData::hasDefaultValue).count();
43+
44+
// A routine implementation's default parameters MUST match its declaration's, or not have any
45+
// default parameters at all. This means that we need to check the declaration to be sure that
46+
// a routine with 0 default parameters in its implementation actually has 0 default parameters.
47+
if (count == 0) {
48+
var declaration = routine.getRoutineNameDeclaration();
49+
if (declaration != null) {
50+
count = declaration.getParameters().stream().filter(Parameter::hasDefaultValue).count();
51+
}
52+
}
53+
54+
if (count > max) {
55+
reportIssue(
56+
context,
57+
routine.getRoutineNameNode(),
58+
String.format(
59+
"Routine has %d default parameters, which is greater than %d authorized.",
60+
count, max));
61+
}
62+
return super.visit(routine, context);
63+
}
64+
}

‎delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
"TabulationCharacter",
8686
"TooLargeRoutine",
8787
"TooLongLine",
88+
"TooManyDefaultParameters",
8889
"TooManyNestedRoutines",
8990
"TooManyParameters",
9091
"TrailingCommaArgumentList",

‎delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/TooManyDefaultParameters.html‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>
3+
Default parameters can be useful in cases when there is a sensible default value. They can,
4+
however, damage API interfaces when used to make a method's behaviour overly configurable
5+
a method with too many default parameters is confusing, unpredictable at a glance, unintuitive
6+
to use, and likely overloaded in functionality.
7+
</p>
8+
<p>
9+
In most cases, using method overloads, multiple methods, or configuration objects is a more
10+
appropriate solution.
11+
</p>
12+
<h2>How to fix it</h2>
13+
<p>
14+
Refactor this routine by re-evaluating the need for the default values, and either:
15+
</p>
16+
<ul>
17+
<li>Removing the unnecessary default values,</li>
18+
<li>Adding multiple overloads with different groups of required parameters, or</li>
19+
<li>Using a configuration object approach instead</li>
20+
</ul>

‎delphi-checks/src/main/resources/org/sonar/l10n/delphi/rules/community-delphi/TooManyDefaultParameters.json‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"title": "Routines should not have too many default parameters",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant/Issue",
7+
"constantCost": "5min"
8+
},
9+
"code": {
10+
"attribute": "FOCUSED",
11+
"impacts": {
12+
"MAINTAINABILITY": "MEDIUM"
13+
}
14+
},
15+
"tags": ["brain-overload"],
16+
"defaultSeverity": "Major",
17+
"scope": "MAIN",
18+
"quickfix": "unknown"
19+
}

‎delphi-checks/src/test/java/au/com/integradev/delphi/checks/TooManyDefaultParametersCheckTest.java‎

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
/*
2+
* Sonar Delphi Plugin
3+
* Copyright (C) 2019 Integrated Application Development
4+
*
5+
* This program is free software; you can redistribute it and/or
6+
* modify it under the terms of the GNU Lesser General Public
7+
* License as published by the Free Software Foundation; either
8+
* version 3 of the License, or (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
* Lesser General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Lesser General Public
16+
* License along with this program; if not, write to the Free Software
17+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
18+
*/
19+
package au.com.integradev.delphi.checks;
20+
21+
import au.com.integradev.delphi.builders.DelphiTestUnitBuilder;
22+
import au.com.integradev.delphi.checks.verifier.CheckVerifier;
23+
import org.junit.jupiter.api.Test;
24+
25+
class TooManyDefaultParametersCheckTest {
26+
@Test
27+
void testImplOnlyNoDefaultParamsShouldNotAddIssue() {
28+
CheckVerifier.newVerifier()
29+
.withCheck(new TooManyDefaultParametersCheck())
30+
.onFile(
31+
new DelphiTestUnitBuilder()
32+
.appendImpl("procedure Foo(")
33+
.appendImpl(" Param1: Boolean;")
34+
.appendImpl(" Param2: Boolean;")
35+
.appendImpl(" Param3: Boolean;")
36+
.appendImpl(" Param4: Boolean;")
37+
.appendImpl(" Param5: Boolean;")
38+
.appendImpl(" Param6: Boolean;")
39+
.appendImpl(" Param7: Boolean;")
40+
.appendImpl(" Param8: Boolean")
41+
.appendImpl(");")
42+
.appendImpl("begin")
43+
.appendImpl(" // do nothing")
44+
.appendImpl("end;"))
45+
.verifyNoIssues();
46+
}
47+
48+
@Test
49+
void testImplOnlySomeDefaultParamsShouldNotAddIssue() {
50+
CheckVerifier.newVerifier()
51+
.withCheck(new TooManyDefaultParametersCheck())
52+
.onFile(
53+
new DelphiTestUnitBuilder()
54+
.appendImpl("procedure Foo(")
55+
.appendImpl(" Param1: Boolean;")
56+
.appendImpl(" Param2: Boolean;")
57+
.appendImpl(" Param3: Boolean;")
58+
.appendImpl(" Param4: Boolean;")
59+
.appendImpl(" Param5: Boolean;")
60+
.appendImpl(" Param6: Boolean;")
61+
.appendImpl(" Param7: Boolean = True;")
62+
.appendImpl(" Param8: Boolean = False")
63+
.appendImpl(");")
64+
.appendImpl("begin")
65+
.appendImpl(" // do nothing")
66+
.appendImpl("end;"))
67+
.verifyNoIssues();
68+
}
69+
70+
@Test
71+
void testImplMatchingDeclSomeDefaultParamsShouldNotAddIssue() {
72+
CheckVerifier.newVerifier()
73+
.withCheck(new TooManyDefaultParametersCheck())
74+
.onFile(
75+
new DelphiTestUnitBuilder()
76+
.appendDecl("procedure Foo(")
77+
.appendDecl(" Param1: Boolean;")
78+
.appendDecl(" Param2: Boolean;")
79+
.appendDecl(" Param3: Boolean;")
80+
.appendDecl(" Param4: Boolean;")
81+
.appendDecl(" Param5: Boolean;")
82+
.appendDecl(" Param6: Boolean;")
83+
.appendDecl(" Param7: Boolean = True;")
84+
.appendDecl(" Param8: Boolean = False")
85+
.appendDecl(");")
86+
.appendImpl("procedure Foo(")
87+
.appendImpl(" Param1: Boolean;")
88+
.appendImpl(" Param2: Boolean;")
89+
.appendImpl(" Param3: Boolean;")
90+
.appendImpl(" Param4: Boolean;")
91+
.appendImpl(" Param5: Boolean;")
92+
.appendImpl(" Param6: Boolean;")
93+
.appendImpl(" Param7: Boolean = True;")
94+
.appendImpl(" Param8: Boolean = False")
95+
.appendImpl(");")
96+
.appendImpl("begin")
97+
.appendImpl(" // do nothing")
98+
.appendImpl("end;"))
99+
.verifyNoIssues();
100+
}
101+
102+
@Test
103+
void testImplNotMatchingDeclSomeDefaultParamsShouldNotAddIssue() {
104+
CheckVerifier.newVerifier()
105+
.withCheck(new TooManyDefaultParametersCheck())
106+
.onFile(
107+
new DelphiTestUnitBuilder()
108+
.appendDecl("procedure Foo(")
109+
.appendDecl(" Param1: Boolean;")
110+
.appendDecl(" Param2: Boolean;")
111+
.appendDecl(" Param3: Boolean;")
112+
.appendDecl(" Param4: Boolean;")
113+
.appendDecl(" Param5: Boolean;")
114+
.appendDecl(" Param6: Boolean;")
115+
.appendDecl(" Param7: Boolean = True;")
116+
.appendDecl(" Param8: Boolean = False")
117+
.appendDecl(");")
118+
.appendImpl("procedure Foo(")
119+
.appendImpl(" Param1: Boolean;")
120+
.appendImpl(" Param2: Boolean;")
121+
.appendImpl(" Param3: Boolean;")
122+
.appendImpl(" Param4: Boolean;")
123+
.appendImpl(" Param5: Boolean;")
124+
.appendImpl(" Param6: Boolean;")
125+
.appendImpl(" Param7: Boolean;")
126+
.appendImpl(" Param8: Boolean")
127+
.appendImpl(");")
128+
.appendImpl("begin")
129+
.appendImpl(" // do nothing")
130+
.appendImpl("end;"))
131+
.verifyNoIssues();
132+
}
133+
134+
@Test
135+
void testImplOnlyTooManyDefaultParamsShouldAddIssue() {
136+
CheckVerifier.newVerifier()
137+
.withCheck(new TooManyDefaultParametersCheck())
138+
.onFile(
139+
new DelphiTestUnitBuilder()
140+
.appendImpl("procedure Foo( // Noncompliant")
141+
.appendImpl(" Param1: Boolean;")
142+
.appendImpl(" Param2: Boolean;")
143+
.appendImpl(" Param3: Boolean;")
144+
.appendImpl(" Param4: Boolean = False;")
145+
.appendImpl(" Param5: Boolean = True;")
146+
.appendImpl(" Param6: Boolean = False;")
147+
.appendImpl(" Param7: Boolean = True;")
148+
.appendImpl(" Param8: Boolean = False")
149+
.appendImpl(");")
150+
.appendImpl("begin")
151+
.appendImpl(" // do nothing")
152+
.appendImpl("end;"))
153+
.verifyIssues();
154+
}
155+
156+
@Test
157+
void testImplMatchingDeclTooManyDefaultParamsShouldAddIssue() {
158+
CheckVerifier.newVerifier()
159+
.withCheck(new TooManyDefaultParametersCheck())
160+
.onFile(
161+
new DelphiTestUnitBuilder()
162+
.appendDecl("procedure Foo(")
163+
.appendDecl(" Param1: Boolean;")
164+
.appendDecl(" Param2: Boolean;")
165+
.appendDecl(" Param3: Boolean;")
166+
.appendDecl(" Param4: Boolean = False;")
167+
.appendDecl(" Param5: Boolean = True;")
168+
.appendDecl(" Param6: Boolean = False;")
169+
.appendDecl(" Param7: Boolean = True;")
170+
.appendDecl(" Param8: Boolean = False")
171+
.appendDecl(");")
172+
.appendImpl("procedure Foo( // Noncompliant")
173+
.appendImpl(" Param1: Boolean;")
174+
.appendImpl(" Param2: Boolean;")
175+
.appendImpl(" Param3: Boolean;")
176+
.appendImpl(" Param4: Boolean = False;")
177+
.appendImpl(" Param5: Boolean = True;")
178+
.appendImpl(" Param6: Boolean = False;")
179+
.appendImpl(" Param7: Boolean = True;")
180+
.appendImpl(" Param8: Boolean = False")
181+
.appendImpl(");")
182+
.appendImpl("begin")
183+
.appendImpl(" // do nothing")
184+
.appendImpl("end;"))
185+
.verifyIssues();
186+
}
187+
188+
@Test
189+
void testImplNotMatchingDeclTooManyDefaultParamsShouldAddIssue() {
190+
CheckVerifier.newVerifier()
191+
.withCheck(new TooManyDefaultParametersCheck())
192+
.onFile(
193+
new DelphiTestUnitBuilder()
194+
.appendDecl("procedure Foo(")
195+
.appendDecl(" Param1: Boolean;")
196+
.appendDecl(" Param2: Boolean;")
197+
.appendDecl(" Param3: Boolean;")
198+
.appendDecl(" Param4: Boolean = False;")
199+
.appendDecl(" Param5: Boolean = True;")
200+
.appendDecl(" Param6: Boolean = False;")
201+
.appendDecl(" Param7: Boolean = True;")
202+
.appendDecl(" Param8: Boolean = False")
203+
.appendDecl(");")
204+
.appendImpl("procedure Foo( // Noncompliant")
205+
.appendImpl(" Param1: Boolean;")
206+
.appendImpl(" Param2: Boolean;")
207+
.appendImpl(" Param3: Boolean;")
208+
.appendImpl(" Param4: Boolean;")
209+
.appendImpl(" Param5: Boolean;")
210+
.appendImpl(" Param6: Boolean;")
211+
.appendImpl(" Param7: Boolean;")
212+
.appendImpl(" Param8: Boolean")
213+
.appendImpl(");")
214+
.appendImpl("begin")
215+
.appendImpl(" // do nothing")
216+
.appendImpl("end;"))
217+
.verifyIssues();
218+
}
219+
}

0 commit comments

Comments
 (0)