Skip to content

Commit 0a71c8e

Browse files
romaniromani
committed
Issue #36: upgrade to checkstyle 7.2
1 parent b3255e5 commit 0a71c8e

File tree

4 files changed

+239
-2
lines changed

4 files changed

+239
-2
lines changed

‎checkstyle-sonar-plugin/src/main/resources/org/sonar/l10n/checkstyle.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.annotation.MissingDepreca
2525
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.sizes.OuterTypeNumberCheck.name=Outer Type Number
2626
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.sizes.OuterTypeNumberCheck.param.max=maximum allowable number of outer types. Default is 1.
2727
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.design.DesignForExtensionCheck.name=Design For Extension
28+
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.design.DesignForExtensionCheck.param.ignoredAnnotations=Annotations which allow the check to skip the method from validation.
2829
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.coding.IllegalThrowsCheck.name=Illegal Throws
2930
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.coding.IllegalThrowsCheck.param.illegalClassNames=throw class names to reject
3031
rule.checkstyle.com.puppycrawl.tools.checkstyle.checks.coding.IllegalThrowsCheck.param.ignoredMethodNames=names of methods to ignore. Default is "finalize".

‎checkstyle-sonar-plugin/src/main/resources/org/sonar/l10n/checkstyle/rules/checkstyle/com.puppycrawl.tools.checkstyle.checks.design.DesignForExtensionCheck.html‎

Lines changed: 234 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,234 @@
1-
Checks that classes are designed for inheritance.
1+
2+
<h2>DesignForExtension</h2>
3+
4+
<div>
5+
<h3><a name="Description"></a>Description</h3>
6+
7+
<p>
8+
The check finds classes that are designed for extension (subclass creation).
9+
</p>
10+
11+
<p>
12+
Nothing wrong could be with founded classes.
13+
This check makes sense only for library project (not an application projects)
14+
which care of ideal OOP-design to make sure that class works in all cases even misusage.
15+
Even in library projects this check most likely will find classes that are designed
16+
for extension by somebody. User needs to use suppressions extensively to got a benefit from
17+
this check, and keep in suppressions all confirmed/known classes that are deigned for
18+
inheritance intentionally to let the check catch only new classes, and bring this to
19+
team/user attention.
20+
</p>
21+
22+
<p>
23+
ATTENTION: Only user can decide whether a class is designed for extension or not.
24+
The check just shows all classes which are possibly designed for extension.
25+
If smth inappropriate is found please use suppression.
26+
</p>
27+
28+
<p>
29+
ATTENTION: If the method which can be overridden in a subclass has a javadoc comment
30+
(a good practise is to explain its self-use of overridable methods) the check will not
31+
rise a violation. The violation can also be skipped if the method which can be overridden
32+
in a subclass has one or more annotations that are specified in ignoredAnnotations
33+
option. Note, that by default @Override annotation is not included in the
34+
ignoredAnnotations set as in a subclass the method which has the annotation can also be
35+
overridden in its subclass.
36+
</p>
37+
38+
<p>
39+
Problem is described at "Effective Java, 2nd Edition by Josh Bloch" book, chapter "Item 17: Design and document for inheritance or else prohibit it".
40+
</p>
41+
42+
<p>
43+
Some quotes from book:
44+
</p>
45+
46+
<blockquote>The class must document its self-use of overridable methods.
47+
By convention, a method that invokes overridable methods contains a description
48+
of these invocations at the end of its documentation comment. The description
49+
begins with the phrase “This implementation.”
50+
</blockquote>
51+
52+
<blockquote>The best solution to this problem is to prohibit subclassing in classes that
53+
are not designed and documented to be safely subclassed.
54+
</blockquote>
55+
56+
<blockquote>If a concrete class does not implement a standard interface, then you may
57+
inconvenience some programmers by prohibiting inheritance. If you feel that you
58+
must allow inheritance from such a class, one reasonable approach is to ensure
59+
that the class never invokes any of its overridable methods and to document this
60+
fact. In other words, eliminate the class’s self-use of overridable methods entirely.
61+
In doing so, you’ll create a class that is reasonably safe to subclass. Overriding a
62+
method will never affect the behavior of any other method.
63+
</blockquote>
64+
65+
<p>
66+
The check finds classes that have overridable methods (public or protected methods
67+
that are non-static, not-final, non-abstract) and have non-empty implementation.
68+
</p>
69+
70+
71+
<p>
72+
Rationale: This library design style protects superclasses against
73+
being broken by subclasses. The downside is that subclasses are
74+
limited in their flexibility, in particular they cannot prevent
75+
execution of code in the superclass, but that also means that
76+
subclasses cannot corrupt the state of the superclass by forgetting
77+
to call the superclass's method.
78+
</p>
79+
80+
<p>
81+
More specifically,
82+
it enforces a programming style where superclasses provide empty
83+
"hooks" that can be implemented by subclasses.
84+
</p>
85+
86+
<p>
87+
Example of code that cause violation as it is designed for extension:
88+
</p>
89+
90+
<div class="source">
91+
<pre>public abstract class Plant {
92+
private String roots;
93+
private String trunk;
94+
95+
protected void validate() {
96+
if (roots == null) throw new IllegalArgumentException("No roots!");
97+
if (trunk == null) throw new IllegalArgumentException("No trunk!");
98+
}
99+
100+
public abstract void grow();
101+
}
102+
103+
public class Tree extends Plant {
104+
private List leaves;
105+
106+
@Overrides
107+
protected void validate() {
108+
super.validate();
109+
if (leaves == null) throw new IllegalArgumentException("No leaves!");
110+
}
111+
112+
public void grow() {
113+
validate();
114+
}
115+
}
116+
</pre></div>
117+
118+
<p>
119+
Example of code without violation:
120+
</p>
121+
122+
<div class="source">
123+
<pre>public abstract class Plant {
124+
private String roots;
125+
private String trunk;
126+
127+
private void validate() {
128+
if (roots == null) throw new IllegalArgumentException("No roots!");
129+
if (trunk == null) throw new IllegalArgumentException("No trunk!");
130+
validateEx();
131+
}
132+
133+
protected void validateEx() { }
134+
135+
public abstract void grow();
136+
}
137+
</pre></div>
138+
</div>
139+
140+
141+
<div>
142+
<h3><a name="Properties"></a>Properties</h3>
143+
144+
<table class="bodyTable" border="0">
145+
146+
<tbody><tr class="a">
147+
148+
<th>name</th>
149+
150+
<th>description</th>
151+
152+
<th>type</th>
153+
154+
<th>default value</th>
155+
</tr>
156+
157+
<tr class="b">
158+
159+
<td>ignoredAnnotations</td>
160+
161+
<td>
162+
Annotations which allow the check to skip the method from validation.
163+
</td>
164+
165+
<td>String Set</td>
166+
167+
<td><tt>Test, Before, After, BeforeClass, AfterClass</tt></td>
168+
</tr>
169+
</tbody></table>
170+
</div>
171+
172+
173+
<div class="section">
174+
<h3><a name="Examples"></a>Examples</h3>
175+
176+
<p>
177+
To configure the check:
178+
</p>
179+
180+
181+
<div class="source">
182+
<pre>&lt;module name="DesignForExtension"/&gt;
183+
</pre></div>
184+
185+
186+
<p>
187+
To configure the check to allow methods which have @Override and @Test annotations to be
188+
designed for extension.
189+
</p>
190+
191+
192+
<div class="source">
193+
<pre>&lt;module name="DesignForExtension"&gt;
194+
&lt;property name="ignoredAnnotations" value="Override, Test"/&gt;
195+
&lt;/module&gt;
196+
</pre></div>
197+
198+
199+
<div class="source">
200+
<pre>public class A extends B {
201+
@Override
202+
public int foo() {
203+
return 2;
204+
}
205+
206+
public int foo2() {return 8;} // violation
207+
}
208+
209+
public class B {
210+
/**
211+
* This implementation ...
212+
@return some int value.
213+
*/
214+
public int foo() {
215+
return 1;
216+
}
217+
218+
public int foo3() {return 3;} // violation
219+
}
220+
221+
public class FooTest {
222+
@Test
223+
public void testFoo() {
224+
final B b = new A();
225+
assertEquals(2, b.foo());
226+
}
227+
228+
public int foo4() {return 4;} // violation
229+
}
230+
</pre></div>
231+
</div>
232+
233+
234+

‎checkstyle-sonar-plugin/src/main/resources/org/sonar/plugins/checkstyle/rules.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,9 @@
447447
<priority>MINOR</priority>
448448
<name><![CDATA[Design For Extension]]></name>
449449
<configKey><![CDATA[Checker/TreeWalker/DesignForExtension]]></configKey>
450+
<param key="ignoredAnnotations" type="s{}">
451+
<defaultValue>Test, Before, After, BeforeClass, AfterClass</defaultValue>
452+
</param>
450453
</rule>
451454

452455
<rule key="com.puppycrawl.tools.checkstyle.checks.blocks.EmptyBlockCheck">

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@
8585
</ciManagement>
8686

8787
<properties>
88-
<checkstyle.version>7.1.2</checkstyle.version>
88+
<checkstyle.version>7.2</checkstyle.version>
8989
<sonar.version>4.5.2</sonar.version>
9090
<sonar-java.version>3.7</sonar-java.version>
9191
<java.version>1.8</java.version>

0 commit comments

Comments
 (0)