Skip to content

Commit

Permalink
Issue checkstyle#78: extract: grab property info
Browse files Browse the repository at this point in the history
  • Loading branch information
Luolc committed Aug 21, 2017
1 parent f92362c commit 577d413
Show file tree
Hide file tree
Showing 8 changed files with 243 additions and 6 deletions.
5 changes: 5 additions & 0 deletions config/findbugs-exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@
<Class name="~.*\.Immutable.*"/>
<Bug pattern="NP_METHOD_PARAMETER_TIGHTENS_ANNOTATION"/>
</Match>
<Match>
<!-- We can't change generated source code. -->
<Class name="~.*\.GsonAdapters.*"/>
<Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD"/>
</Match>
</FindBugsFilter>
2 changes: 1 addition & 1 deletion config/sevntu_suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
"http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">

<suppressions>
<suppress checks="MultipleStringLiteralsExtended" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="MultipleStringLiteralsExtended" files="[\\/]ExtractInfoGeneratorTest.java$|.*[\\/]src[\\/]test[\\/]"/>
</suppressions>
2 changes: 1 addition & 1 deletion config/suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<suppress checks="MagicNumber" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="AvoidStaticImport" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="WriteTag" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="MultipleStringLiterals" files=".*[\\/]src[\\/]test[\\/]"/>
<suppress checks="MultipleStringLiterals" files="[\\/]ExtractInfoGeneratorTest.java$|.*[\\/]src[\\/]test[\\/]"/>

<suppress checks="AbstractClassName" files=".*[\\/]data[\\/]"/>
</suppressions>
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package com.github.checkstyle.regression.data;

import java.util.List;

import org.immutables.gson.Gson;
import org.immutables.value.Value;

Expand Down Expand Up @@ -48,11 +50,46 @@ public abstract class ModuleExtractInfo {
*/
public abstract String parent();

/**
* The properties of this module.
* @return the properties of this module
*/
public abstract List<ModuleProperty> properties();

/**
* The full qualified name of this module.
* @return the full qualified name of this module
*/
public String fullName() {
return packageName() + "." + name();
}

/** Represents a property of checkstyle module. */
@Gson.TypeAdapters
@Value.Immutable
public interface ModuleProperty {
/**
* The name of this property.
* @return the name of this property
*/
String name();

/**
* The type of this property.
* The value should be one of the followings:
* - Pattern
* - SeverityLevel
* - boolean
* - Scope
* - double[]
* - int[]
* - String[]
* - String
* - URI
* - AccessModifier[]
* - int
* @return the type of this property
*/
String type();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,26 @@

package com.puppycrawl.tools.checkstyle;

import java.beans.PropertyDescriptor;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;

import org.apache.commons.beanutils.PropertyUtils;
import org.junit.Test;

import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck;
import com.puppycrawl.tools.checkstyle.checks.javadoc.AbstractJavadocCheck;
import com.puppycrawl.tools.checkstyle.internal.CheckUtil;
import com.puppycrawl.tools.checkstyle.internal.TestUtils;
import com.puppycrawl.tools.checkstyle.utils.ModuleReflectionUtils;

/**
Expand All @@ -40,12 +47,57 @@
* @author LuoLiangchen
*/
public class ExtractInfoGeneratorTest {
/** Modules which do not have global properties to drop. */
private static final List<String> XML_FILESET_LIST = Arrays.asList(
"TreeWalker",
"Checker",
"Header",
"Translation",
"SeverityMatchFilter",
"SuppressionFilter",
"SuppressWarningsFilter",
"BeforeExecutionExclusionFileFilter",
"RegexpHeader",
"RegexpOnFilename",
"RegexpSingleline",
"RegexpMultiline",
"JavadocPackage",
"NewlineAtEndOfFile",
"UniqueProperties",
"FileLength",
"FileTabCharacter"
);

/** Properties of abstract check. */
private static final Set<String> CHECK_PROPERTIES = getProperties(AbstractCheck.class);

/** Properties of abstract Javadoc check. */
private static final Set<String> JAVADOC_CHECK_PROPERTIES =
getProperties(AbstractJavadocCheck.class);

/** Properties of abstract file-set check. */
private static final Set<String> FILESET_PROPERTIES = getProperties(AbstractFileSetCheck.class);

/** Properties without document. */
private static final List<String> UNDOCUMENTED_PROPERTIES = Arrays.asList(
"Checker.classLoader",
"Checker.classloader",
"Checker.moduleClassLoader",
"Checker.moduleFactory",
"TreeWalker.classLoader",
"TreeWalker.moduleFactory",
"TreeWalker.cacheFile",
"TreeWalker.upChild",
"SuppressWithNearbyCommentFilter.fileContents",
"SuppressionCommentFilter.fileContents"
);

/**
* Generates the extract info file named as "checkstyle_modules.json".
* @throws IOException failure when generating the file
* @throws Exception failure when generating the file
*/
@Test
public void generateExtractInfoFile() throws IOException {
public void generateExtractInfoFile() throws Exception {
final List<Class<?>> modules = new ArrayList<>(CheckUtil.getCheckstyleModules());
modules.sort(Comparator.comparing(Class::getSimpleName));
final JsonUtil.JsonArray moduleJsonArray = new JsonUtil.JsonArray();
Expand All @@ -62,8 +114,10 @@ public void generateExtractInfoFile() throws IOException {
* Creates Json object for a module from the module class.
* @param clazz the given module class
* @return the Json object describing the extract info of the module
* @throws Exception failure when creating Json object
*/
private static JsonUtil.JsonObject createJsonObjectFromModuleClass(Class<?> clazz) {
private static JsonUtil.JsonObject createJsonObjectFromModuleClass(Class<?> clazz)
throws Exception {
final JsonUtil.JsonObject object = new JsonUtil.JsonObject();

final String name = clazz.getSimpleName();
Expand Down Expand Up @@ -94,6 +148,116 @@ else if (ModuleReflectionUtils.isRootModule(clazz)) {
object.add("interfaces", interfaces);
object.add("hierarchies", hierarchies);

final JsonUtil.JsonArray properties = new JsonUtil.JsonArray();
for (String propertyName : getNecessaryProperties(clazz)) {
final JsonUtil.JsonObject property = new JsonUtil.JsonObject();
property.addProperty("name", propertyName);
Arrays.stream(PropertyUtils.getPropertyDescriptors(clazz))
.filter(p -> p.getName().equals(propertyName))
.map(PropertyDescriptor::getPropertyType)
.map(Class::getSimpleName)
.findAny()
.ifPresent(type -> property.addProperty("type", type));
properties.add(property);
}
object.add("properties", properties);

return object;
}

/**
* Gets the necessary properties of a checkstyle module.
* Global properties and undocumented properties are not necessary for us.
* @param clazz the class instance of the given module
* @return a set of the necessary properties of the module
* @throws Exception failure when getting properties
*/
// -@cs[CyclomaticComplexity] many different kinds of module
private static Set<String> getNecessaryProperties(Class<?> clazz)
throws Exception {
final Set<String> properties = getProperties(clazz);
if (hasParentModule(clazz.getSimpleName())) {
if (AbstractJavadocCheck.class.isAssignableFrom(clazz)) {
properties.removeAll(JAVADOC_CHECK_PROPERTIES);
}
else if (ModuleReflectionUtils.isCheckstyleCheck(clazz)) {
properties.removeAll(CHECK_PROPERTIES);
}
}
if (ModuleReflectionUtils.isFileSetModule(clazz)) {
properties.removeAll(FILESET_PROPERTIES);

// override
properties.add("fileExtensions");
}

// undocumented properties are not necessary
properties.removeIf(prop -> UNDOCUMENTED_PROPERTIES.contains(
clazz.getSimpleName() + "." + prop));

final PackageObjectFactory factory = TestUtils.getPackageObjectFactory();
final Object instance = factory.createModule(clazz.getSimpleName());

if (ModuleReflectionUtils.isCheckstyleCheck(clazz)) {
final AbstractCheck check = (AbstractCheck) instance;

final int[] acceptableTokens = check.getAcceptableTokens();
Arrays.sort(acceptableTokens);
final int[] defaultTokens = check.getDefaultTokens();
Arrays.sort(defaultTokens);
final int[] requiredTokens = check.getRequiredTokens();
Arrays.sort(requiredTokens);

if (!Arrays.equals(acceptableTokens, defaultTokens)
|| !Arrays.equals(acceptableTokens, requiredTokens)) {
properties.add("tokens");
}
}

if (AbstractJavadocCheck.class.isAssignableFrom(clazz)) {
final AbstractJavadocCheck check = (AbstractJavadocCheck) instance;

final int[] acceptableJavadocTokens = check.getAcceptableJavadocTokens();
Arrays.sort(acceptableJavadocTokens);
final int[] defaultJavadocTokens = check.getDefaultJavadocTokens();
Arrays.sort(defaultJavadocTokens);
final int[] requiredJavadocTokens = check.getRequiredJavadocTokens();
Arrays.sort(requiredJavadocTokens);

if (!Arrays.equals(acceptableJavadocTokens, defaultJavadocTokens)
|| !Arrays.equals(acceptableJavadocTokens, requiredJavadocTokens)) {
properties.add("javadocTokens");
}
}

return properties;
}

/**
* Gets the properties of a checkstyle module.
* @param clazz the class instance of the given module
* @return a set of the properties of the module
*/
private static Set<String> getProperties(Class<?> clazz) {
final Set<String> result = new TreeSet<>();
final PropertyDescriptor[] map = PropertyUtils.getPropertyDescriptors(clazz);

for (PropertyDescriptor p : map) {
if (p.getWriteMethod() != null) {
result.add(p.getName());
}
}

return result;
}

/**
* Checks whether a module has a parent that may contains global properties.
* @param className the class name of given module
* @return true if the module has a parent
*/
private static boolean hasParentModule(String className) {
return !XML_FILESET_LIST.contains(className) && XML_FILESET_LIST.stream()
.map(name -> name + "Check").noneMatch(name -> name.equals(className));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.junit.Test;

import com.github.checkstyle.regression.data.ImmutableModuleExtractInfo;
import com.github.checkstyle.regression.data.ImmutableModuleProperty;
import com.github.checkstyle.regression.data.ModuleExtractInfo;

public class ExtractInfoProcessorTest {
Expand Down Expand Up @@ -69,6 +70,14 @@ public void testGetModuleExtractInfosFromReader() throws Exception {
.name(module1)
.packageName(BASE_PACKAGE + ".checks")
.parent("Checker")
.addProperties(ImmutableModuleProperty.builder()
.name("fileExtensions")
.type("String[]")
.build())
.addProperties(ImmutableModuleProperty.builder()
.name("lineSeparator")
.type("String")
.build())
.build();
final String module2 = "EmptyStatementCheck";
final ModuleExtractInfo extractInfo2 = ImmutableModuleExtractInfo.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.github.checkstyle.regression.data.ImmutableGitChange;
import com.github.checkstyle.regression.data.ImmutableModuleExtractInfo;
import com.github.checkstyle.regression.data.ImmutableModuleInfo;
import com.github.checkstyle.regression.data.ImmutableModuleProperty;
import com.github.checkstyle.regression.data.ModuleExtractInfo;
import com.github.checkstyle.regression.data.ModuleInfo;
import com.github.checkstyle.regression.extract.ExtractInfoProcessor;
Expand Down Expand Up @@ -117,6 +118,14 @@ public void testGenerateConfigNodesForValidChanges2() {
.name("NewlineAtEndOfFileCheck")
.packageName(BASE_PACKAGE + ".checks")
.parent("Checker")
.addProperties(ImmutableModuleProperty.builder()
.name("fileExtensions")
.type("String[]")
.build())
.addProperties(ImmutableModuleProperty.builder()
.name("lineSeparator")
.type("String")
.build())
.build();
final List<ModuleInfo> moduleInfos =
ModuleCollector.generate(changes);
Expand Down
13 changes: 13 additions & 0 deletions src/test/resources/checkstyle_modules.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@
"com.puppycrawl.tools.checkstyle.api.FileSetCheck",
"com.puppycrawl.tools.checkstyle.api.Configurable",
"com.puppycrawl.tools.checkstyle.api.Contextualizable"
],
"properties": [
{
"name": "fileExtensions",
"type": "String[]"
},
{
"name": "lineSeparator",
"type": "String"
}
]
},
{
Expand All @@ -26,6 +36,9 @@
"interfaces": [
"com.puppycrawl.tools.checkstyle.api.Configurable",
"com.puppycrawl.tools.checkstyle.api.Contextualizable"
],
"properties": [

]
}
]

0 comments on commit 577d413

Please sign in to comment.