Pull #5351: fixed RequireThisCheck and catch variable handling

3 files changed, 97 insertions, 1 deletions

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 c2e3de96e..cc2bed11a 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 @@ public class RequireThisCheck extends AbstractCheck {
                 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 @@ public class RequireThisCheck extends AbstractCheck {
                 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 @@ public class RequireThisCheck extends AbstractCheck {
             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 @@ public class RequireThisCheck extends AbstractCheck {
         METHOD_FRAME,
         /** Block frame type. */
         BLOCK_FRAME,
+        /** Catch frame type. */
+        CATCH_FRAME,
     }
 
     /**
@@ -1357,4 +1367,24 @@ public class RequireThisCheck extends AbstractCheck {
             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 6474fc4e1..c2289d0e9 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 @@ package com.puppycrawl.tools.checkstyle.checks.coding;
 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.DefaultConfiguration;
 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 @@ public class RequireThisCheckTest extends AbstractModuleTestSupport {
     }
 
     @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 @@ public class RequireThisCheckTest extends AbstractModuleTestSupport {
         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 000000000..920dd6369
--- /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) {}
+}