TouchInstinct
/
checkstyle
307
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Illegal Type Check, updated default illegal types, added memberModifiers option, issue #567

This commit is contained in:
alexkravin 2015-01-30 15:38:49 +04:00
parent ba4d62dc78
commit 677acc1e84
5 changed files with 151 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/IllegalTypeCheck.java
View File

@ -24,6 +24,9 @@ import com.puppycrawl.tools.checkstyle.api.FullIdent;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.checks.AbstractFormatCheck;
import com.puppycrawl.tools.checkstyle.checks.CheckUtils;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
/**
@ -66,7 +69,22 @@ import java.util.Set;
* will be ok.
* </p>
* <p>
* <b>ignoredMethodNames</b> - Methods that should not be checked..
* <b>ignoredMethodNames</b> - Methods that should not be checked.
* </p>
* <p>
* <b>memberModifiers</b> - To check only methods and fields with only specified modifiers.
* </p>
* <p>
* In most cases it's justified to put following classes to <b>illegalClassNames</b>:
* <ul>
* <li>GregorianCalendar</li>
* <li>Hashtable</li>
* <li>ArrayList</li>
* <li>LinkedList</li>
* <li>Vector</li>
* </ul>
* as methods that are differ from interface methods are rear used, so in most cases user will
* benefit from checking for them.
* </p>
*
* @author <a href="mailto:simon@redhillconsulting.com.au">Simon Harris</a>
@ -80,28 +98,18 @@ public final class IllegalTypeCheck extends AbstractFormatCheck
private static final String[] DEFAULT_LEGAL_ABSTRACT_NAMES = {};
/** Types illegal by default. */
private static final String[] DEFAULT_ILLEGAL_TYPES = {
"GregorianCalendar",
"Hashtable",
"HashSet",
"HashMap",
"ArrayList",
"LinkedList",
"LinkedHashMap",
"LinkedHashSet",
"TreeSet",
"TreeMap",
"Vector",
"java.util.GregorianCalendar",
"java.util.Hashtable",
"java.util.HashSet",
"java.util.HashMap",
"java.util.ArrayList",
"java.util.LinkedList",
"java.util.LinkedHashMap",
"java.util.LinkedHashSet",
"java.util.TreeSet",
"java.util.TreeMap",
"java.util.Vector",
};
/** Default ignored method names. */
@ -116,6 +124,8 @@ public final class IllegalTypeCheck extends AbstractFormatCheck
private final Set<String> legalAbstractClassNames = Sets.newHashSet();
/** methods which should be ignored. */
private final Set<String> ignoredMethodNames = Sets.newHashSet();
/** check methods and fields with only corresponding modifiers. */
private List<Integer> memberModifiers;
/** Creates new instance of the check. */
public IllegalTypeCheck()
@ -142,10 +152,14 @@ public final class IllegalTypeCheck extends AbstractFormatCheck
{
switch (ast.getType()) {
case TokenTypes.METHOD_DEF:
visitMethodDef(ast);
if (isVerifiable(ast)) {
visitMethodDef(ast);
}
break;
case TokenTypes.VARIABLE_DEF:
visitVariableDef(ast);
if (isVerifiable(ast)) {
visitVariableDef(ast);
}
break;
case TokenTypes.PARAMETER_DEF:
visitParameterDef(ast);
@ -158,6 +172,32 @@ public final class IllegalTypeCheck extends AbstractFormatCheck
}
}
/**
* Checks if current method's return type or variable's type is verifiable
* according to <b>memberModifiers</b> option.
* @param methodOrVariableDef METHOD_DEF or VARIABLE_DEF ast node.
* @return true if member is verifiable according to <b>memberModifiers</b> option.
*/
private boolean isVerifiable(DetailAST methodOrVariableDef)
{
boolean result = true;
if (memberModifiers != null) {
result = false;
final DetailAST modifiersAst = methodOrVariableDef.
findFirstToken(TokenTypes.MODIFIERS);
if (modifiersAst.getFirstChild() != null) {
for (DetailAST modifier = modifiersAst.getFirstChild(); modifier != null;
modifier = modifier.getNextSibling())
{
if (memberModifiers.contains(modifier.getType())) {
result = true;
}
}
}
}
return result;
}
/**
* Checks return type of a given method.
* @param methodDef method for check.
@ -404,4 +444,17 @@ public final class IllegalTypeCheck extends AbstractFormatCheck
return legalAbstractClassNames.toArray(
new String[legalAbstractClassNames.size()]);
}
/**
* Set the list of member modifiers (of methods and fields) which should be checked.
* @param modifiers String contains modifiers.
*/
public void setMemberModifiers(String modifiers)
{
final List<Integer> modifiersList = new ArrayList<Integer>(modifiers.length());
for (String modifier : modifiers.split(", ")) {
modifiersList.add(TokenTypes.getTokenId(modifier));
}
this.memberModifiers = modifiersList;
}
}

37
src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/IllegalTypeCheckTest.java
View File

@ -42,8 +42,8 @@ public class IllegalTypeCheckTest extends BaseCheckTestSupport
"9:13: Declaring variables, return values or parameters of type "
+ "'com.puppycrawl.tools.checkstyle.coding.InputIllegalType.AbstractClass'"
+ " is not allowed.",
"16:13: Declaring variables, return values or parameters of type 'java.util.Hashtable' is not allowed.",
"17:13: Declaring variables, return values or parameters of type 'Hashtable' is not allowed.",
"16:13: Declaring variables, return values or parameters of type 'java.util.TreeSet' is not allowed.",
"17:13: Declaring variables, return values or parameters of type 'TreeSet' is not allowed.",
};
verify(checkConfig, getPath("coding" + File.separator + "InputIllegalType.java"), expected);
@ -59,7 +59,7 @@ public class IllegalTypeCheckTest extends BaseCheckTestSupport
"9:13: Declaring variables, return values or parameters of type "
+ "'com.puppycrawl.tools.checkstyle.coding.InputIllegalType.AbstractClass'"
+ " is not allowed.",
"16:13: Declaring variables, return values or parameters of type 'java.util.Hashtable' is not allowed.",
"16:13: Declaring variables, return values or parameters of type 'java.util.TreeSet' is not allowed.",
};
verify(checkConfig, getPath("coding" + File.separator + "InputIllegalType.java"), expected);
@ -71,8 +71,8 @@ public class IllegalTypeCheckTest extends BaseCheckTestSupport
checkConfig.addAttribute("format", "^$");
String[] expected = {
"16:13: Declaring variables, return values or parameters of type 'java.util.Hashtable' is not allowed.",
"17:13: Declaring variables, return values or parameters of type 'Hashtable' is not allowed.",
"16:13: Declaring variables, return values or parameters of type 'java.util.TreeSet' is not allowed.",
"17:13: Declaring variables, return values or parameters of type 'TreeSet' is not allowed.",
};
verify(checkConfig, getPath("coding" + File.separator + "InputIllegalType.java"), expected);
@ -87,8 +87,8 @@ public class IllegalTypeCheckTest extends BaseCheckTestSupport
"9:13: Declaring variables, return values or parameters of type "
+ "'com.puppycrawl.tools.checkstyle.coding.InputIllegalType.AbstractClass'"
+ " is not allowed.",
"16:13: Declaring variables, return values or parameters of type 'java.util.Hashtable' is not allowed.",
"17:13: Declaring variables, return values or parameters of type 'Hashtable' is not allowed.",
"16:13: Declaring variables, return values or parameters of type 'java.util.TreeSet' is not allowed.",
"17:13: Declaring variables, return values or parameters of type 'TreeSet' is not allowed.",
};
verify(checkConfig, getPath("coding" + File.separator + "InputIllegalType.java"), expected);
@ -152,4 +152,27 @@ public class IllegalTypeCheckTest extends BaseCheckTestSupport
verify(checkConfig, getPath("coding" + File.separator
+ "InputIllegalTypeStaticImports.java"), expected);
}
@Test
public void testMemberModifiers() throws Exception
{
checkConfig.addAttribute("memberModifiers", "LITERAL_PRIVATE, LITERAL_PROTECTED,"
+ " LITERAL_STATIC");
String[] expected = {
"6:13: Declaring variables, return values or parameters of type 'AbstractClass' is not allowed.",
"9:13: Declaring variables, return values or parameters of type "
+ "'com.puppycrawl.tools.checkstyle.coding.InputIllegalTypeMemberModifiers.AbstractClass'"
+ " is not allowed.",
"16:13: Declaring variables, return values or parameters of type 'java.util.TreeSet' is not allowed.",
"17:13: Declaring variables, return values or parameters of type 'TreeSet' is not allowed.",
"23:15: Declaring variables, return values or parameters of type "
+ "'com.puppycrawl.tools.checkstyle.coding.InputIllegalTypeMemberModifiers.AbstractClass'"
+ " is not allowed.",
"25:25: Declaring variables, return values or parameters of type 'java.util.TreeSet' is not allowed.",
"33:15: Declaring variables, return values or parameters of type 'AbstractClass' is not allowed.",
};
verify(checkConfig, getPath("coding" + File.separator
+ "InputIllegalTypeMemberModifiers.java"), expected);
}
}

6
src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputIllegalType.java
View File

@ -1,5 +1,5 @@
package com.puppycrawl.tools.checkstyle.coding;
import java.util.TreeSet;
import java.util.Hashtable;
//configuration: default
public class InputIllegalType {
@ -13,8 +13,8 @@ public class InputIllegalType {
private class NotAnAbstractClass {}
private java.util.Hashtable table1() { return null; } //WARNING
private Hashtable table2() { return null; } //WARNING
private java.util.TreeSet table1() { return null; } //WARNING
private TreeSet table2() { return null; } //WARNING
static class SomeStaticClass {
}

34
src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputIllegalTypeMemberModifiers.java Normal file
View File

@ -0,0 +1,34 @@
package com.puppycrawl.tools.checkstyle.coding;
import java.util.TreeSet;
import java.util.Hashtable;
//configuration: default
public class InputIllegalTypeMemberModifiers {
private AbstractClass a = null; //WARNING
private NotAnAbstractClass b = null; /*another comment*/
private com.puppycrawl.tools.checkstyle.coding.InputIllegalTypeMemberModifiers.AbstractClass c = null; //WARNING
private com.puppycrawl.tools.checkstyle.coding.InputIllegalTypeMemberModifiers.NotAnAbstractClass d = null;
private abstract class AbstractClass {/*one more comment*/}
private class NotAnAbstractClass {}
private java.util.TreeSet table1() { return null; } //WARNING
private TreeSet table2() { return null; } //WARNING
static class SomeStaticClass {
}
//WARNING if memberModifiers is set and contains TokenTypes.LITERAL_PROTECTED
protected com.puppycrawl.tools.checkstyle.coding.InputIllegalTypeMemberModifiers.AbstractClass c1 = null;
//NO WARNING if memberModifiers is set and does not contain TokenTypes.LITERAL_PUBLIC
public final static java.util.TreeSet table3() { return null; }
java.util.TreeSet table4() { java.util.TreeSet treeSet = null; return null; }
private class Some {
java.util.TreeSet treeSet = null;
}
//WARNING if memberModifiers is set and contains TokenTypes.LITERAL_PROTECTED
protected AbstractClass a1 = null;
}

22
src/xdocs/config_coding.xml
View File

@ -1750,11 +1750,9 @@ if (&quot;something&quot;.equals(x))
<td>Classes that should not be used as types in variable
declarations, return values or parameters</td>
<td><a href="property_types.html#stringSet">String Set</a></td>
<td>&quot;java.util.GregorianCalendar, java.util.Hashtable,
java.util.HashSet, java.util.HashMap, java.util.ArrayList,
java.util.LinkedList, java.util.LinkedHashMap,
<td>&quot;java.util.HashSet, java.util.HashMap, java.util.LinkedHashMap,
java.util.LinkedHashSet, java.util.TreeSet,
java.util.TreeMap, java.util.Vector&quot;</td>
java.util.TreeMap&quot;</td>
</tr>
<tr>
<td>legalAbstractClassNames</td>
@ -1774,6 +1772,12 @@ if (&quot;something&quot;.equals(x))
<td><a href="property_types.html#regexp">regular expression</a></td>
<td><code>^(.*[\\.])?Abstract.*$</code></td>
</tr>
<tr>
<td>memberModifiers</td>
<td>Check methods and fields with only corresponding modifiers.</td>
<td><a href="property_types.html#intSet">List of integers</a></td>
<td><code>null</code></td>
</tr>
</table>
</subsection>
@ -1784,6 +1788,16 @@ if (&quot;something&quot;.equals(x))
<source>
&lt;module name=&quot;IllegalType&quot;&gt;
&lt;property name=&quot;ignoredMethodNames&quot; value=&quot;getInstance&quot;/&gt;
&lt;/module&gt;
</source>
<p>
To configure the Check so that it verifies only public, protected and static
methods and fields:
</p>
<source>
&lt;module name=&quot;IllegalType&quot;&gt;
&lt;property name=&quot;memberModifiers&quot; value=&quot;LITERAL_PUBLIC,
LITERAL_PROTECTED, LITERAL_STATIC&quot;/&gt;
&lt;/module&gt;
</source>
</subsection>