8073645: Add lambda-based lazy eval versions of Assert.check methods

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
This commit is contained in:
Maurizio Cimadamore 2015-03-05 13:10:49 +00:00
parent 105275fb87
commit 1114c26925
9 changed files with 127 additions and 16 deletions

View File

@ -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">

View File

@ -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" />

View File

@ -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
}
}

View File

@ -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

View File

@ -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 @@ import com.sun.tools.javac.util.Assert;
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 @@ public class AssertCheckAnalyzer extends AbstractCodingRulesAnalyzer {
@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));
}
}
}
}

View File

@ -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=\

View File

@ -184,6 +184,7 @@ public class Symtab {
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 @@ public class Symtab {
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");

View File

@ -129,7 +129,7 @@ public class TreeMaker implements JCTree.Factory {
|| 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;

View File

@ -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
@ -93,6 +95,15 @@ public class Assert {
error(msg);
}
/** 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;
*/
@ -109,6 +120,15 @@ public class Assert {
error(msg);
}
/** 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;
*/
@ -118,6 +138,16 @@ public class Assert {
return t;
}
/** 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;
*/