changeset 2840:84849fdb360b

8073645: Add lambda-based lazy eval versions of Assert.check methods Summary: Enhance Assert so that lazy string computation can occurr where needed; enhance static roding rule checkers to make sure the right version of the method is called. Reviewed-by: jlahoda
author mcimadamore
date Thu, 05 Mar 2015 13:10:49 +0000
parents 592d64800143
children edf685b5d413
files make/intellij/build.xml make/intellij/workspace.xml make/test/crules/CodingRulesAnalyzerPlugin/Test.java make/test/crules/CodingRulesAnalyzerPlugin/Test.out make/tools/crules/AssertCheckAnalyzer.java make/tools/crules/resources/crules.properties src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java src/jdk.compiler/share/classes/com/sun/tools/javac/util/Assert.java
diffstat 9 files changed, 127 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/make/intellij/build.xml	Mon Mar 02 10:41:08 2015 +0530
+++ b/make/intellij/build.xml	Thu Mar 05 13:10:49 2015 +0000
@@ -28,6 +28,10 @@
         </sequential>
     </macrodef>
 
+    <target name="crules" depends="build-all-tools,-def-jtreg">
+        <jtreg-tool name="all" tests="tools/all/RunCodingRules.java"/>
+    </target>
+
     <target name="post-make" depends="clean, build-all-tools"/>
 
     <target name="jtreg-debug" depends="build-all-tools,-def-jtreg">
--- a/make/intellij/workspace.xml	Mon Mar 02 10:41:08 2015 +0530
+++ b/make/intellij/workspace.xml	Thu Mar 05 13:10:49 2015 +0000
@@ -156,7 +156,7 @@
         <filter targetName="clean" isVisible="true" />
         <filter targetName="jtreg" isVisible="true" />
         <filter targetName="jtreg-debug" isVisible="true" />
-        <filter targetName="checkstyle" isVisible="true" />
+        <filter targetName="crules" isVisible="true" />
       </targetFilters>
       <viewClosedWhenNoErrors value="true" />
       <expanded value="false" />
--- a/make/test/crules/CodingRulesAnalyzerPlugin/Test.java	Mon Mar 02 10:41:08 2015 +0530
+++ b/make/test/crules/CodingRulesAnalyzerPlugin/Test.java	Thu Mar 05 13:10:49 2015 +0000
@@ -5,7 +5,31 @@
 import com.sun.tools.javac.util.Assert;
 
 public class Test {
-    public void check(String value) {
-        Assert.check(value.trim().length() > 0, "value=" + value);
+
+    String v;
+
+    public void check1(String value) {
+        Assert.check(value.trim().length() > 0, "value=" + value); //fail
+    }
+    public void check2(String value) {
+        Assert.check(value.trim().length() > 0, "value=" + "value"); //ok
+    }
+    public void check3(String value) {
+        Assert.check(value.trim().length() > 0, () -> "value=" + value); //ok
+    }
+    public void check4(String value) {
+        Assert.check(value.trim().length() > 0, value); //ok
+    }
+    public void check5(String value) {
+        Assert.check(value.trim().length() > 0, v); //ok
+    }
+    public void check6(String value) {
+        Assert.check(value.trim().length() > 0, () -> "value=" + "value"); //fail
+    }
+    public void check7(String value) {
+        Assert.check(value.trim().length() > 0, () -> value); //fail
+    }
+    public void check8(String value) {
+        Assert.check(value.trim().length() > 0, () -> v); //fail
     }
 }
--- a/make/test/crules/CodingRulesAnalyzerPlugin/Test.out	Mon Mar 02 10:41:08 2015 +0530
+++ b/make/test/crules/CodingRulesAnalyzerPlugin/Test.out	Thu Mar 05 13:10:49 2015 +0000
@@ -1,2 +1,5 @@
-Test.java:9:21: compiler.err.proc.messager: compiler.misc.crules.should.not.use.string.concatenation
-1 error
+Test.java:12:58: compiler.err.proc.messager: compiler.misc.crules.should.not.use.eager.string.evaluation
+Test.java:27:49: compiler.err.proc.messager: compiler.misc.crules.should.not.use.lazy.string.evaluation
+Test.java:30:49: compiler.err.proc.messager: compiler.misc.crules.should.not.use.lazy.string.evaluation
+Test.java:33:49: compiler.err.proc.messager: compiler.misc.crules.should.not.use.lazy.string.evaluation
+4 errors
--- a/make/tools/crules/AssertCheckAnalyzer.java	Mon Mar 02 10:41:08 2015 +0530
+++ b/make/tools/crules/AssertCheckAnalyzer.java	Thu Mar 05 13:10:49 2015 +0000
@@ -23,10 +23,14 @@
 
 package crules;
 
+import com.sun.source.tree.LambdaExpressionTree.BodyKind;
 import com.sun.source.util.JavacTask;
 import com.sun.source.util.TaskEvent.Kind;
+import com.sun.tools.javac.code.Kinds;
 import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.tree.JCTree.JCExpression;
+import com.sun.tools.javac.tree.JCTree.JCLambda;
 import com.sun.tools.javac.tree.JCTree.JCMethodInvocation;
 import com.sun.tools.javac.tree.JCTree.Tag;
 import com.sun.tools.javac.tree.TreeInfo;
@@ -35,6 +39,22 @@
 
 public class AssertCheckAnalyzer extends AbstractCodingRulesAnalyzer {
 
+    enum AssertOverloadKind {
+        EAGER("crules.should.not.use.eager.string.evaluation"),
+        LAZY("crules.should.not.use.lazy.string.evaluation"),
+        NONE(null);
+
+        String errKey;
+
+        AssertOverloadKind(String errKey) {
+            this.errKey = errKey;
+        }
+
+        boolean simpleArgExpected() {
+            return this == AssertOverloadKind.EAGER;
+        }
+    }
+
     public AssertCheckAnalyzer(JavacTask task) {
         super(task);
         treeVisitor = new AssertCheckVisitor();
@@ -45,20 +65,46 @@
 
         @Override
         public void visitApply(JCMethodInvocation tree) {
-            Symbol method = TreeInfo.symbolFor(tree);
-            if (method != null &&
-                method.owner.getQualifiedName().contentEquals(Assert.class.getName()) &&
-                !method.name.contentEquals("error")) {
+            Symbol m = TreeInfo.symbolFor(tree);
+            AssertOverloadKind ak = assertOverloadKind(m);
+            if (ak != AssertOverloadKind.NONE &&
+                !m.name.contentEquals("error")) {
                 JCExpression lastParam = tree.args.last();
-                if (lastParam != null &&
-                    lastParam.type.tsym == syms.stringType.tsym &&
-                    lastParam.hasTag(Tag.PLUS)) {
-                    messages.error(tree, "crules.should.not.use.string.concatenation");
+                if (isSimpleStringArg(lastParam) != ak.simpleArgExpected()) {
+                    messages.error(lastParam, ak.errKey);
                 }
             }
 
             super.visitApply(tree);
         }
 
+        AssertOverloadKind assertOverloadKind(Symbol method) {
+            if (method == null ||
+                !method.owner.getQualifiedName().contentEquals(Assert.class.getName()) ||
+                method.type.getParameterTypes().tail == null) {
+                return AssertOverloadKind.NONE;
+            }
+            Type formal = method.type.getParameterTypes().last();
+            if (types.isSameType(formal, syms.stringType)) {
+                return AssertOverloadKind.EAGER;
+            } else if (types.isSameType(types.erasure(formal), types.erasure(syms.supplierType))) {
+                return AssertOverloadKind.LAZY;
+            } else {
+                return AssertOverloadKind.NONE;
+            }
+        }
+
+        boolean isSimpleStringArg(JCExpression e) {
+            switch (e.getTag()) {
+                case LAMBDA:
+                    JCLambda lambda = (JCLambda)e;
+                    return (lambda.getBodyKind() == BodyKind.EXPRESSION) &&
+                            isSimpleStringArg((JCExpression)lambda.body);
+                default:
+                    Symbol argSym = TreeInfo.symbolFor(e);
+                    return (e.type.constValue() != null ||
+                            (argSym != null && argSym.kind == Kinds.Kind.VAR));
+            }
+        }
     }
 }
--- a/make/tools/crules/resources/crules.properties	Mon Mar 02 10:41:08 2015 +0530
+++ b/make/tools/crules/resources/crules.properties	Thu Mar 05 13:10:49 2015 +0000
@@ -26,8 +26,10 @@
 # 0: symbol
 crules.err.var.must.be.final=\
     Static variable {0} must be final
-crules.should.not.use.string.concatenation=\
-    Should not use string concatenation.
+crules.should.not.use.eager.string.evaluation=\
+    Should not use eager string evaluation. Use lazy version instead.
+crules.should.not.use.lazy.string.evaluation=\
+    Should not use eager lazy evaluation. Use eager version instead.
 crules.no.defined.by=\
     This method implements a public API method, and should be marked with @DefinedBy.
 crules.wrong.defined.by=\
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java	Mon Mar 02 10:41:08 2015 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java	Thu Mar 05 13:10:49 2015 +0000
@@ -184,6 +184,7 @@
     public final Type retentionType;
     public final Type deprecatedType;
     public final Type suppressWarningsType;
+    public final Type supplierType;
     public final Type inheritedType;
     public final Type profileType;
     public final Type proprietaryType;
@@ -449,6 +450,7 @@
         retentionType = enterClass("java.lang.annotation.Retention");
         deprecatedType = enterClass("java.lang.Deprecated");
         suppressWarningsType = enterClass("java.lang.SuppressWarnings");
+        supplierType = enterClass("java.util.function.Supplier");
         inheritedType = enterClass("java.lang.annotation.Inherited");
         repeatableType = enterClass("java.lang.annotation.Repeatable");
         documentedType = enterClass("java.lang.annotation.Documented");
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java	Mon Mar 02 10:41:08 2015 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java	Thu Mar 05 13:10:49 2015 +0000
@@ -129,7 +129,7 @@
                 || node instanceof JCErroneous
                 || (node instanceof JCExpressionStatement
                     && ((JCExpressionStatement)node).expr instanceof JCErroneous),
-                node.getClass().getSimpleName());
+                    () -> node.getClass().getSimpleName());
         JCCompilationUnit tree = new JCCompilationUnit(defs);
         tree.pos = pos;
         return tree;
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Assert.java	Mon Mar 02 10:41:08 2015 +0530
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Assert.java	Thu Mar 05 13:10:49 2015 +0000
@@ -25,6 +25,8 @@
 
 package com.sun.tools.javac.util;
 
+import java.util.function.Supplier;
+
 /**
  * Simple facility for unconditional assertions.
  * The methods in this class are described in terms of equivalent assert
@@ -94,6 +96,15 @@
     }
 
     /** Equivalent to
+     *   assert cond : msg.get();
+     *  Note: message string is computed lazily.
+     */
+    public static void check(boolean cond, Supplier<String> msg) {
+        if (!cond)
+            error(msg.get());
+    }
+
+    /** Equivalent to
      *   assert (o == null) : value;
      */
     public static void checkNull(Object o, Object value) {
@@ -110,6 +121,15 @@
     }
 
     /** Equivalent to
+     *   assert (o == null) : msg.get();
+     *  Note: message string is computed lazily.
+     */
+    public static void checkNull(Object o, Supplier<String> msg) {
+        if (o != null)
+            error(msg.get());
+    }
+
+    /** Equivalent to
      *   assert (o != null) : msg;
      */
     public static <T> T checkNonNull(T t, String msg) {
@@ -119,6 +139,16 @@
     }
 
     /** Equivalent to
+     *   assert (o != null) : msg.get();
+     *  Note: message string is computed lazily.
+     */
+    public static <T> T checkNonNull(T t, Supplier<String> msg) {
+        if (t == null)
+            error(msg.get());
+        return t;
+    }
+
+    /** Equivalent to
      *   assert false;
      */
     public static void error() {