View Javadoc
1   ///////////////////////////////////////////////////////////////////////////////////////////////
2   // checkstyle: Checks Java source code and other text files for adherence to a set of rules.
3   // Copyright (C) 2001-2024 the original author or authors.
4   //
5   // This library is free software; you can redistribute it and/or
6   // modify it under the terms of the GNU Lesser General Public
7   // License as published by the Free Software Foundation; either
8   // version 2.1 of the License, or (at your option) any later version.
9   //
10  // This library is distributed in the hope that it will be useful,
11  // but WITHOUT ANY WARRANTY; without even the implied warranty of
12  // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
13  // Lesser General Public License for more details.
14  //
15  // You should have received a copy of the GNU Lesser General Public
16  // License along with this library; if not, write to the Free Software
17  // Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
18  ///////////////////////////////////////////////////////////////////////////////////////////////
19  
20  package com.puppycrawl.tools.checkstyle.checks.coding;
21  
22  import java.util.BitSet;
23  
24  import com.puppycrawl.tools.checkstyle.StatelessCheck;
25  import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
26  import com.puppycrawl.tools.checkstyle.api.DetailAST;
27  import com.puppycrawl.tools.checkstyle.api.TokenTypes;
28  import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
29  import com.puppycrawl.tools.checkstyle.utils.TokenUtil;
30  
31  /**
32   * <p>
33   * Checks for assignments in subexpressions, such as in
34   * {@code String s = Integer.toString(i = 2);}.
35   * </p>
36   * <p>
37   * Rationale: Except for the loop idioms,
38   * all assignments should occur in their own top-level statement to increase readability.
39   * With inner assignments like the one given above, it is difficult to see all places
40   * where a variable is set.
41   * </p>
42   * <p>
43   * Note: Check allows usage of the popular assignments in loops:
44   * </p>
45   * <pre>
46   * String line;
47   * while ((line = bufferedReader.readLine()) != null) { // OK
48   *   // process the line
49   * }
50   *
51   * for (;(line = bufferedReader.readLine()) != null;) { // OK
52   *   // process the line
53   * }
54   *
55   * do {
56   *   // process the line
57   * }
58   * while ((line = bufferedReader.readLine()) != null); // OK
59   * </pre>
60   * <p>
61   * Assignment inside a condition is not a problem here, as the assignment is surrounded
62   * by an extra pair of parentheses. The comparison is {@code != null} and there is no chance that
63   * intention was to write {@code line == reader.readLine()}.
64   * </p>
65   * <p>
66   * Parent is {@code com.puppycrawl.tools.checkstyle.TreeWalker}
67   * </p>
68   * <p>
69   * Violation Message Keys:
70   * </p>
71   * <ul>
72   * <li>
73   * {@code assignment.inner.avoid}
74   * </li>
75   * </ul>
76   *
77   * @since 3.0
78   */
79  @StatelessCheck
80  public class InnerAssignmentCheck
81          extends AbstractCheck {
82  
83      /**
84       * A key is pointing to the warning message text in "messages.properties"
85       * file.
86       */
87      public static final String MSG_KEY = "assignment.inner.avoid";
88  
89      /**
90       * Allowed AST types from an assignment AST node
91       * towards the root.
92       */
93      private static final int[][] ALLOWED_ASSIGNMENT_CONTEXT = {
94          {TokenTypes.EXPR, TokenTypes.SLIST},
95          {TokenTypes.VARIABLE_DEF},
96          {TokenTypes.EXPR, TokenTypes.ELIST, TokenTypes.FOR_INIT},
97          {TokenTypes.EXPR, TokenTypes.ELIST, TokenTypes.FOR_ITERATOR},
98          {TokenTypes.ANNOTATION_MEMBER_VALUE_PAIR}, {
99              TokenTypes.RESOURCE,
100             TokenTypes.RESOURCES,
101             TokenTypes.RESOURCE_SPECIFICATION,
102         },
103         {TokenTypes.EXPR, TokenTypes.LAMBDA},
104         {TokenTypes.EXPR, TokenTypes.SWITCH_RULE, TokenTypes.LITERAL_SWITCH, TokenTypes.SLIST},
105     };
106 
107     /**
108      * Allowed AST types from an assignment AST node
109      * towards the root.
110      */
111     private static final int[][] CONTROL_CONTEXT = {
112         {TokenTypes.EXPR, TokenTypes.LITERAL_DO},
113         {TokenTypes.EXPR, TokenTypes.LITERAL_FOR},
114         {TokenTypes.EXPR, TokenTypes.LITERAL_WHILE},
115         {TokenTypes.EXPR, TokenTypes.LITERAL_IF},
116         {TokenTypes.EXPR, TokenTypes.LITERAL_ELSE},
117     };
118 
119     /**
120      * Allowed AST types from a comparison node (above an assignment)
121      * towards the root.
122      */
123     private static final int[][] ALLOWED_ASSIGNMENT_IN_COMPARISON_CONTEXT = {
124         {TokenTypes.EXPR, TokenTypes.LITERAL_WHILE},
125         {TokenTypes.EXPR, TokenTypes.FOR_CONDITION},
126         {TokenTypes.EXPR, TokenTypes.LITERAL_DO},
127     };
128 
129     /**
130      * The token types that identify comparison operators.
131      */
132     private static final BitSet COMPARISON_TYPES = TokenUtil.asBitSet(
133         TokenTypes.EQUAL,
134         TokenTypes.GE,
135         TokenTypes.GT,
136         TokenTypes.LE,
137         TokenTypes.LT,
138         TokenTypes.NOT_EQUAL
139     );
140 
141     /**
142      * The token types that are ignored while checking "loop-idiom".
143      */
144     private static final BitSet LOOP_IDIOM_IGNORED_PARENTS = TokenUtil.asBitSet(
145         TokenTypes.LAND,
146         TokenTypes.LOR,
147         TokenTypes.LNOT,
148         TokenTypes.BOR,
149         TokenTypes.BAND
150     );
151 
152     @Override
153     public int[] getDefaultTokens() {
154         return getRequiredTokens();
155     }
156 
157     @Override
158     public int[] getAcceptableTokens() {
159         return getRequiredTokens();
160     }
161 
162     @Override
163     public int[] getRequiredTokens() {
164         return new int[] {
165             TokenTypes.ASSIGN,            // '='
166             TokenTypes.DIV_ASSIGN,        // "/="
167             TokenTypes.PLUS_ASSIGN,       // "+="
168             TokenTypes.MINUS_ASSIGN,      // "-="
169             TokenTypes.STAR_ASSIGN,       // "*="
170             TokenTypes.MOD_ASSIGN,        // "%="
171             TokenTypes.SR_ASSIGN,         // ">>="
172             TokenTypes.BSR_ASSIGN,        // ">>>="
173             TokenTypes.SL_ASSIGN,         // "<<="
174             TokenTypes.BXOR_ASSIGN,       // "^="
175             TokenTypes.BOR_ASSIGN,        // "|="
176             TokenTypes.BAND_ASSIGN,       // "&="
177         };
178     }
179 
180     @Override
181     public void visitToken(DetailAST ast) {
182         if (!isInContext(ast, ALLOWED_ASSIGNMENT_CONTEXT, CommonUtil.EMPTY_BIT_SET)
183                 && !isInNoBraceControlStatement(ast)
184                 && !isInLoopIdiom(ast)) {
185             log(ast, MSG_KEY);
186         }
187     }
188 
189     /**
190      * Determines if ast is in the body of a flow control statement without
191      * braces. An example of such a statement would be
192      * <pre>
193      * if (y &lt; 0)
194      *     x = y;
195      * </pre>
196      * <p>
197      * This leads to the following AST structure:
198      * </p>
199      * <pre>
200      * LITERAL_IF
201      *     LPAREN
202      *     EXPR // test
203      *     RPAREN
204      *     EXPR // body
205      *     SEMI
206      * </pre>
207      * <p>
208      * We need to ensure that ast is in the body and not in the test.
209      * </p>
210      *
211      * @param ast an assignment operator AST
212      * @return whether ast is in the body of a flow control statement
213      */
214     private static boolean isInNoBraceControlStatement(DetailAST ast) {
215         boolean result = false;
216         if (isInContext(ast, CONTROL_CONTEXT, CommonUtil.EMPTY_BIT_SET)) {
217             final DetailAST expr = ast.getParent();
218             final DetailAST exprNext = expr.getNextSibling();
219             result = exprNext.getType() == TokenTypes.SEMI;
220         }
221         return result;
222     }
223 
224     /**
225      * Tests whether the given AST is used in the "assignment in loop" idiom.
226      * <pre>
227      * String line;
228      * while ((line = bufferedReader.readLine()) != null) {
229      *   // process the line
230      * }
231      * for (;(line = bufferedReader.readLine()) != null;) {
232      *   // process the line
233      * }
234      * do {
235      *   // process the line
236      * }
237      * while ((line = bufferedReader.readLine()) != null);
238      * </pre>
239      * Assignment inside a condition is not a problem here, as the assignment is surrounded by an
240      * extra pair of parentheses. The comparison is {@code != null} and there is no chance that
241      * intention was to write {@code line == reader.readLine()}.
242      *
243      * @param ast assignment AST
244      * @return whether the context of the assignment AST indicates the idiom
245      */
246     private static boolean isInLoopIdiom(DetailAST ast) {
247         return isComparison(ast.getParent())
248                     && isInContext(ast.getParent(),
249                             ALLOWED_ASSIGNMENT_IN_COMPARISON_CONTEXT,
250                             LOOP_IDIOM_IGNORED_PARENTS);
251     }
252 
253     /**
254      * Checks if an AST is a comparison operator.
255      *
256      * @param ast the AST to check
257      * @return true iff ast is a comparison operator.
258      */
259     private static boolean isComparison(DetailAST ast) {
260         final int astType = ast.getType();
261         return COMPARISON_TYPES.get(astType);
262     }
263 
264     /**
265      * Tests whether the provided AST is in
266      * one of the given contexts.
267      *
268      * @param ast the AST from which to start walking towards root
269      * @param contextSet the contexts to test against.
270      * @param skipTokens parent token types to ignore
271      *
272      * @return whether the parents nodes of ast match one of the allowed type paths.
273      */
274     private static boolean isInContext(DetailAST ast, int[][] contextSet, BitSet skipTokens) {
275         boolean found = false;
276         for (int[] element : contextSet) {
277             DetailAST current = ast;
278             for (int anElement : element) {
279                 current = getParent(current, skipTokens);
280                 if (current.getType() == anElement) {
281                     found = true;
282                 }
283                 else {
284                     found = false;
285                     break;
286                 }
287             }
288 
289             if (found) {
290                 break;
291             }
292         }
293         return found;
294     }
295 
296     /**
297      * Get ast parent, ignoring token types from {@code skipTokens}.
298      *
299      * @param ast token to get parent
300      * @param skipTokens token types to skip
301      * @return first not ignored parent of ast
302      */
303     private static DetailAST getParent(DetailAST ast, BitSet skipTokens) {
304         DetailAST result = ast.getParent();
305         while (skipTokens.get(result.getType())) {
306             result = result.getParent();
307         }
308         return result;
309     }
310 
311 }