Some enhancement and documentation for RequireThis check.

docs/config_coding.html

@@ -109,6 +109,9 @@
         <li>
           <a href="#ReturnCount">ReturnCount</a>
         </li>
+        <li>
+          <a href="#RequireThis">RequireThis</a>
+        </li>
         <li>
           <a href="#SimplifyBooleanExpression">SimplifyBooleanExpression</a>
         </li>
@@ -1511,6 +1514,58 @@ return !valid();
       <a href="config.html#treewalker">TreeWalker</a>
       </p>
 
+      <!-- --> <a name="RequireThis"></a> <h2>RequireThis</h2> <h4>Description</h4>
+      <p class="body">
+      Checks that code doesn't rely on the &quot;this.&quot; default,
+      i.e. references to instance variables and methods of the present
+      object are explicitly of the form &quot;this.varName&quot; or
+      &quot;this.methodName(args)&quot;.
+      </p>
+      <h4>Properties</h4>
+      <table width="100%" border="1" cellpadding="5" class="body">
+        <tr class="header">
+          <th>name</th>
+          <th>description</th>
+          <th>type</th>
+          <th>default value</th>
+        </tr>
+        <tr>
+          <td>checkFields</td>
+          <td>whether we should check fields usage or not</td>
+          <td><a href="property_types.html#boolean">boolean</a></td>
+          <td><span class="default">true</span></td>
+        </tr>
+        <tr>
+          <td>checkMethods</td>
+          <td>whether we should check methods usage or not</td>
+          <td><a href="property_types.html#boolean">boolean</a></td>
+          <td><span class="default">true</span></td>
+        </tr>
+      </table>
+      <h4>Examples</h4>
+      <p class="body">
+      To configure the default check:
+      </p>
+      <pre class="body">
+&lt;module name=&quot;RequireThis&quot;/&gt;
+      </pre>
+      <p class="body">
+      To configure to check <code>this</code> qualifier for fields only:
+      </p>
+      <pre class="body">
+&lt;module name="RequireThis"&gt;
+    &lt;property name=&quot;checkMethods&quot; value=&quot;false&quot;/&gt;
+&lt;/module&gt;
+      </pre>
+      <h4>Package</h4>
+      <p class="body">
+      com.puppycrawl.tools.checkstyle.checks.coding
+      </p>
+      <h4>Parent Module</h4>
+      <p class="body">
+      <a href="config.html#treewalker">TreeWalker</a>
+      </p>
+
     </td>
   </tr>
 </table>

docs/releasenotes.html

@@ -102,6 +102,11 @@
           MultipleVariableDeclarations, requests 639233, 753858,
           844705)</li>
 
+          <li class="body">Added check to ensure that references to
+          non-static fields and methods should be quailified.
+          (module RequireThis, contributed by Stephen Bloch, requests
+          755550, 696295).</li>
+
         </ul>
 
       <p class="body">

src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java

@@ -27,22 +27,23 @@ import java.util.Iterator;
 import java.util.LinkedList;
 
 /**
- * <p>Checks that code doesn't rely on the "this." default, i.e. references to
- * instance variables and methods of the present object are explicitly of
- * the form "this.varName" or "this.methodName(args)".</P>
+ * <p>Checks that code doesn't rely on the &quot;this.&quot; default,
+ * i.e. references to instance variables and methods of the present
+ * object are explicitly of the form &quot;this.varName&quot; or
+ * &quot;this.methodName(args)&quot;.
+ *</p>
  * <p>Examples of use:
  * <pre>
- * &lt;module name="RequireThis"/&gt;
+ * &lt;module name=&quot;RequireThis&quot;/&gt;
  * </pre>
- * The following isn't implemented yet:
+ * An example of how to configure to check <code>this</code> qualifier for
+ * methods only:
  * <pre>
- * &lt;module name="RequireThis"&gt;
- *   &lt;property name="kinds" value="variables,methods"&gt;\
+ * &lt;module name=&quot;RequireThis&quot;&gt;
+ *   &lt;property name=&quot;checkFields&quot; value=&quot;false&quot;/&gt;
+ *   &lt;property name=&quot;checkMethods&quot; value=&quot;true&quot;/&gt;
  * &lt;/module&gt;
  * </pre>
- * ("variables,methods" is the default; either one can be specified by
- * itself, or you could even specify the empty string, in which case
- * the module would have no effect.)
  * </p>
  * <p>Limitations: I'm not currently doing anything about static variables
  * or catch-blocks.  Static methods invoked on a class name seem to be OK;
@@ -53,9 +54,47 @@ import java.util.LinkedList;
  * <code>HiddenFieldCheck</code>.</p>
  *
  * @author Stephen Bloch
+ * @author o_sukhodolsky
  */
 public class RequireThisCheck extends Check
 {
+    /** whether we should check fields usage. */
+    private boolean mCheckFields = true;
+    /** whether we should check methods usage. */
+    private boolean mCheckMethods = true;
+
+    /**
+     * Setter for checkFields property.
+     * @param aCheckFields should we check fields usage or not.
+     */
+    public void setCheckFields(boolean aCheckFields)
+    {
+        mCheckFields = aCheckFields;
+    }
+    /**
+     * @return true if we should check fields usage false overwise.
+     */
+    public boolean getCheckFields()
+    {
+        return mCheckFields;
+    }
+
+    /**
+     * Setter for checkMethods property.
+     * @param aCheckMethods should we check methods usage or not.
+     */
+    public void setCheckMethods(boolean aCheckMethods)
+    {
+        mCheckMethods = aCheckMethods;
+    }
+    /**
+     * @return true if we should check methods usage false overwise.
+     */
+    public boolean getCheckMethods()
+    {
+        return mCheckMethods;
+    }
+
     /** Stack of varibale declaration frames. */
     private FrameStack mFrames;
 
@@ -75,7 +114,6 @@ public class RequireThisCheck extends Check
             TokenTypes.METHOD_DEF,
             TokenTypes.CTOR_DEF,
             TokenTypes.SLIST,
-            // TokenTypes.LITERAL_CATCH also introduces a parameter name
         };
     }
 
@@ -154,8 +192,17 @@ public class RequireThisCheck extends Check
     {
         int parentType = aAST.getParent().getType();
 
+        // let's check method calls
         if (parentType == TokenTypes.METHOD_CALL) {
-            log(aAST, "require.this.method", aAST.getText());
+            if (mCheckMethods) {
+                log(aAST, "require.this.method", aAST.getText());
+            }
+            return;
+        }
+
+        // let's check fields
+        if (!mCheckFields) {
+            // we shouldn't check fields
             return;
         }
         if (parentType == TokenTypes.DOT
@@ -180,12 +227,13 @@ public class RequireThisCheck extends Check
             return;
         }
 
-        final LexicalFrame declared = this.mFrames.findFrame(aAST.getText());
+        final String name = aAST.getText();
+        final LexicalFrame declared = this.mFrames.findFrame(name);
         if (declared == null) {
-            log(aAST, "require.this.unfound.variable", aAST.getText());
+            log(aAST, "require.this.unfound.variable", name);
         }
         else if (declared instanceof ClassFrame) {
-            log(aAST, "require.this.variable", aAST.getText());
+            log(aAST, "require.this.variable", name);
         }
     }
 } // end class RequireThis

src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputRequireThis.java

@@ -9,5 +9,12 @@ public class InputRequireThis {
         this.i = i;
         method1();
         j--;
+        try {
+            this.method1();
+        }
+        catch (RuntimeException e) {
+            e.printStackTrace();
+        }
+        this.i--;
     }
 }

src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java

@@ -20,4 +20,31 @@ public class RequireThisCheckTest extends BaseCheckTestCase
                getPath("coding" + File.separator + "InputRequireThis.java"),
                expected);
     }
+
+    public void testMethodsOnly() throws Exception
+    {
+        final DefaultConfiguration checkConfig =
+            createCheckConfig(RequireThisCheck.class);
+        checkConfig.addAttribute("checkFields", "false");
+        final String[] expected = {
+            "10:9: Method call to 'method1' needs \"this.\".",
+        };
+        verify(checkConfig,
+               getPath("coding" + File.separator + "InputRequireThis.java"),
+               expected);
+    }
+
+    public void testFieldsOnly() throws Exception
+    {
+        final DefaultConfiguration checkConfig =
+            createCheckConfig(RequireThisCheck.class);
+        checkConfig.addAttribute("checkMethods", "false");
+        final String[] expected = {
+            "4:9: Reference to instance variable 'i' needs \"this.\".",
+            "11:9: Unable find where 'j' is declared.",
+        };
+        verify(checkConfig,
+               getPath("coding" + File.separator + "InputRequireThis.java"),
+               expected);
+    }
 }