7023233: False positive for -Xlint:try with nested try with resources blocks

Thu, 03 Mar 2011 09:43:24 +0000

author
mcimadamore
date
Thu, 03 Mar 2011 09:43:24 +0000
changeset 905
e9b8fbb30f5a
parent 904
4baab658f357
child 906
c15d788cb381

7023233: False positive for -Xlint:try with nested try with resources blocks
Summary: Wrong lint warning issued about unused resource when nested try-with-resource blocks are found
Reviewed-by: jjg

src/share/classes/com/sun/tools/javac/comp/Flow.java file | annotate | diff | comparison | revisions
test/tools/javac/TryWithResources/UnusedResourcesTest.java file | annotate | diff | comparison | revisions
     1.1 --- a/src/share/classes/com/sun/tools/javac/comp/Flow.java	Wed Mar 02 21:13:55 2011 -0800
     1.2 +++ b/src/share/classes/com/sun/tools/javac/comp/Flow.java	Thu Mar 03 09:43:24 2011 +0000
     1.3 @@ -272,7 +272,7 @@
     1.4  
     1.5      /** The list of unreferenced automatic resources.
     1.6       */
     1.7 -    Map<VarSymbol, JCVariableDecl> unrefdResources;
     1.8 +    Scope unrefdResources;
     1.9  
    1.10      /** Set when processing a loop body the second time for DU analysis. */
    1.11      boolean loopPassTwo = false;
    1.12 @@ -992,7 +992,6 @@
    1.13      public void visitTry(JCTry tree) {
    1.14          List<Type> caughtPrev = caught;
    1.15          List<Type> thrownPrev = thrown;
    1.16 -        Map<VarSymbol, JCVariableDecl> unrefdResourcesPrev = unrefdResources;
    1.17          thrown = List.nil();
    1.18          for (List<JCCatch> l = tree.catchers; l.nonEmpty(); l = l.tail) {
    1.19              List<JCExpression> subClauses = TreeInfo.isMultiCatch(l.head) ?
    1.20 @@ -1002,17 +1001,18 @@
    1.21                  caught = chk.incl(ct.type, caught);
    1.22              }
    1.23          }
    1.24 +        ListBuffer<JCVariableDecl> resourceVarDecls = ListBuffer.lb();
    1.25          Bits uninitsTryPrev = uninitsTry;
    1.26          ListBuffer<PendingExit> prevPendingExits = pendingExits;
    1.27          pendingExits = new ListBuffer<PendingExit>();
    1.28          Bits initsTry = inits.dup();
    1.29          uninitsTry = uninits.dup();
    1.30 -        unrefdResources = new LinkedHashMap<VarSymbol, JCVariableDecl>();
    1.31          for (JCTree resource : tree.resources) {
    1.32              if (resource instanceof JCVariableDecl) {
    1.33                  JCVariableDecl vdecl = (JCVariableDecl) resource;
    1.34                  visitVarDef(vdecl);
    1.35 -                unrefdResources.put(vdecl.sym, vdecl);
    1.36 +                unrefdResources.enter(vdecl.sym);
    1.37 +                resourceVarDecls.append(vdecl);
    1.38              } else if (resource instanceof JCExpression) {
    1.39                  scanExpr((JCExpression) resource);
    1.40              } else {
    1.41 @@ -1049,11 +1049,14 @@
    1.42          Bits uninitsEnd = uninits;
    1.43          int nextadrCatch = nextadr;
    1.44  
    1.45 -        if (!unrefdResources.isEmpty() &&
    1.46 +        if (!resourceVarDecls.isEmpty() &&
    1.47                  lint.isEnabled(Lint.LintCategory.TRY)) {
    1.48 -            for (Map.Entry<VarSymbol, JCVariableDecl> e : unrefdResources.entrySet()) {
    1.49 -                log.warning(Lint.LintCategory.TRY, e.getValue().pos(),
    1.50 -                            "try.resource.not.referenced", e.getKey());
    1.51 +            for (JCVariableDecl resVar : resourceVarDecls) {
    1.52 +                if (unrefdResources.includes(resVar.sym)) {
    1.53 +                    log.warning(Lint.LintCategory.TRY, resVar.pos(),
    1.54 +                                "try.resource.not.referenced", resVar.sym);
    1.55 +                    unrefdResources.remove(resVar.sym);
    1.56 +                }
    1.57              }
    1.58          }
    1.59  
    1.60 @@ -1143,7 +1146,6 @@
    1.61              while (exits.nonEmpty()) pendingExits.append(exits.next());
    1.62          }
    1.63          uninitsTry.andSet(uninitsTryPrev).andSet(uninits);
    1.64 -        unrefdResources = unrefdResourcesPrev;
    1.65      }
    1.66  
    1.67      public void visitConditional(JCConditional tree) {
    1.68 @@ -1369,9 +1371,7 @@
    1.69      }
    1.70  
    1.71      void referenced(Symbol sym) {
    1.72 -        if (unrefdResources != null && unrefdResources.containsKey(sym)) {
    1.73 -            unrefdResources.remove(sym);
    1.74 -        }
    1.75 +        unrefdResources.remove(sym);
    1.76      }
    1.77  
    1.78      public void visitTypeCast(JCTypeCast tree) {
    1.79 @@ -1430,6 +1430,7 @@
    1.80              alive = true;
    1.81              this.thrown = this.caught = null;
    1.82              this.classDef = null;
    1.83 +            unrefdResources = new Scope(env.enclClass.sym);
    1.84              scan(tree);
    1.85          } finally {
    1.86              // note that recursive invocations of this method fail hard
    1.87 @@ -1444,6 +1445,7 @@
    1.88              this.make = null;
    1.89              this.thrown = this.caught = null;
    1.90              this.classDef = null;
    1.91 +            unrefdResources = null;
    1.92          }
    1.93      }
    1.94  }
     2.1 --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
     2.2 +++ b/test/tools/javac/TryWithResources/UnusedResourcesTest.java	Thu Mar 03 09:43:24 2011 +0000
     2.3 @@ -0,0 +1,250 @@
     2.4 +/*
     2.5 + * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
     2.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
     2.7 + *
     2.8 + * This code is free software; you can redistribute it and/or modify it
     2.9 + * under the terms of the GNU General Public License version 2 only, as
    2.10 + * published by the Free Software Foundation.
    2.11 + *
    2.12 + * This code is distributed in the hope that it will be useful, but WITHOUT
    2.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    2.14 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    2.15 + * version 2 for more details (a copy is included in the LICENSE file that
    2.16 + * accompanied this code).
    2.17 + *
    2.18 + * You should have received a copy of the GNU General Public License version
    2.19 + * 2 along with this work; if not, write to the Free Software Foundation,
    2.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
    2.21 + *
    2.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
    2.23 + * or visit www.oracle.com if you need additional information or have any
    2.24 + * questions.
    2.25 + */
    2.26 +
    2.27 +/*
    2.28 + * @test
    2.29 + * @bug 7023233
    2.30 + * @summary False positive for -Xlint:try with nested try with resources blocks
    2.31 + */
    2.32 +
    2.33 +import com.sun.source.util.JavacTask;
    2.34 +import com.sun.tools.javac.api.JavacTool;
    2.35 +import com.sun.tools.javac.util.JCDiagnostic;
    2.36 +import java.net.URI;
    2.37 +import java.util.Arrays;
    2.38 +import javax.tools.Diagnostic;
    2.39 +import javax.tools.JavaCompiler;
    2.40 +import javax.tools.JavaFileObject;
    2.41 +import javax.tools.SimpleJavaFileObject;
    2.42 +import javax.tools.StandardJavaFileManager;
    2.43 +import javax.tools.ToolProvider;
    2.44 +
    2.45 +public class UnusedResourcesTest {
    2.46 +
    2.47 +    enum XlintOption {
    2.48 +        NONE("none"),
    2.49 +        TRY("try");
    2.50 +
    2.51 +        String opt;
    2.52 +
    2.53 +        XlintOption(String opt) {
    2.54 +            this.opt = opt;
    2.55 +        }
    2.56 +
    2.57 +        String getXlintOption() {
    2.58 +            return "-Xlint:" + opt;
    2.59 +        }
    2.60 +    }
    2.61 +
    2.62 +    enum TwrStmt {
    2.63 +        TWR1("res1"),
    2.64 +        TWR2("res2"),
    2.65 +        TWR3("res3");
    2.66 +
    2.67 +        final String resourceName;
    2.68 +
    2.69 +        private TwrStmt(String resourceName) {
    2.70 +            this.resourceName = resourceName;
    2.71 +        }
    2.72 +    }
    2.73 +
    2.74 +    enum SuppressLevel {
    2.75 +        NONE,
    2.76 +        SUPPRESS;
    2.77 +
    2.78 +        String getSuppressAnno() {
    2.79 +            return this == SUPPRESS ?
    2.80 +                "@SuppressWarnings(\"try\")" :
    2.81 +                "";
    2.82 +        }
    2.83 +    }
    2.84 +
    2.85 +    enum ResourceUsage {
    2.86 +        NONE(null),
    2.87 +        USE_R1(TwrStmt.TWR1),
    2.88 +        USE_R2(TwrStmt.TWR2),
    2.89 +        USE_R3(TwrStmt.TWR3);
    2.90 +
    2.91 +        TwrStmt stmt;
    2.92 +
    2.93 +        private ResourceUsage(TwrStmt stmt) {
    2.94 +            this.stmt = stmt;
    2.95 +        }
    2.96 +
    2.97 +        String usedResourceName() {
    2.98 +            return stmt != null ? stmt.resourceName : null;
    2.99 +        }
   2.100 +
   2.101 +        boolean isUsedIn(TwrStmt res, TwrStmt stmt) {
   2.102 +            return this.stmt == res &&
   2.103 +                    stmt.ordinal() >= this.stmt.ordinal();
   2.104 +        }
   2.105 +
   2.106 +        String getUsage(TwrStmt stmt) {
   2.107 +            return this != NONE && stmt.ordinal() >= this.stmt.ordinal() ?
   2.108 +                "use(" + usedResourceName() + ");" :
   2.109 +                "";
   2.110 +        }
   2.111 +    }
   2.112 +
   2.113 +    static class JavaSource extends SimpleJavaFileObject {
   2.114 +
   2.115 +        String template = "class Resource implements AutoCloseable {\n" +
   2.116 +                              "public void close() {}\n" +
   2.117 +                          "}\n" +
   2.118 +                          "class Test {\n" +
   2.119 +                              "void use(Resource r) {}\n" +
   2.120 +                              "#S void test() {\n" +
   2.121 +                                 "try (Resource #R1 = new Resource()) {\n" +
   2.122 +                                    "#U1_R1\n" +
   2.123 +                                    "#U1_R2\n" +
   2.124 +                                    "#U1_R3\n" +
   2.125 +                                    "try (Resource #R2 = new Resource()) {\n" +
   2.126 +                                       "#U2_R1\n" +
   2.127 +                                       "#U2_R2\n" +
   2.128 +                                       "#U2_R3\n" +
   2.129 +                                       "try (Resource #R3 = new Resource()) {\n" +
   2.130 +                                           "#U3_R1\n" +
   2.131 +                                           "#U3_R2\n" +
   2.132 +                                           "#U3_R3\n" +
   2.133 +                                       "}\n" +
   2.134 +                                    "}\n" +
   2.135 +                                 "}\n" +
   2.136 +                              "}\n" +
   2.137 +                          "}\n";
   2.138 +
   2.139 +        String source;
   2.140 +
   2.141 +        public JavaSource(SuppressLevel suppressLevel, ResourceUsage usage1,
   2.142 +                ResourceUsage usage2, ResourceUsage usage3) {
   2.143 +            super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
   2.144 +            source = template.replace("#S", suppressLevel.getSuppressAnno()).
   2.145 +                    replace("#R1", TwrStmt.TWR1.resourceName).
   2.146 +                    replace("#R2", TwrStmt.TWR2.resourceName).
   2.147 +                    replace("#R3", TwrStmt.TWR3.resourceName).
   2.148 +                    replace("#U1_R1", usage1.getUsage(TwrStmt.TWR1)).
   2.149 +                    replace("#U1_R2", usage2.getUsage(TwrStmt.TWR1)).
   2.150 +                    replace("#U1_R3", usage3.getUsage(TwrStmt.TWR1)).
   2.151 +                    replace("#U2_R1", usage1.getUsage(TwrStmt.TWR2)).
   2.152 +                    replace("#U2_R2", usage2.getUsage(TwrStmt.TWR2)).
   2.153 +                    replace("#U2_R3", usage3.getUsage(TwrStmt.TWR2)).
   2.154 +                    replace("#U3_R1", usage1.getUsage(TwrStmt.TWR3)).
   2.155 +                    replace("#U3_R2", usage2.getUsage(TwrStmt.TWR3)).
   2.156 +                    replace("#U3_R3", usage3.getUsage(TwrStmt.TWR3));
   2.157 +        }
   2.158 +
   2.159 +        @Override
   2.160 +        public CharSequence getCharContent(boolean ignoreEncodingErrors) {
   2.161 +            return source;
   2.162 +        }
   2.163 +    }
   2.164 +
   2.165 +    public static void main(String... args) throws Exception {
   2.166 +        for (XlintOption xlint : XlintOption.values()) {
   2.167 +            for (SuppressLevel suppressLevel : SuppressLevel.values()) {
   2.168 +                for (ResourceUsage usage1 : ResourceUsage.values()) {
   2.169 +                    for (ResourceUsage usage2 : ResourceUsage.values()) {
   2.170 +                        for (ResourceUsage usage3 : ResourceUsage.values()) {
   2.171 +                                test(xlint,
   2.172 +                                        suppressLevel,
   2.173 +                                        usage1,
   2.174 +                                        usage2,
   2.175 +                                        usage3);
   2.176 +                        }
   2.177 +                    }
   2.178 +                }
   2.179 +            }
   2.180 +        }
   2.181 +    }
   2.182 +
   2.183 +    // Create a single file manager and reuse it for each compile to save time.
   2.184 +    static StandardJavaFileManager fm = JavacTool.create().getStandardFileManager(null, null, null);
   2.185 +
   2.186 +    static void test(XlintOption xlint, SuppressLevel suppressLevel, ResourceUsage usage1,
   2.187 +                ResourceUsage usage2, ResourceUsage usage3) throws Exception {
   2.188 +        final JavaCompiler tool = ToolProvider.getSystemJavaCompiler();
   2.189 +        JavaSource source = new JavaSource(suppressLevel, usage1, usage2, usage3);
   2.190 +        DiagnosticChecker dc = new DiagnosticChecker();
   2.191 +        JavacTask ct = (JavacTask)tool.getTask(null, fm, dc,
   2.192 +                Arrays.asList(xlint.getXlintOption()), null, Arrays.asList(source));
   2.193 +        ct.analyze();
   2.194 +        check(source, xlint, suppressLevel, usage1, usage2, usage3, dc);
   2.195 +    }
   2.196 +
   2.197 +    static void check(JavaSource source, XlintOption xlint, SuppressLevel suppressLevel,
   2.198 +                ResourceUsage usage1, ResourceUsage usage2, ResourceUsage usage3, DiagnosticChecker dc) {
   2.199 +
   2.200 +        ResourceUsage[] usages = { usage1, usage2, usage3 };
   2.201 +        boolean[] unusedFound = { dc.unused_r1, dc.unused_r2, dc.unused_r3 };
   2.202 +        boolean[] usedResources = { false, false, false };
   2.203 +
   2.204 +        for (TwrStmt res : TwrStmt.values()) {
   2.205 +            outer: for (TwrStmt stmt : TwrStmt.values()) {
   2.206 +                for (ResourceUsage usage : usages) {
   2.207 +                    if (usage.isUsedIn(res, stmt)) {
   2.208 +                        usedResources[res.ordinal()] = true;
   2.209 +                        break outer;
   2.210 +                    }
   2.211 +                }
   2.212 +            }
   2.213 +        }
   2.214 +
   2.215 +        for (TwrStmt stmt : TwrStmt.values()) {
   2.216 +            boolean unused = !usedResources[stmt.ordinal()] &&
   2.217 +                    xlint == XlintOption.TRY &&
   2.218 +                    suppressLevel != SuppressLevel.SUPPRESS;
   2.219 +            if (unused != unusedFound[stmt.ordinal()]) {
   2.220 +                throw new Error("invalid diagnostics for source:\n" +
   2.221 +                    source.getCharContent(true) +
   2.222 +                    "\nOptions: " + xlint.getXlintOption() +
   2.223 +                    "\nFound unused res1: " + unusedFound[0] +
   2.224 +                    "\nFound unused res2: " + unusedFound[1] +
   2.225 +                    "\nFound unused res3: " + unusedFound[2] +
   2.226 +                    "\nExpected unused res1: " + !usedResources[0] +
   2.227 +                    "\nExpected unused res2: " + !usedResources[1] +
   2.228 +                    "\nExpected unused res3: " + !usedResources[2]);
   2.229 +            }
   2.230 +        }
   2.231 +    }
   2.232 +
   2.233 +    static class DiagnosticChecker implements javax.tools.DiagnosticListener<JavaFileObject> {
   2.234 +
   2.235 +        boolean unused_r1 = false;
   2.236 +        boolean unused_r2 = false;
   2.237 +        boolean unused_r3 = false;
   2.238 +
   2.239 +        public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
   2.240 +            if (diagnostic.getKind() == Diagnostic.Kind.WARNING &&
   2.241 +                    diagnostic.getCode().contains("try.resource.not.referenced")) {
   2.242 +                String varName = ((JCDiagnostic)diagnostic).getArgs()[0].toString();
   2.243 +                if (varName.equals(TwrStmt.TWR1.resourceName)) {
   2.244 +                    unused_r1 = true;
   2.245 +                } else if (varName.equals(TwrStmt.TWR2.resourceName)) {
   2.246 +                    unused_r2 = true;
   2.247 +                } else if (varName.equals(TwrStmt.TWR3.resourceName)) {
   2.248 +                    unused_r3 = true;
   2.249 +                }
   2.250 +            }
   2.251 +        }
   2.252 +    }
   2.253 +}

mercurial