diff --git a/docs/config_coding.html b/docs/config_coding.html index fcf8958cc..df09331f9 100644 --- a/docs/config_coding.html +++ b/docs/config_coding.html @@ -109,6 +109,9 @@
  • ReturnCount
    • +
  • + RequireThis +
  • SimplifyBooleanExpression
    • @@ -1511,6 +1514,58 @@ return !valid(); TreeWalker

    +

    RequireThis

    Description

    +

    + 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)". +

    +

    Properties

    + + + + + + + + + + + + + + + + + + + +
    namedescriptiontypedefault value
    checkFieldswhether we should check fields usage or notbooleantrue
    checkMethodswhether we should check methods usage or notbooleantrue
    +

    Examples

    +

    + To configure the default check: +

    + 
    +<module name="RequireThis"/>
+
    +

    + To configure to check this qualifier for fields only: +

    + 
    +<module name="RequireThis">
+    <property name="checkMethods" value="false"/>
+</module>
+
    +

    Package

    +

    + com.puppycrawl.tools.checkstyle.checks.coding +

    +

    Parent Module

    +

    + TreeWalker +

    + diff --git a/docs/releasenotes.html b/docs/releasenotes.html index 760835dfc..9d92b5a37 100644 --- a/docs/releasenotes.html +++ b/docs/releasenotes.html @@ -102,6 +102,11 @@ MultipleVariableDeclarations, requests 639233, 753858, 844705) +
  • Added check to ensure that references to + non-static fields and methods should be quailified. + (module RequireThis, contributed by Stephen Bloch, requests + 755550, 696295).
    • +

    diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java index 972fe12ed..3364a7447 100755 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java @@ -27,22 +27,23 @@ import java.util.Iterator; import java.util.LinkedList; /** - *

    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)".

    + *

    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)". + *

    *

    Examples of use: * 

    - * <module name="RequireThis"/>
+ * <module name="RequireThis"/>
  *
    - * The following isn't implemented yet: + * An example of how to configure to check this qualifier for + * methods only: * 
    - * <module name="RequireThis">
- *   <property name="kinds" value="variables,methods">\
+ * <module name="RequireThis">
+ *   <property name="checkFields" value="false"/>
+ *   <property name="checkMethods" value="true"/>
  * </module>
  *
    - * ("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.) *

    *

    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; * HiddenFieldCheck.

    * * @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 diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputRequireThis.java b/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputRequireThis.java index 7f8fb8663..e4488ab4b 100755 --- a/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputRequireThis.java +++ b/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--; } } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java index 533652563..10ece644a 100755 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java +++ b/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); + } }