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

Remove non-testable reflection code from HandlerFactory. #1270 

In `HandlerFactory` any reflection-related exception would mean programmer's mistake with no recovery. In such cases checked exceptions are useless, so new methods in utils are provided to wrap any checked exceptions into unchecked ones.
This commit is contained in:
Michal Kordas 2015-07-08 22:23:58 +02:00 committed by Roman Ivanov
parent a07cae0aca
commit 8f19f1d706
4 changed files with 89 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pom.xml
View File

@ -1138,7 +1138,7 @@
<regex><pattern>.*.checks.indentation.ElseHandler</pattern><branchRate>75</branchRate><lineRate>100</lineRate></regex>
<regex><pattern>.*.checks.indentation.AbstractExpressionHandler</pattern><branchRate>91</branchRate><lineRate>97</lineRate></regex>
<regex><pattern>.*.checks.indentation.ForHandler</pattern><branchRate>75</branchRate><lineRate>95</lineRate></regex>
<regex><pattern>.*.checks.indentation.HandlerFactory</pattern><branchRate>77</branchRate><lineRate>81</lineRate></regex>
<regex><pattern>.*.checks.indentation.HandlerFactory</pattern><branchRate>81</branchRate><lineRate>100</lineRate></regex>
<regex><pattern>.*.checks.indentation.ImportHandler</pattern><branchRate>50</branchRate><lineRate>87</lineRate></regex>
<regex><pattern>.*.checks.indentation.IndentationCheck</pattern><branchRate>100</branchRate><lineRate>93</lineRate></regex>
<regex><pattern>.*.checks.indentation.IndexHandler</pattern><branchRate>100</branchRate><lineRate>75</lineRate></regex>

33
src/main/java/com/puppycrawl/tools/checkstyle/Utils.java
View File

@ -24,7 +24,9 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import org.apache.commons.beanutils.ConversionException;
import java.io.File;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ResourceBundle;
@ -336,4 +338,35 @@ public final class Utils {
public static boolean isCommentType(String type) {
return isCommentType(getTokenId(type));
}
/**
* @param targetClass from which constructor is returned
* @param parameterTypes of constructor
* @return constructor of targetClass or {@link IllegalStateException} if any exception occurs
* @see Class#getConstructor(Class[])
*/
public static Constructor<?> getConstructor(Class<?> targetClass, Class<?>... parameterTypes) {
try {
return targetClass.getConstructor(parameterTypes);
}
catch (NoSuchMethodException ex) {
throw new IllegalStateException(ex);
}
}
/**
* @param constructor to invoke
* @param parameters to pass to constructor
* @param <T> type of constructor
* @return new instance of class or {@link IllegalStateException} if any exception occurs
* @see Constructor#newInstance(Object...)
*/
public static <T> T invokeConstructor(Constructor<T> constructor, Object... parameters) {
try {
return constructor.newInstance(parameters);
}
catch (InstantiationException | IllegalAccessException | InvocationTargetException ex) {
throw new IllegalStateException(ex);
}
}
}

51
src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/HandlerFactory.java
View File

@ -20,14 +20,12 @@
package com.puppycrawl.tools.checkstyle.checks.indentation;
import com.google.common.collect.Maps;
import com.puppycrawl.tools.checkstyle.Utils;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.Map;
import java.util.Set;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
/**
* Factory for handlers. Looks up constructor via reflection.
@ -35,9 +33,6 @@ import org.apache.commons.logging.LogFactory;
* @author jrichard
*/
public class HandlerFactory {
/** Logger for indentation check. */
private static final Log LOG = LogFactory.getLog(HandlerFactory.class);
/**
* Registered handlers.
*/
@ -90,19 +85,12 @@ public class HandlerFactory {
* the handler to register
*/
private void register(int type, Class<?> handlerClass) {
try {
final Constructor<?> ctor = handlerClass
.getConstructor(new Class[] {IndentationCheck.class,
DetailAST.class, // current AST
AbstractExpressionHandler.class, // parent
});
typeHandlers.put(type, ctor);
}
catch (final NoSuchMethodException | SecurityException e) {
final String message = "couldn't find ctor for " + handlerClass;
LOG.debug(message, e);
throw new RuntimeException(message);
}
final Constructor<?> ctor = Utils.getConstructor(handlerClass,
IndentationCheck.class,
DetailAST.class, // current AST
AbstractExpressionHandler.class // parent
);
typeHandlers.put(type, ctor);
}
/**
@ -154,26 +142,11 @@ public class HandlerFactory {
}
AbstractExpressionHandler expHandler = null;
try {
final Constructor<?> handlerCtor =
typeHandlers.get(ast.getType());
if (handlerCtor != null) {
expHandler = (AbstractExpressionHandler) handlerCtor.newInstance(
indentCheck, ast, parent);
}
}
catch (final InstantiationException | InvocationTargetException e) {
final String message = "couldn't instantiate constructor for " + ast;
LOG.debug(message, e);
throw new RuntimeException(message);
}
catch (final IllegalAccessException e) {
final String message = "couldn't access constructor for " + ast;
LOG.debug(message, e);
throw new RuntimeException(message);
}
if (expHandler == null) {
throw new RuntimeException("no handler for type " + ast.getType());
final Constructor<?> handlerCtor =
typeHandlers.get(ast.getType());
if (handlerCtor != null) {
expHandler = (AbstractExpressionHandler) Utils.invokeConstructor(
handlerCtor, indentCheck, ast, parent);
}
return expHandler;
}

43
src/test/java/com/puppycrawl/tools/checkstyle/UtilsTest.java
View File

@ -26,9 +26,12 @@ import static com.puppycrawl.tools.checkstyle.Utils.fileExtensionMatches;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.lang.reflect.Constructor;
import java.io.File;
import java.io.IOException;
import java.util.Dictionary;
import org.apache.commons.beanutils.ConversionException;
import org.junit.Test;
@ -124,4 +127,44 @@ public class UtilsTest {
boolean result = Utils.isPatternValid("some[invalidPattern");
assertFalse(result);
}
@Test
public void testGetExistingConstructor() throws NoSuchMethodException {
Constructor<?> constructor = Utils.getConstructor(String.class, String.class);
assertEquals(String.class.getConstructor(String.class), constructor);
}
@Test
public void testGetNonExistingConstructor() throws NoSuchMethodException {
try {
Utils.getConstructor(Math.class);
fail();
}
catch (IllegalStateException expected) {
assertEquals(NoSuchMethodException.class, expected.getCause().getClass());
}
}
@Test
public void testInvokeConstructor() throws NoSuchMethodException {
Constructor<String> constructor = String.class.getConstructor(String.class);
String constructedString = Utils.invokeConstructor(constructor, "string");
assertEquals("string", constructedString);
}
@Test
public void testInvokeConstructorThatFails() throws NoSuchMethodException {
Constructor<Dictionary> constructor = Dictionary.class.getConstructor();
try {
Utils.invokeConstructor(constructor);
fail();
}
catch (IllegalStateException expected) {
assertEquals(InstantiationException.class, expected.getCause().getClass());
}
}
}