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
--- a/langtools/make/intellij/build.xml Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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/langtools/make/intellij/workspace.xml Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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/langtools/make/test/crules/CodingRulesAnalyzerPlugin/Test.java Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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/langtools/make/test/crules/CodingRulesAnalyzerPlugin/Test.out Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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/langtools/make/tools/crules/AssertCheckAnalyzer.java Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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/langtools/make/tools/crules/resources/crules.properties Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symtab.java Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Assert.java Mon Mar 02 10:41:08 2015 +0530
+++ b/langtools/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() {