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