Skip to content

Commit

Permalink
Issue #461: Field single declaration check implemented
Browse files Browse the repository at this point in the history
  • Loading branch information
yaziza committed Apr 14, 2020
1 parent bd5ca6b commit 7e6921b
Show file tree
Hide file tree
Showing 12 changed files with 317 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ EmptyPublicCtorInClassCheck.desc = <p>This Check looks for useless empty public
EmptyPublicCtorInClassCheck.classAnnotationNames = Regex which matches names of class annotations which require class to have public no-argument ctor. Default value is "javax\\.persistence\\.Entity".
EmptyPublicCtorInClassCheck.ctorAnnotationNames = Regex which matches names of ctor annotations which make empty public ctor essential. Default value is "com\\.google\\.inject\\.Inject".
FieldSingleDeclarationCheck.name = Field Single Declaration Check
FieldSingleDeclarationCheck.desc = <p>This checks ensures that classes have at most 1 field of the given className. This can be useful for example to ensure that only one Logger is used.</p>
FieldSingleDeclarationCheck.fullyQualifiedClassName = Fully qualified name of class of which there should only be max. 1 field per class. Example: "org.slf4j.Logger".
FinalizeImplementationCheck.name = Finalize Implementation
FinalizeImplementationCheck.desc = <p>This Check detects 3 most common cases of incorrect finalize() method implementation:</p><ul><li>negates effect of superclass finalize<br/><code>protected void finalize() { } <br/> protected void finalize() { doSomething(); }</code></li><li>useless (or worse) finalize<br/><code>protected void finalize() { super.finalize(); }</code></li><li>public finalize<br/><code>public void finalize() { try { doSomething(); } finally { super.finalize() } }</code></li></ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,17 @@
<message-key key="empty.public.ctor"/>
</rule-metadata>

<rule-metadata name="%FieldSingleDeclarationCheck.name" internal-name="FieldSingleDeclarationCheck" parent="TreeWalker">
<alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.FieldSingleDeclarationCheck"/>
<description>%FieldSingleDeclarationCheck.desc</description>

<property-metadata name="fullyQualifiedClassName" datatype="String" default-value="java.util.logging.Logger">
<description>%FieldSingleDeclarationCheck.fullyQualifiedClassName</description>
</property-metadata>

<message-key key="field.count"/>
</rule-metadata>

<rule-metadata name="%ForbidCertainImportsCheck.name" internal-name="ForbidCertainImportsCheck" parent="TreeWalker">
<alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.ForbidCertainImportsCheck"/>
<description>%ForbidCertainImportsCheck.desc</description>
Expand Down
3 changes: 3 additions & 0 deletions sevntu-checks/sevntu-checks.xml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@
<property name="ignoreFieldNamePattern" value="serialVersionUID"/>
</module>
<module name="com.github.sevntu.checkstyle.checks.coding.EmptyPublicCtorInClassCheck"/>
<module name="com.github.sevntu.checkstyle.checks.coding.FieldSingleDeclarationCheck">
<property name="fullyQualifiedClassName" value="java.util.logging.Logger"/>
</module>
<module name="com.github.sevntu.checkstyle.checks.coding.DiamondOperatorForVariableDefinitionCheck"/>
<module name="com.github.sevntu.checkstyle.checks.coding.NameConventionForJunit4TestClassesCheck"/>
<module name="com.github.sevntu.checkstyle.checks.coding.UselessSuperCtorCallCheck"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@

import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.FullIdent;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.utils.CheckUtil;
import com.puppycrawl.tools.checkstyle.utils.TokenUtil;

/**
Expand Down Expand Up @@ -76,4 +79,40 @@ public static DetailAST getNextSubTreeNode(DetailAST node, DetailAST subTreeRoot
return toVisitAst;
}

/**
* Obtains full type name of first token in node.
* @param ast an AST node
* @return fully qualified name (FQN) of type, or null if none was found
*/
public static String getTypeNameOfFirstToken(DetailAST ast) {
final DetailAST findFirstToken = ast.findFirstToken(TokenTypes.TYPE);
final FullIdent ident = CheckUtil.createFullType(findFirstToken);

return ident.getText();
}

/**
* Checks node for matching class, taken both FQN and short name into account.
*
* @param ast
* an AST node
* @param fqClassName
* fully qualified class name
* @return true if type name of first token in node is fqnClassName, or its short name; false
* otherwise
*/
public static boolean matchesFullyQualifiedName(DetailAST ast, String fqClassName) {
final String typeName = getTypeNameOfFirstToken(ast);
final int lastDotPosition = fqClassName.lastIndexOf('.');
boolean isMatched = false;
if (lastDotPosition == -1) {
isMatched = typeName.equals(fqClassName);
}
else {
final String shortClassName = fqClassName.substring(lastDotPosition + 1);
isMatched = typeName.equals(fqClassName) || typeName.equals(shortClassName);
}
return isMatched;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code for adherence to a set of rules.
// Copyright (C) 2001-2020 the original author or authors.
//
// This library is free software; you can redistribute it and/or
// modify it under the terms of the GNU Lesser General Public
// License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
//
// This library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
// Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
////////////////////////////////////////////////////////////////////////////////

package com.github.sevntu.checkstyle.checks.coding;

import com.github.sevntu.checkstyle.SevntuUtil;
import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;

/**
* <p>This checks ensures that classes have at most 1 field of the given className.</p>
*
* <p>This can be useful for example to ensure that only one Logger is used:</p>
*
* <pre>
* &lt;module name="TreeWalker"&gt;
* &lt;module name="com.github.sevntu.checkstyle.checks.coding.FieldSingleDeclarationCheck"&gt;
* &lt;property name="fullyQualifiedClassName" value="org.slf4j.Logger" /&gt;
* &lt;/module&gt;
* &lt;/module&gt;
*
* class Example {
* private static final org.slf4j.Logger logger = LoggerFactory.getLogger(Example.class); // OK
* private static org.slf4j.Logger logger2; // NOK!
* }
* </pre>
*
* @author Milos Fabian, Pantheon Technologies - original author (in OpenDaylight.org)
* @author Michael Vorburger.ch - refactored and made more generalized for contribution to Sevntu
* @author <a href="mailto:[email protected]">Yasser Aziza</a> - completed sevntu integration
*/
public class FieldSingleDeclarationCheck extends AbstractCheck {

/**
* Violation message key.
*/
public static final String MSG_KEY = "field.count";

/**
* Configuration property with class name check for.
*/
private String fullyQualifiedClassName;

/**
* Field to remember if class was previously seen in current File.
*/
private boolean hasPreviouslySeenClass;

/**
* Set Class of which there should only be max. 1 field per class.
* @param className the fully qualified name (FQN) of the class
*/
public void setFullyQualifiedClassName(String className) {
this.fullyQualifiedClassName = className;
}

@Override
public int[] getDefaultTokens() {
return new int[] {TokenTypes.VARIABLE_DEF };
}

@Override
public int[] getRequiredTokens() {
return getDefaultTokens();
}

@Override
public int[] getAcceptableTokens() {
return getDefaultTokens();
}

@Override
public void beginTree(DetailAST rootAST) {
this.hasPreviouslySeenClass = false;
}

@Override
public void visitToken(DetailAST ast) {
if (fullyQualifiedClassName == null) {
throw new IllegalStateException("Must set mandatory fullyQualifiedClassName property in"
+ getClass().getSimpleName());
}
if (SevntuUtil.matchesFullyQualifiedName(ast, fullyQualifiedClassName)) {
if (hasPreviouslySeenClass) {
log(ast.getLineNo(), MSG_KEY, fullyQualifiedClassName);
}
this.hasPreviouslySeenClass = true;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ custom.declaration.order.method=Method definition in wrong order. Expected ''{0}
diamond.operator.for.variable.definition=Diamond operator expected.
either.log.or.throw=Either log or throw exception.
empty.public.ctor=This empty public constructor is useless.
field.count={0} should be declared only once.
finalize.implementation.missed.super.finalize=You have to call super.finalize() right after finally opening brace.
finalize.implementation.missed.try.finally=finalize() method should contain try-finally block with super.finalize() call inside finally block.
finalize.implementation.public=finalize() method should have a "protected" visibility.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code for adherence to a set of rules.
// Copyright (C) 2001-2020 the original author or authors.
//
// This library is free software; you can redistribute it and/or
// modify it under the terms of the GNU Lesser General Public
// License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
//
// This library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
// Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
////////////////////////////////////////////////////////////////////////////////

package com.github.sevntu.checkstyle.checks.coding;

import org.junit.Test;

import com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport;
import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
import com.puppycrawl.tools.checkstyle.api.CheckstyleException;

/**
* LoggerDeclarationsCountCheck test.
*
* @author <a href="mailto:[email protected]">Michael Vorburger</a> - completed sevntu integration
*/
public class FieldSingleDeclarationCheckTest extends AbstractModuleTestSupport {

private static final String CLASS_NAME_KEY = "fullyQualifiedClassName";
private static final String CLASS_NAME_VALUE = "org.slf4j.Logger";

private final DefaultConfiguration checkConfig =
createModuleConfig(FieldSingleDeclarationCheck.class);

@Override
protected String getPackageLocation() {
return "com/github/sevntu/checkstyle/checks/coding";
}

@Test
public void testFieldSingleDeclarationOk() throws Exception {
checkConfig.addAttribute(CLASS_NAME_KEY, CLASS_NAME_VALUE);

verify(checkConfig, getPath("InputFieldSingleDeclarationCheckOk.java"), new String[] {});
}

@Test
public void testFieldSingleDeclarationNok() throws Exception {
checkConfig.addAttribute(CLASS_NAME_KEY, CLASS_NAME_VALUE);

final String[] expected = {
"10: " + getCheckMessage(FieldSingleDeclarationCheck.MSG_KEY, CLASS_NAME_VALUE),
};

verify(checkConfig, getPath("InputFieldSingleDeclarationCheckNok.java"), expected);
verify(checkConfig, getPath("InputFieldSingleDeclarationCheckOk.java"), new String[] {});
}

@Test
public void testFieldSingleDeclarationFqnNok() throws Exception {
checkConfig.addAttribute(CLASS_NAME_KEY, CLASS_NAME_VALUE);

final String[] expected = {
"7: " + getCheckMessage(FieldSingleDeclarationCheck.MSG_KEY, CLASS_NAME_VALUE),
};

verify(checkConfig, getPath("InputFieldSingleDeclarationCheckFqnNok.java"), expected);
verify(checkConfig, getPath("InputFieldSingleDeclarationCheckOk.java"), new String[] {});
}

@Test(expected = CheckstyleException.class)
public void testNoClassNameConfigured() throws Exception {
verify(checkConfig, getPath("InputFieldSingleDeclarationCheckOk.java"), new String[] {});
}

@Test
public void testCustomTokens() throws Exception {
checkConfig.addAttribute(CLASS_NAME_KEY, CLASS_NAME_VALUE);
// This is required just so that getAcceptableTokens() gets coverage
checkConfig.addAttribute("tokens", "VARIABLE_DEF");

verify(checkConfig, getPath("InputFieldSingleDeclarationCheckOk.java"), new String[] {});
}

@Test
public void testUnknownClassName() throws Exception {
checkConfig.addAttribute(CLASS_NAME_KEY, "logger");
// This is required just so that getAcceptableTokens() gets coverage
checkConfig.addAttribute("tokens", "VARIABLE_DEF");

verify(checkConfig, getPath("InputFieldSingleDeclarationCheckOk.java"), new String[] {});
verify(checkConfig, getPath("InputFieldSingleDeclarationCheckNok.java"), new String[] {});
verify(checkConfig, getPath("InputFieldSingleDeclarationCheckFqnNok.java"),
new String[] {});
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.github.sevntu.checkstyle.checks.coding;

public class InputFieldSingleDeclarationCheckFqnNok {

// even twice a FQN type names work:
org.slf4j.Logger logger1;
org.slf4j.Logger logger2; // <= Checkstyle violation raised here!

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.github.sevntu.checkstyle.checks.coding;

import java.io.File;
import java.util.logging.Logger;

public class InputFieldSingleDeclarationCheckNok {

// both FQN and import'ed type names work:
Logger logger1;
org.slf4j.Logger logger2; // <= Checkstyle violation raised here!

// some field of another type (req. for test coverage)
File file;

// a method, so that there are not only fields here (req. for test coverage)
void foo() { }

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.github.sevntu.checkstyle.checks.coding;

public class InputFieldSingleDeclarationCheckOk {

org.slf4j.Logger logger;

}
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,19 @@
</param>
</rule>

<rule>
<key>com.github.sevntu.checkstyle.checks.coding.FieldSingleDeclarationCheck</key>
<name>Field Single Declaration Check</name>
<tag>coding</tag>
<description>This checks ensures that classes have at most 1 field of the given className. This can be useful for example to ensure that only one Logger is used.</description>
<configKey>Checker/TreeWalker/com.github.sevntu.checkstyle.checks.coding.FieldSingleDeclarationCheck</configKey>

<param key="fullyQualifiedClassName" type="STRING">
<defaultValue>java.util.logging.Logger</defaultValue>
<description>Fully qualified name of class of which there should only be max. 1 field per class. Example: "java.util.logging.Logger".</description>
</param>
</rule>

<rule>
<key>com.github.sevntu.checkstyle.checks.coding.FinalizeImplementationCheck</key>
<name>Finalize Implementation Check</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void testAllRulesValid() {
Assert.assertEquals("Incorrect repository language", "java", repository.language());

final List<RulesDefinition.Rule> rules = repository.rules();
Assert.assertEquals("Incorrect number of loaded rules", 59, rules.size());
Assert.assertEquals("Incorrect number of loaded rules", 60, rules.size());

for (RulesDefinition.Rule rule : rules) {
Assert.assertNotNull("Rule key is not set", rule.key());
Expand Down

0 comments on commit 7e6921b

Please sign in to comment.