Thu, 07 Mar 2013 10:04:28 +0000
8009138: javac, equals-hashCode warning tuning
Reviewed-by: mcimadamore
strarup@1594 | 1 | /* |
strarup@1594 | 2 | * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. |
strarup@1594 | 3 | * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. |
strarup@1594 | 4 | * |
strarup@1594 | 5 | * This code is free software; you can redistribute it and/or modify it |
strarup@1594 | 6 | * under the terms of the GNU General Public License version 2 only, as |
strarup@1594 | 7 | * published by the Free Software Foundation. |
strarup@1594 | 8 | * |
strarup@1594 | 9 | * This code is distributed in the hope that it will be useful, but WITHOUT |
strarup@1594 | 10 | * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or |
strarup@1594 | 11 | * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License |
strarup@1594 | 12 | * version 2 for more details (a copy is included in the LICENSE file that |
strarup@1594 | 13 | * accompanied this code). |
strarup@1594 | 14 | * |
strarup@1594 | 15 | * You should have received a copy of the GNU General Public License version |
strarup@1594 | 16 | * 2 along with this work; if not, write to the Free Software Foundation, |
strarup@1594 | 17 | * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. |
strarup@1594 | 18 | * |
strarup@1594 | 19 | * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA |
strarup@1594 | 20 | * or visit www.oracle.com if you need additional information or have any |
strarup@1594 | 21 | * questions. |
strarup@1594 | 22 | */ |
strarup@1594 | 23 | |
strarup@1594 | 24 | import com.sun.tools.classfile.*; |
strarup@1594 | 25 | import java.io.*; |
strarup@1594 | 26 | import javax.lang.model.element.*; |
strarup@1594 | 27 | import java.util.*; |
strarup@1594 | 28 | |
strarup@1594 | 29 | /** |
strarup@1594 | 30 | * The {@code ClassFileVisitor} reads a class file using the |
strarup@1594 | 31 | * {@code com.sun.tools.classfile} library. It iterates over the methods |
strarup@1594 | 32 | * in a class, and checks MethodParameters attributes against JLS |
strarup@1594 | 33 | * requirements, as well as assumptions about the javac implementations. |
strarup@1594 | 34 | * <p> |
strarup@1594 | 35 | * It enforces the following rules: |
strarup@1594 | 36 | * <ul> |
strarup@1594 | 37 | * <li>All non-synthetic methods with arguments must have the |
strarup@1594 | 38 | * MethodParameters attribute. </li> |
strarup@1594 | 39 | * <li>At most one MethodParameters attribute per method.</li> |
strarup@1594 | 40 | * <li>An empty MethodParameters attribute is not allowed (i.e. no |
strarup@1594 | 41 | * attribute for methods taking no parameters).</li> |
strarup@1594 | 42 | * <li>The number of recorded parameter names much equal the number |
strarup@1594 | 43 | * of parameters, including any implicit or synthetic parameters generated |
strarup@1594 | 44 | * by the compiler.</li> |
strarup@1594 | 45 | * <li>Although the spec allow recording parameters with no name, the javac |
strarup@1594 | 46 | * implementation is assumed to record a name for all parameters. That is, |
strarup@1594 | 47 | * the Methodparameters attribute must record a non-zero, valid constant |
strarup@1594 | 48 | * pool index for each parameter.</li> |
strarup@1594 | 49 | * <li>Check presence, expected names (e.g. this$N, $enum$name, ...) and flags |
strarup@1594 | 50 | * (e.g. ACC_SYNTHETIC, ACC_MANDATED) for compiler generated parameters.</li> |
strarup@1594 | 51 | * <li>Names of explicit parameters must reflect the names in the Java source. |
strarup@1594 | 52 | * This is checked by assuming a design pattern where any name is permitted |
strarup@1594 | 53 | * for the first explicit parameter. For subsequent parameters the following |
strarup@1594 | 54 | * rule is checked: <i>param[n] == ++param[n-1].charAt(0) + param[n-1]</i> |
strarup@1594 | 55 | * </ul> |
strarup@1594 | 56 | */ |
strarup@1594 | 57 | class ClassFileVisitor extends Tester.Visitor { |
strarup@1594 | 58 | |
strarup@1594 | 59 | Tester tester; |
strarup@1594 | 60 | |
strarup@1594 | 61 | public String cname; |
strarup@1594 | 62 | public boolean isEnum; |
strarup@1594 | 63 | public boolean isInterface; |
strarup@1594 | 64 | public boolean isInner; |
strarup@1594 | 65 | public boolean isPublic; |
strarup@1594 | 66 | public boolean isStatic; |
strarup@1594 | 67 | public boolean isAnon; |
strarup@1594 | 68 | public ClassFile classFile; |
strarup@1594 | 69 | |
strarup@1594 | 70 | |
strarup@1594 | 71 | public ClassFileVisitor(Tester tester) { |
strarup@1594 | 72 | super(tester); |
strarup@1594 | 73 | } |
strarup@1594 | 74 | |
strarup@1594 | 75 | public void error(String msg) { |
strarup@1594 | 76 | super.error("classfile: " + msg); |
strarup@1594 | 77 | } |
strarup@1594 | 78 | |
strarup@1594 | 79 | public void warn(String msg) { |
strarup@1594 | 80 | super.warn("classfile: " + msg); |
strarup@1594 | 81 | } |
strarup@1594 | 82 | |
strarup@1594 | 83 | /** |
strarup@1594 | 84 | * Read the class and determine some key characteristics, like if it's |
strarup@1594 | 85 | * an enum, or inner class, etc. |
strarup@1594 | 86 | */ |
strarup@1594 | 87 | void visitClass(final String cname, final File cfile, final StringBuilder sb) |
strarup@1594 | 88 | throws Exception { |
strarup@1594 | 89 | this.cname = cname; |
strarup@1594 | 90 | classFile = ClassFile.read(cfile); |
strarup@1594 | 91 | isEnum = classFile.access_flags.is(AccessFlags.ACC_ENUM); |
strarup@1594 | 92 | isInterface = classFile.access_flags.is(AccessFlags.ACC_INTERFACE); |
strarup@1594 | 93 | isPublic = classFile.access_flags.is(AccessFlags.ACC_PUBLIC); |
strarup@1594 | 94 | isInner = false; |
strarup@1594 | 95 | isStatic = true; |
strarup@1594 | 96 | isAnon = false; |
strarup@1594 | 97 | |
strarup@1594 | 98 | Attribute attr = classFile.getAttribute("InnerClasses"); |
strarup@1594 | 99 | if (attr != null) attr.accept(new InnerClassVisitor(), null); |
strarup@1594 | 100 | isAnon = isInner & isAnon; |
strarup@1594 | 101 | |
strarup@1594 | 102 | sb.append(isStatic ? "static " : "") |
strarup@1594 | 103 | .append(isPublic ? "public " : "") |
strarup@1594 | 104 | .append(isEnum ? "enum " : isInterface ? "interface " : "class ") |
strarup@1594 | 105 | .append(cname).append(" -- ") |
strarup@1594 | 106 | .append(isInner? "inner " : "" ) |
strarup@1594 | 107 | .append(isAnon ? "anon" : "") |
strarup@1594 | 108 | .append("\n");; |
strarup@1594 | 109 | |
strarup@1594 | 110 | for (Method method : classFile.methods) { |
strarup@1594 | 111 | new MethodVisitor().visitMethod(method, sb); |
strarup@1594 | 112 | } |
strarup@1594 | 113 | } |
strarup@1594 | 114 | |
strarup@1594 | 115 | /** |
strarup@1594 | 116 | * Used to visit InnerClasses_attribute of a class, |
strarup@1594 | 117 | * to determne if this class is an local class, and anonymous |
strarup@1594 | 118 | * inner class or a none-static member class. These types of |
strarup@1594 | 119 | * classes all have an containing class instances field that |
strarup@1594 | 120 | * requires an implicit or synthetic constructor argument. |
strarup@1594 | 121 | */ |
strarup@1594 | 122 | class InnerClassVisitor extends AttributeVisitor<Void, Void> { |
strarup@1594 | 123 | public Void visitInnerClasses(InnerClasses_attribute iattr, Void v) { |
strarup@1594 | 124 | try{ |
strarup@1594 | 125 | for (InnerClasses_attribute.Info info : iattr.classes) { |
strarup@1594 | 126 | if (info.getInnerClassInfo(classFile.constant_pool) == null) continue; |
strarup@1594 | 127 | String in = info.getInnerClassInfo(classFile.constant_pool).getName(); |
strarup@1594 | 128 | if (in == null || !cname.equals(in)) continue; |
strarup@1594 | 129 | isInner = true; |
strarup@1594 | 130 | isAnon = null == info.getInnerName(classFile.constant_pool); |
strarup@1594 | 131 | isStatic = info.inner_class_access_flags.is(AccessFlags.ACC_STATIC); |
strarup@1594 | 132 | break; |
strarup@1594 | 133 | } |
strarup@1594 | 134 | } catch(Exception e) { |
strarup@1594 | 135 | throw new IllegalStateException(e); |
strarup@1594 | 136 | } |
strarup@1594 | 137 | return null; |
strarup@1594 | 138 | } |
strarup@1594 | 139 | } |
strarup@1594 | 140 | |
strarup@1594 | 141 | /** |
strarup@1594 | 142 | * Check the MethodParameters attribute of a method. |
strarup@1594 | 143 | */ |
strarup@1594 | 144 | class MethodVisitor extends AttributeVisitor<Void, StringBuilder> { |
strarup@1594 | 145 | |
strarup@1594 | 146 | public String mName; |
strarup@1594 | 147 | public Descriptor mDesc; |
strarup@1594 | 148 | public int mParams; |
strarup@1594 | 149 | public int mAttrs; |
strarup@1594 | 150 | public int mNumParams; |
strarup@1594 | 151 | public boolean mSynthetic; |
strarup@1594 | 152 | public boolean mIsConstructor; |
strarup@1594 | 153 | public String prefix; |
strarup@1594 | 154 | |
strarup@1594 | 155 | void visitMethod(Method method, StringBuilder sb) throws Exception { |
strarup@1594 | 156 | |
strarup@1594 | 157 | mName = method.getName(classFile.constant_pool); |
strarup@1594 | 158 | mDesc = method.descriptor; |
strarup@1594 | 159 | mParams = mDesc.getParameterCount(classFile.constant_pool); |
strarup@1594 | 160 | mAttrs = method.attributes.attrs.length; |
strarup@1594 | 161 | mNumParams = -1; // no MethodParameters attribute found |
strarup@1594 | 162 | mSynthetic = method.access_flags.is(AccessFlags.ACC_SYNTHETIC); |
strarup@1594 | 163 | mIsConstructor = mName.equals("<init>"); |
strarup@1594 | 164 | prefix = cname + "." + mName + "() - "; |
strarup@1594 | 165 | |
strarup@1594 | 166 | sb.append(cname).append(".").append(mName).append("("); |
strarup@1594 | 167 | |
strarup@1594 | 168 | for (Attribute a : method.attributes) { |
strarup@1594 | 169 | a.accept(this, sb); |
strarup@1594 | 170 | } |
strarup@1594 | 171 | if (mNumParams == -1) { |
strarup@1594 | 172 | if (mSynthetic) { |
strarup@1594 | 173 | sb.append("<none>)!!"); |
strarup@1594 | 174 | } else { |
strarup@1594 | 175 | sb.append("<none>)"); |
strarup@1594 | 176 | } |
strarup@1594 | 177 | } |
strarup@1594 | 178 | sb.append("\n"); |
strarup@1594 | 179 | |
strarup@1594 | 180 | // IMPL: methods with arguments must have a MethodParameters |
strarup@1594 | 181 | // attribute, except possibly some synthetic methods. |
strarup@1594 | 182 | if (mNumParams == -1 && mParams > 0 && ! mSynthetic) { |
strarup@1594 | 183 | error(prefix + "missing MethodParameters attribute"); |
strarup@1594 | 184 | } |
strarup@1594 | 185 | } |
strarup@1594 | 186 | |
strarup@1594 | 187 | public Void visitMethodParameters(MethodParameters_attribute mp, |
strarup@1594 | 188 | StringBuilder sb) { |
strarup@1594 | 189 | |
strarup@1594 | 190 | // SPEC: At most one MethodParameters attribute allowed |
strarup@1594 | 191 | if (mNumParams != -1) { |
strarup@1594 | 192 | error(prefix + "Multiple MethodParameters attributes"); |
strarup@1594 | 193 | return null; |
strarup@1594 | 194 | } |
strarup@1594 | 195 | |
strarup@1594 | 196 | mNumParams = mp.method_parameter_table_length; |
strarup@1594 | 197 | |
strarup@1594 | 198 | // SPEC: An empty attribute is not allowed! |
strarup@1594 | 199 | if (mNumParams == 0) { |
strarup@1594 | 200 | error(prefix + "0 length MethodParameters attribute"); |
strarup@1594 | 201 | return null; |
strarup@1594 | 202 | } |
strarup@1594 | 203 | |
strarup@1594 | 204 | // SPEC: one name per parameter. |
strarup@1594 | 205 | if (mNumParams != mParams) { |
strarup@1594 | 206 | error(prefix + "found " + mNumParams + |
strarup@1594 | 207 | " parameters, expected " + mParams); |
strarup@1594 | 208 | return null; |
strarup@1594 | 209 | } |
strarup@1594 | 210 | |
strarup@1594 | 211 | // IMPL: Whether MethodParameters attributes will be generated |
strarup@1594 | 212 | // for some synthetics is unresolved. For now, assume no. |
strarup@1594 | 213 | if (mSynthetic) { |
strarup@1594 | 214 | warn(prefix + "synthetic has MethodParameter attribute"); |
strarup@1594 | 215 | } |
strarup@1594 | 216 | |
strarup@1594 | 217 | String sep = ""; |
strarup@1594 | 218 | String userParam = null; |
strarup@1594 | 219 | for (int x = 0; x < mNumParams; x++) { |
strarup@1594 | 220 | |
strarup@1594 | 221 | // IMPL: Assume all parameters are named, something. |
strarup@1594 | 222 | int cpi = mp.method_parameter_table[x].name_index; |
strarup@1594 | 223 | if (cpi == 0) { |
strarup@1594 | 224 | error(prefix + "name expected, param[" + x + "]"); |
strarup@1594 | 225 | return null; |
strarup@1594 | 226 | } |
strarup@1594 | 227 | |
strarup@1594 | 228 | // SPEC: a non 0 index, must be valid! |
strarup@1594 | 229 | String param = null; |
strarup@1594 | 230 | try { |
strarup@1594 | 231 | param = classFile.constant_pool.getUTF8Value(cpi); |
strarup@1594 | 232 | sb.append(sep).append(param); |
strarup@1594 | 233 | sep = ", "; |
strarup@1594 | 234 | } catch(ConstantPoolException e) { |
strarup@1594 | 235 | error(prefix + "invalid index " + cpi + " for param[" |
strarup@1594 | 236 | + x + "]"); |
strarup@1594 | 237 | return null; |
strarup@1594 | 238 | } |
strarup@1594 | 239 | |
strarup@1594 | 240 | |
strarup@1594 | 241 | // Check availability, flags and special names |
strarup@1594 | 242 | int check = checkParam(mp, param, x, sb); |
strarup@1594 | 243 | if (check < 0) { |
strarup@1594 | 244 | return null; |
strarup@1594 | 245 | } |
strarup@1594 | 246 | |
strarup@1594 | 247 | // TEST: check test assumptions about parameter name. |
strarup@1594 | 248 | // Expected names are calculated starting with the |
strarup@1594 | 249 | // 2nd explicit (user given) parameter. |
strarup@1594 | 250 | // param[n] == ++param[n-1].charAt(0) + param[n-1] |
strarup@1594 | 251 | String expect = null; |
strarup@1594 | 252 | if (userParam != null) { |
strarup@1594 | 253 | char c = userParam.charAt(0); |
strarup@1594 | 254 | expect = (++c) + userParam; |
strarup@1594 | 255 | } |
strarup@1594 | 256 | if (check > 0) { |
strarup@1594 | 257 | userParam = param; |
strarup@1594 | 258 | } |
strarup@1594 | 259 | if (expect != null && !param.equals(expect)) { |
strarup@1594 | 260 | error(prefix + "param[" + x + "]='" |
strarup@1594 | 261 | + param + "' expected '" + expect + "'"); |
strarup@1594 | 262 | return null; |
strarup@1594 | 263 | } |
strarup@1594 | 264 | } |
strarup@1594 | 265 | if (mSynthetic) { |
strarup@1594 | 266 | sb.append(")!!"); |
strarup@1594 | 267 | } else { |
strarup@1594 | 268 | sb.append(")"); |
strarup@1594 | 269 | } |
strarup@1594 | 270 | return null; |
strarup@1594 | 271 | } |
strarup@1594 | 272 | |
strarup@1594 | 273 | /* |
strarup@1594 | 274 | * Check a parameter for conformity to JLS and javac specific |
strarup@1594 | 275 | * assumptions. |
strarup@1594 | 276 | * Return -1, if an error is detected. Otherwise, return 0, if |
strarup@1594 | 277 | * the parameter is compiler generated, or 1 for an (presumably) |
strarup@1594 | 278 | * explicitly declared parameter. |
strarup@1594 | 279 | */ |
strarup@1594 | 280 | int checkParam(MethodParameters_attribute mp, String param, int index, |
strarup@1594 | 281 | StringBuilder sb) { |
strarup@1594 | 282 | |
strarup@1594 | 283 | boolean synthetic = (mp.method_parameter_table[index].flags |
strarup@1594 | 284 | & AccessFlags.ACC_SYNTHETIC) != 0; |
strarup@1594 | 285 | boolean mandated = (mp.method_parameter_table[index].flags |
strarup@1594 | 286 | & AccessFlags.ACC_MANDATED) != 0; |
strarup@1594 | 287 | |
strarup@1594 | 288 | // Setup expectations for flags and special names |
strarup@1594 | 289 | String expect = null; |
strarup@1594 | 290 | boolean allowMandated = false; |
strarup@1594 | 291 | boolean allowSynthetic = false; |
strarup@1594 | 292 | if (mSynthetic || synthetic) { |
strarup@1594 | 293 | // not an implementation gurantee, but okay for now |
strarup@1594 | 294 | expect = "arg" + index; // default |
strarup@1594 | 295 | } |
strarup@1594 | 296 | if (mIsConstructor) { |
strarup@1594 | 297 | if (isEnum) { |
strarup@1594 | 298 | if (index == 0) { |
strarup@1594 | 299 | expect = "\\$enum\\$name"; |
strarup@1594 | 300 | allowSynthetic = true; |
strarup@1594 | 301 | } else if(index == 1) { |
strarup@1594 | 302 | expect = "\\$enum\\$ordinal"; |
strarup@1594 | 303 | allowSynthetic = true; |
strarup@1594 | 304 | } |
strarup@1594 | 305 | } else if (index == 0) { |
strarup@1594 | 306 | if (isAnon) { |
strarup@1594 | 307 | allowMandated = true; |
strarup@1594 | 308 | expect = "this\\$[0-n]*"; |
strarup@1594 | 309 | } else if (isInner && !isStatic) { |
strarup@1594 | 310 | allowMandated = true; |
strarup@1594 | 311 | if (!isPublic) { |
strarup@1594 | 312 | // some but not all non-public inner classes |
strarup@1594 | 313 | // have synthetic argument. For now we give |
strarup@1594 | 314 | // the test a bit of slack and allow either. |
strarup@1594 | 315 | allowSynthetic = true; |
strarup@1594 | 316 | } |
strarup@1594 | 317 | expect = "this\\$[0-n]*"; |
strarup@1594 | 318 | } |
strarup@1594 | 319 | } else if (isAnon) { |
strarup@1594 | 320 | // not an implementation gurantee, but okay for now |
strarup@1594 | 321 | expect = "x[0-n]*"; |
strarup@1594 | 322 | } |
strarup@1594 | 323 | } else if (isEnum && mNumParams == 1 && index == 0 && mName.equals("valueOf")) { |
strarup@1594 | 324 | expect = "name"; |
strarup@1594 | 325 | allowMandated = true; |
strarup@1594 | 326 | } |
strarup@1594 | 327 | if (mandated) sb.append("!"); |
strarup@1594 | 328 | if (synthetic) sb.append("!!"); |
strarup@1594 | 329 | |
strarup@1594 | 330 | // IMPL: our rules a somewhat fuzzy, sometimes allowing both mandated |
strarup@1594 | 331 | // and synthetic. However, a parameters cannot be both. |
strarup@1594 | 332 | if (mandated && synthetic) { |
strarup@1594 | 333 | error(prefix + "param[" + index + "] == \"" + param |
strarup@1594 | 334 | + "\" ACC_SYNTHETIC and ACC_MANDATED"); |
strarup@1594 | 335 | return -1; |
strarup@1594 | 336 | } |
strarup@1594 | 337 | // ... but must be either, if both "allowed". |
strarup@1594 | 338 | if (!(mandated || synthetic) && allowMandated && allowSynthetic) { |
strarup@1594 | 339 | error(prefix + "param[" + index + "] == \"" + param |
strarup@1594 | 340 | + "\" expected ACC_MANDATED or ACC_SYNTHETIC"); |
strarup@1594 | 341 | return -1; |
strarup@1594 | 342 | } |
strarup@1594 | 343 | |
strarup@1594 | 344 | // ... if only one is "allowed", we meant "required". |
strarup@1594 | 345 | if (!mandated && allowMandated && !allowSynthetic) { |
strarup@1594 | 346 | error(prefix + "param[" + index + "] == \"" + param |
strarup@1594 | 347 | + "\" expected ACC_MANDATED"); |
strarup@1594 | 348 | return -1; |
strarup@1594 | 349 | } |
strarup@1594 | 350 | if (!synthetic && !allowMandated && allowSynthetic) { |
strarup@1594 | 351 | error(prefix + "param[" + index + "] == \"" + param |
strarup@1594 | 352 | + "\" expected ACC_SYNTHETIC"); |
strarup@1594 | 353 | return -1; |
strarup@1594 | 354 | } |
strarup@1594 | 355 | |
strarup@1594 | 356 | // ... and not "allowed", means prohibited. |
strarup@1594 | 357 | if (mandated && !allowMandated) { |
strarup@1594 | 358 | error(prefix + "param[" + index + "] == \"" + param |
strarup@1594 | 359 | + "\" unexpected, is ACC_MANDATED"); |
strarup@1594 | 360 | return -1; |
strarup@1594 | 361 | } |
strarup@1594 | 362 | if (synthetic && !allowSynthetic) { |
strarup@1594 | 363 | error(prefix + "param[" + index + "] == \"" + param |
strarup@1594 | 364 | + "\" unexpected, is ACC_SYNTHETIC"); |
strarup@1594 | 365 | return -1; |
strarup@1594 | 366 | } |
strarup@1594 | 367 | |
strarup@1594 | 368 | // Test special name expectations |
strarup@1594 | 369 | if (expect != null) { |
strarup@1594 | 370 | if (param.matches(expect)) { |
strarup@1594 | 371 | return 0; |
strarup@1594 | 372 | } |
strarup@1594 | 373 | error(prefix + "param[" + index + "]='" + param + |
strarup@1594 | 374 | "' expected '" + expect + "'"); |
strarup@1594 | 375 | return -1; |
strarup@1594 | 376 | } |
strarup@1594 | 377 | |
strarup@1594 | 378 | // No further checking for synthetic methods. |
strarup@1594 | 379 | if (mSynthetic) { |
strarup@1594 | 380 | return 0; |
strarup@1594 | 381 | } |
strarup@1594 | 382 | |
strarup@1594 | 383 | // Otherwise, do check test parameter naming convention. |
strarup@1594 | 384 | return 1; |
strarup@1594 | 385 | } |
strarup@1594 | 386 | } |
strarup@1594 | 387 | } |