Pull #5351: fixed RequireThisCheck and catch variable handling
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java
index c2e3de9..cc2bed1 100644
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java
@@ -336,7 +336,8 @@
break;
case TokenTypes.PARAMETER_DEF :
if (!CheckUtils.isReceiverParameter(ast)
- && !isLambdaParameter(ast)) {
+ && !isLambdaParameter(ast)
+ && ast.getParent().getType() != TokenTypes.LITERAL_CATCH) {
final DetailAST parameterIdent = ast.findFirstToken(TokenTypes.IDENT);
frame.addIdent(parameterIdent);
}
@@ -366,6 +367,12 @@
final DetailAST ctorFrameNameIdent = ast.findFirstToken(TokenTypes.IDENT);
frameStack.addFirst(new ConstructorFrame(frame, ctorFrameNameIdent));
break;
+ case TokenTypes.LITERAL_CATCH:
+ final AbstractFrame catchFrame = new CatchFrame(frame, ast);
+ catchFrame.addIdent(ast.findFirstToken(TokenTypes.PARAMETER_DEF).findFirstToken(
+ TokenTypes.IDENT));
+ frameStack.addFirst(catchFrame);
+ break;
case TokenTypes.LITERAL_NEW:
if (isAnonymousClassDef(ast)) {
frameStack.addFirst(new AnonymousClassFrame(frame,
@@ -414,6 +421,7 @@
case TokenTypes.SLIST :
case TokenTypes.METHOD_DEF :
case TokenTypes.CTOR_DEF :
+ case TokenTypes.LITERAL_CATCH :
frames.put(ast, frameStack.poll());
break;
case TokenTypes.LITERAL_NEW :
@@ -952,6 +960,8 @@
METHOD_FRAME,
/** Block frame type. */
BLOCK_FRAME,
+ /** Catch frame type. */
+ CATCH_FRAME,
}
/**
@@ -1357,4 +1367,24 @@
return FrameType.BLOCK_FRAME;
}
}
+
+ /**
+ * A frame initiated on entering a catch block; holds local catch variable names.
+ * @author Richard Veach
+ */
+ public static class CatchFrame extends AbstractFrame {
+ /**
+ * Creates catch frame.
+ * @param parent parent frame.
+ * @param ident ident frame name ident.
+ */
+ protected CatchFrame(AbstractFrame parent, DetailAST ident) {
+ super(parent, ident);
+ }
+
+ @Override
+ public FrameType getType() {
+ return FrameType.CATCH_FRAME;
+ }
+ }
}
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java
index 6474fc4..c2289d0 100644
--- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java
@@ -22,6 +22,7 @@
import static com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck.MSG_METHOD;
import static com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck.MSG_VARIABLE;
+import java.lang.reflect.Constructor;
import java.util.SortedSet;
import org.junit.Assert;
@@ -33,6 +34,7 @@
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.LocalizedMessage;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+import com.puppycrawl.tools.checkstyle.internal.utils.TestUtil;
import com.puppycrawl.tools.checkstyle.utils.CommonUtils;
public class RequireThisCheckTest extends AbstractModuleTestSupport {
@@ -296,6 +298,16 @@
}
@Test
+ public void testCatchVariables() throws Exception {
+ final DefaultConfiguration checkConfig = createModuleConfig(RequireThisCheck.class);
+ checkConfig.addAttribute("validateOnlyOverlapping", "false");
+ final String[] expected = {
+ "29:21: " + getCheckMessage(MSG_VARIABLE, "ex", ""),
+ };
+ verify(checkConfig, getPath("InputRequireThisCatchVariables.java"), expected);
+ }
+
+ @Test
public void test() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(RequireThisCheck.class);
final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
@@ -309,4 +321,18 @@
final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
verify(checkConfig, getPath("InputRequireThisExtendedMethod.java"), expected);
}
+
+ @Test
+ public void testUnusedMethod() throws Exception {
+ final DetailAST ident = new DetailAST();
+ ident.setText("testName");
+
+ final Class<?> cls = Class.forName(RequireThisCheck.class.getName() + "$CatchFrame");
+ final Constructor<?> constructor = cls.getDeclaredConstructors()[0];
+ constructor.setAccessible(true);
+ final Object o = constructor.newInstance(null, ident);
+
+ Assert.assertEquals("expected ident token", ident,
+ TestUtil.getClassDeclaredMethod(cls, "getFrameNameIdent").invoke(o));
+ }
}
diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/requirethis/InputRequireThisCatchVariables.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/requirethis/InputRequireThisCatchVariables.java
new file mode 100644
index 0000000..920dd63
--- /dev/null
+++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/requirethis/InputRequireThisCatchVariables.java
@@ -0,0 +1,40 @@
+package com.puppycrawl.tools.checkstyle.checks.coding.requirethis;
+
+public class InputRequireThisCatchVariables extends Thread {
+ private Throwable ex;
+
+ public InputRequireThisCatchVariables(Throwable ex) {
+ this.ex = ex;
+ }
+
+ @Override
+ public void run() {
+ if (this.ex != null) {
+ try {
+ exceptional(this.ex);
+ }
+ catch (RuntimeException ex) {
+ if (ex == this.ex) {
+ debug("Expected exception thrown", ex);
+ }
+ else {
+ ex.printStackTrace();
+ }
+ }
+ catch (Error err) {
+ if (err == this.ex) {
+ debug("Expected exception thrown", err);
+ }
+ else {
+ ex.printStackTrace();
+ }
+ }
+ catch (Throwable ex) {
+ ex.printStackTrace();
+ }
+ }
+ }
+
+ private static void exceptional(Throwable ex) {}
+ private static void debug(String message, Throwable err) {}
+}
