001//////////////////////////////////////////////////////////////////////////////// 002// checkstyle: Checks Java source code for adherence to a set of rules. 003// Copyright (C) 2001-2020 the original author or authors. 004// 005// This library is free software; you can redistribute it and/or 006// modify it under the terms of the GNU Lesser General Public 007// License as published by the Free Software Foundation; either 008// version 2.1 of the License, or (at your option) any later version. 009// 010// This library is distributed in the hope that it will be useful, 011// but WITHOUT ANY WARRANTY; without even the implied warranty of 012// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU 013// Lesser General Public License for more details. 014// 015// You should have received a copy of the GNU Lesser General Public 016// License along with this library; if not, write to the Free Software 017// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA 018//////////////////////////////////////////////////////////////////////////////// 019 020package com.puppycrawl.tools.checkstyle.checks.coding; 021 022import java.util.Arrays; 023 024import com.puppycrawl.tools.checkstyle.StatelessCheck; 025import com.puppycrawl.tools.checkstyle.api.AbstractCheck; 026import com.puppycrawl.tools.checkstyle.api.DetailAST; 027import com.puppycrawl.tools.checkstyle.api.TokenTypes; 028 029/** 030 * <p> 031 * Checks for assignments in subexpressions, such as in 032 * {@code String s = Integer.toString(i = 2);}. 033 * </p> 034 * <p> 035 * Rationale: With the exception of {@code for} iterators and assignment in {@code while} idiom, 036 * all assignments should occur in their own top-level statement to increase readability. 037 * With inner assignments like the one given above, it is difficult to see all places 038 * where a variable is set. 039 * </p> 040 * <p> 041 * Note: Check allows usage of the popular assignment in {@code while} idiom: 042 * </p> 043 * <pre> 044 * String line; 045 * while ((line = bufferedReader.readLine()) != null) { 046 * // process the line 047 * } 048 * </pre> 049 * <p> 050 * Assignment inside a condition is not a problem here, as the assignment is surrounded 051 * by an extra pair of parentheses. The comparison is {@code != null} and there is no chance that 052 * intention was to write {@code line == reader.readLine()}. 053 * </p> 054 * <p> 055 * To configure the check: 056 * </p> 057 * <pre> 058 * <module name="InnerAssignment"/> 059 * </pre> 060 * <p>Example:</p> 061 * <pre> 062 * class MyClass { 063 * 064 * void foo() { 065 * int a, b; 066 * a = b = 5; // violation, assignment to each variable should be in a separate statement 067 * a = b += 5; // violation 068 * 069 * a = 5; // OK 070 * b = 5; // OK 071 * a = 5; b = 5; // OK 072 * 073 * double myDouble; 074 * double[] doubleArray = new double[] {myDouble = 4.5, 15.5}; // violation 075 * 076 * String nameOne; 077 * List<String> myList = new ArrayList<String>(); 078 * myList.add(nameOne = "tom"); // violation 079 * for (int k = 0; k < 10; k = k + 2) { // OK 080 * // some code 081 * } 082 * 083 * boolean someVal; 084 * if (someVal = true) { // violation 085 * // some code 086 * } 087 * 088 * while (someVal = false) {} // violation 089 * 090 * InputStream is = new FileInputStream("textFile.txt"); 091 * while ((b = is.read()) != -1) { // OK, this is a common idiom 092 * // some code 093 * } 094 * 095 * } 096 * 097 * boolean testMethod() { 098 * boolean val; 099 * return val = true; // violation 100 * } 101 * } 102 * </pre> 103 * <p> 104 * Parent is {@code com.puppycrawl.tools.checkstyle.TreeWalker} 105 * </p> 106 * <p> 107 * Violation Message Keys: 108 * </p> 109 * <ul> 110 * <li> 111 * {@code assignment.inner.avoid} 112 * </li> 113 * </ul> 114 * 115 * @since 3.0 116 */ 117@StatelessCheck 118public class InnerAssignmentCheck 119 extends AbstractCheck { 120 121 /** 122 * A key is pointing to the warning message text in "messages.properties" 123 * file. 124 */ 125 public static final String MSG_KEY = "assignment.inner.avoid"; 126 127 /** 128 * List of allowed AST types from an assignment AST node 129 * towards the root. 130 */ 131 private static final int[][] ALLOWED_ASSIGNMENT_CONTEXT = { 132 {TokenTypes.EXPR, TokenTypes.SLIST}, 133 {TokenTypes.VARIABLE_DEF}, 134 {TokenTypes.EXPR, TokenTypes.ELIST, TokenTypes.FOR_INIT}, 135 {TokenTypes.EXPR, TokenTypes.ELIST, TokenTypes.FOR_ITERATOR}, 136 {TokenTypes.ANNOTATION_MEMBER_VALUE_PAIR}, { 137 TokenTypes.RESOURCE, 138 TokenTypes.RESOURCES, 139 TokenTypes.RESOURCE_SPECIFICATION, 140 }, 141 {TokenTypes.EXPR, TokenTypes.LAMBDA}, 142 }; 143 144 /** 145 * List of allowed AST types from an assignment AST node 146 * towards the root. 147 */ 148 private static final int[][] CONTROL_CONTEXT = { 149 {TokenTypes.EXPR, TokenTypes.LITERAL_DO}, 150 {TokenTypes.EXPR, TokenTypes.LITERAL_FOR}, 151 {TokenTypes.EXPR, TokenTypes.LITERAL_WHILE}, 152 {TokenTypes.EXPR, TokenTypes.LITERAL_IF}, 153 {TokenTypes.EXPR, TokenTypes.LITERAL_ELSE}, 154 }; 155 156 /** 157 * List of allowed AST types from a comparison node (above an assignment) 158 * towards the root. 159 */ 160 private static final int[][] ALLOWED_ASSIGNMENT_IN_COMPARISON_CONTEXT = { 161 {TokenTypes.EXPR, TokenTypes.LITERAL_WHILE, }, 162 }; 163 164 /** 165 * The token types that identify comparison operators. 166 */ 167 private static final int[] COMPARISON_TYPES = { 168 TokenTypes.EQUAL, 169 TokenTypes.GE, 170 TokenTypes.GT, 171 TokenTypes.LE, 172 TokenTypes.LT, 173 TokenTypes.NOT_EQUAL, 174 }; 175 176 /** 177 * The token types that are ignored while checking "while-idiom". 178 */ 179 private static final int[] WHILE_IDIOM_IGNORED_PARENTS = { 180 TokenTypes.LAND, 181 TokenTypes.LOR, 182 TokenTypes.LNOT, 183 TokenTypes.BOR, 184 TokenTypes.BAND, 185 }; 186 187 static { 188 Arrays.sort(COMPARISON_TYPES); 189 Arrays.sort(WHILE_IDIOM_IGNORED_PARENTS); 190 } 191 192 @Override 193 public int[] getDefaultTokens() { 194 return getRequiredTokens(); 195 } 196 197 @Override 198 public int[] getAcceptableTokens() { 199 return getRequiredTokens(); 200 } 201 202 @Override 203 public int[] getRequiredTokens() { 204 return new int[] { 205 TokenTypes.ASSIGN, // '=' 206 TokenTypes.DIV_ASSIGN, // "/=" 207 TokenTypes.PLUS_ASSIGN, // "+=" 208 TokenTypes.MINUS_ASSIGN, // "-=" 209 TokenTypes.STAR_ASSIGN, // "*=" 210 TokenTypes.MOD_ASSIGN, // "%=" 211 TokenTypes.SR_ASSIGN, // ">>=" 212 TokenTypes.BSR_ASSIGN, // ">>>=" 213 TokenTypes.SL_ASSIGN, // "<<=" 214 TokenTypes.BXOR_ASSIGN, // "^=" 215 TokenTypes.BOR_ASSIGN, // "|=" 216 TokenTypes.BAND_ASSIGN, // "&=" 217 }; 218 } 219 220 @Override 221 public void visitToken(DetailAST ast) { 222 if (!isInContext(ast, ALLOWED_ASSIGNMENT_CONTEXT) 223 && !isInNoBraceControlStatement(ast) 224 && !isInWhileIdiom(ast)) { 225 log(ast, MSG_KEY); 226 } 227 } 228 229 /** 230 * Determines if ast is in the body of a flow control statement without 231 * braces. An example of such a statement would be 232 * <p> 233 * <pre> 234 * if (y < 0) 235 * x = y; 236 * </pre> 237 * </p> 238 * <p> 239 * This leads to the following AST structure: 240 * </p> 241 * <p> 242 * <pre> 243 * LITERAL_IF 244 * LPAREN 245 * EXPR // test 246 * RPAREN 247 * EXPR // body 248 * SEMI 249 * </pre> 250 * </p> 251 * <p> 252 * We need to ensure that ast is in the body and not in the test. 253 * </p> 254 * 255 * @param ast an assignment operator AST 256 * @return whether ast is in the body of a flow control statement 257 */ 258 private static boolean isInNoBraceControlStatement(DetailAST ast) { 259 boolean result = false; 260 if (isInContext(ast, CONTROL_CONTEXT)) { 261 final DetailAST expr = ast.getParent(); 262 final DetailAST exprNext = expr.getNextSibling(); 263 result = exprNext.getType() == TokenTypes.SEMI; 264 } 265 return result; 266 } 267 268 /** 269 * Tests whether the given AST is used in the "assignment in while" idiom. 270 * <pre> 271 * String line; 272 * while ((line = bufferedReader.readLine()) != null) { 273 * // process the line 274 * } 275 * </pre> 276 * Assignment inside a condition is not a problem here, as the assignment is surrounded by an 277 * extra pair of parentheses. The comparison is {@code != null} and there is no chance that 278 * intention was to write {@code line == reader.readLine()}. 279 * 280 * @param ast assignment AST 281 * @return whether the context of the assignment AST indicates the idiom 282 */ 283 private static boolean isInWhileIdiom(DetailAST ast) { 284 boolean result = false; 285 if (isComparison(ast.getParent())) { 286 result = isInContext(ast.getParent(), 287 ALLOWED_ASSIGNMENT_IN_COMPARISON_CONTEXT, 288 WHILE_IDIOM_IGNORED_PARENTS 289 ); 290 } 291 return result; 292 } 293 294 /** 295 * Checks if an AST is a comparison operator. 296 * 297 * @param ast the AST to check 298 * @return true iff ast is a comparison operator. 299 */ 300 private static boolean isComparison(DetailAST ast) { 301 final int astType = ast.getType(); 302 return Arrays.binarySearch(COMPARISON_TYPES, astType) >= 0; 303 } 304 305 /** 306 * Tests whether the provided AST is in 307 * one of the given contexts. 308 * 309 * @param ast the AST from which to start walking towards root 310 * @param contextSet the contexts to test against. 311 * @param skipTokens parent token types to ignore 312 * 313 * @return whether the parents nodes of ast match one of the allowed type paths. 314 */ 315 private static boolean isInContext(DetailAST ast, int[][] contextSet, int... skipTokens) { 316 boolean found = false; 317 for (int[] element : contextSet) { 318 DetailAST current = ast; 319 for (int anElement : element) { 320 current = getParent(current, skipTokens); 321 if (current.getType() == anElement) { 322 found = true; 323 } 324 else { 325 found = false; 326 break; 327 } 328 } 329 330 if (found) { 331 break; 332 } 333 } 334 return found; 335 } 336 337 /** 338 * Get ast parent, ignoring token types from {@code skipTokens}. 339 * 340 * @param ast token to get parent 341 * @param skipTokens token types to skip 342 * @return first not ignored parent of ast 343 */ 344 private static DetailAST getParent(DetailAST ast, int... skipTokens) { 345 DetailAST result = ast.getParent(); 346 while (Arrays.binarySearch(skipTokens, result.getType()) > -1) { 347 result = result.getParent(); 348 } 349 return result; 350 } 351 352}