Author: sco...@google.com Date: Fri Feb 13 15:26:20 2009 New Revision: 4723
Added: releases/1.6/dev/core/src/com/google/gwt/dev/javac/JsniChecker.java - copied, changed from r4717, /releases/1.6/dev/core/src/com/google/gwt/dev/javac/LongFromJSNIChecker.java releases/1.6/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java - copied, changed from r4717, /releases/1.6/dev/core/test/com/google/gwt/dev/javac/LongFromJSNITest.java Removed: releases/1.6/dev/core/src/com/google/gwt/dev/javac/LongFromJSNIChecker.java releases/1.6/dev/core/test/com/google/gwt/dev/javac/LongFromJSNITest.java Modified: releases/1.6/dev/core/src/com/google/gwt/dev/javac/CompilationUnitInvalidator.java releases/1.6/dev/core/test/com/google/gwt/dev/javac/JavaCompilationSuite.java Log: Adds a warning to JsniChecker to detect references to anonymous classes. Review by: spoon (TBR) Modified: releases/1.6/dev/core/src/com/google/gwt/dev/javac/CompilationUnitInvalidator.java ============================================================================== --- releases/1.6/dev/core/src/com/google/gwt/dev/javac/CompilationUnitInvalidator.java (original) +++ releases/1.6/dev/core/src/com/google/gwt/dev/javac/CompilationUnitInvalidator.java Fri Feb 13 15:26:20 2009 @@ -156,7 +156,7 @@ if (unit.getState() == State.COMPILED) { CompilationUnitDeclaration jdtCud = unit.getJdtCud(); JSORestrictionsChecker.check(jdtCud); - LongFromJSNIChecker.check(jdtCud); + JsniChecker.check(jdtCud); BinaryTypeReferenceRestrictionsChecker.check(jdtCud, validBinaryTypeNames); } Copied: releases/1.6/dev/core/src/com/google/gwt/dev/javac/JsniChecker.java (from r4717, /releases/1.6/dev/core/src/com/google/gwt/dev/javac/LongFromJSNIChecker.java) ============================================================================== --- /releases/1.6/dev/core/src/com/google/gwt/dev/javac/LongFromJSNIChecker.java (original) +++ releases/1.6/dev/core/src/com/google/gwt/dev/javac/JsniChecker.java Fri Feb 13 15:26:20 2009 @@ -36,8 +36,10 @@ import org.eclipse.jdt.internal.compiler.lookup.ProblemReasons; import org.eclipse.jdt.internal.compiler.lookup.ProblemReferenceBinding; import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding; +import org.eclipse.jdt.internal.compiler.lookup.TagBits; import org.eclipse.jdt.internal.compiler.lookup.TypeBinding; import org.eclipse.jdt.internal.compiler.lookup.TypeIds; +import org.eclipse.jdt.internal.compiler.problem.ProblemSeverities; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -45,15 +47,16 @@ import java.util.Set; /** - * Tests for access to Java longs from JSNI. Issues a warning for: + * Tests for access to Java from JSNI. Issues a warning for: * <ul> - * <li> JSNI methods with a parameter or return type of long. - * <li> Access from JSNI to a field whose type is long. - * <li> Access from JSNI to a method with a parameter or return type of long. + * <li>JSNI methods with a parameter or return type of long.</li> + * <li>Access from JSNI to a field whose type is long.</li> + * <li>Access from JSNI to a method with a parameter or return type of long.</li> + * <li>JSNI references to anonymous classes.</li> * </ul> * All tests also apply for arrays of longs, arrays of arrays of longs, etc. */ -public class LongFromJSNIChecker { +public class JsniChecker { private class CheckingVisitor extends ASTVisitor implements ClassFileConstants { @Override @@ -82,9 +85,10 @@ } } - private void checkFieldRef(JsniRef jsniRef, Set<String> errors) { + private void checkFieldRef(ReferenceBinding clazz, JsniRef jsniRef, + Set<String> errors) { assert jsniRef.isField(); - FieldBinding target = getField(jsniRef); + FieldBinding target = getField(clazz, jsniRef); if (target == null) { return; } @@ -95,9 +99,10 @@ } } - private void checkMethodRef(JsniRef jsniRef, Set<String> errors) { + private void checkMethodRef(ReferenceBinding clazz, JsniRef jsniRef, + Set<String> errors) { assert jsniRef.isMethod(); - MethodBinding target = getMethod(jsniRef); + MethodBinding target = getMethod(clazz, jsniRef); if (target == null) { return; } @@ -136,13 +141,26 @@ JsniRef jsniRef = JsniRef.parse(jsniRefString); if (jsniRef != null) { Set<String> refErrors = new LinkedHashSet<String>(); - if (jsniRef.isMethod()) { - checkMethodRef(jsniRef, refErrors); - } else { - checkFieldRef(jsniRef, refErrors); - } - if (!refErrors.isEmpty()) { - errors.put(jsniRefString, refErrors); + ReferenceBinding clazz = findClass(jsniRef); + if (clazz != null) { + if (clazz.isAnonymousType()) { + GWTProblem.recordInCud( + ProblemSeverities.Warning, + meth, + cud, + "Referencing class '" + jsniRef.className() + + ": JSNI references to anonymous classes are deprecated", + null); + } else { + if (jsniRef.isMethod()) { + checkMethodRef(clazz, jsniRef, refErrors); + } else { + checkFieldRef(clazz, jsniRef, refErrors); + } + if (!refErrors.isEmpty()) { + errors.put(jsniRefString, refErrors); + } + } } } } @@ -189,11 +207,9 @@ if (binding instanceof ProblemReferenceBinding) { ProblemReferenceBinding prb = (ProblemReferenceBinding) binding; - if (prb.problemId() == ProblemReasons.NotVisible - && prb.closestMatch() instanceof ReferenceBinding) { - // It's just a visibility problem, so try drilling - // down manually - ReferenceBinding drilling = (ReferenceBinding) prb.closestMatch(); + if (prb.problemId() == ProblemReasons.NotVisible) { + // It's just a visibility problem, so try drilling down manually + ReferenceBinding drilling = prb.closestMatch(); for (int i = prb.compoundName.length; i < compoundName.length; i++) { drilling = drilling.getMemberType(compoundName[i]); } @@ -201,31 +217,31 @@ } } - if (binding instanceof ProblemReferenceBinding) { - return null; - } - if (binding instanceof ReferenceBinding) { + if (binding instanceof ReferenceBinding + && !(binding instanceof ProblemReferenceBinding)) { return (ReferenceBinding) binding; } + // See if it looks like an anonymous inner class, we can't look those up. + for (char[] part : compoundName) { + if (Character.isDigit(part[0])) { + return new ReferenceBinding() { + { + tagBits = TagBits.IsAnonymousType; + } + }; + } + } return null; } - private FieldBinding getField(JsniRef jsniRef) { + private FieldBinding getField(ReferenceBinding clazz, JsniRef jsniRef) { assert jsniRef.isField(); - ReferenceBinding type = findClass(jsniRef); - if (type == null) { - return null; - } - return type.getField(jsniRef.memberName().toCharArray(), false); + return clazz.getField(jsniRef.memberName().toCharArray(), false); } - private MethodBinding getMethod(JsniRef jsniRef) { + private MethodBinding getMethod(ReferenceBinding clazz, JsniRef jsniRef) { assert jsniRef.isMethod(); - ReferenceBinding type = findClass(jsniRef); - if (type == null) { - return null; - } - for (MethodBinding method : type.getMethods(jsniRef.memberName().toCharArray())) { + for (MethodBinding method : clazz.getMethods(jsniRef.memberName().toCharArray())) { if (paramTypesMatch(method, jsniRef)) { return method; } @@ -294,13 +310,13 @@ * */ public static void check(CompilationUnitDeclaration cud) { - LongFromJSNIChecker checker = new LongFromJSNIChecker(cud); + JsniChecker checker = new JsniChecker(cud); checker.check(); } private final CompilationUnitDeclaration cud; - private LongFromJSNIChecker(CompilationUnitDeclaration cud) { + private JsniChecker(CompilationUnitDeclaration cud) { this.cud = cud; } Modified: releases/1.6/dev/core/test/com/google/gwt/dev/javac/JavaCompilationSuite.java ============================================================================== --- releases/1.6/dev/core/test/com/google/gwt/dev/javac/JavaCompilationSuite.java (original) +++ releases/1.6/dev/core/test/com/google/gwt/dev/javac/JavaCompilationSuite.java Fri Feb 13 15:26:20 2009 @@ -35,7 +35,7 @@ suite.addTestSuite(JdtCompilerTest.class); suite.addTestSuite(JSORestrictionsTest.class); suite.addTestSuite(JavaSourceOracleImplTest.class); - suite.addTestSuite(LongFromJSNITest.class); + suite.addTestSuite(JsniCheckerTest.class); suite.addTestSuite(TypeOracleMediatorTest.class); return suite; Copied: releases/1.6/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java (from r4717, /releases/1.6/dev/core/test/com/google/gwt/dev/javac/LongFromJSNITest.java) ============================================================================== --- /releases/1.6/dev/core/test/com/google/gwt/dev/javac/LongFromJSNITest.java (original) +++ releases/1.6/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java Fri Feb 13 15:26:20 2009 @@ -16,6 +16,7 @@ package com.google.gwt.dev.javac; import com.google.gwt.core.ext.TreeLogger; +import com.google.gwt.core.ext.TreeLogger.Type; import com.google.gwt.core.ext.typeinfo.TypeOracle; import com.google.gwt.dev.util.UnitTestTreeLogger; @@ -27,7 +28,28 @@ /** * Test access to longs from JSNI. */ -public class LongFromJSNITest extends TestCase { +public class JsniCheckerTest extends TestCase { + + /** + * JSNI references to anonymous inner classes is deprecated. + */ + public void testAnoymousJsniRef() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" static void main() {\n"); + code.append(" new Object() {\n"); + code.append(" int foo = 3;\n"); + code.append(" };\n"); + code.append(" }\n"); + code.append(" native void jsniMeth(Object o) /*-{\n"); + code.append(" o...@buggy$1::foo;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + + shouldGenerateWarning(code, 7, "Referencing class \'Buggy$1: " + + "JSNI references to anonymous classes are deprecated"); + } + public void testCyclicReferences() { { StringBuffer buggy = new StringBuffer(); @@ -281,24 +303,33 @@ units.toArray(new CompilationUnit[units.size()])); } - private void shouldGenerateError(CharSequence buggyCode, - CharSequence extraCode, int line, String message) { + private void shouldGenerate(CharSequence buggyCode, CharSequence extraCode, + int line, Type logLevel, String logHeader, String message) { UnitTestTreeLogger.Builder b = new UnitTestTreeLogger.Builder(); - b.setLowestLogLevel(TreeLogger.ERROR); + b.setLowestLogLevel(logLevel); if (message != null) { - b.expect(TreeLogger.ERROR, "Errors in '/mock/Buggy'", null); + b.expect(logLevel, logHeader + " in '/mock/Buggy'", null); final String fullMessage = "Line " + line + ": " + message; - b.expect(TreeLogger.ERROR, fullMessage, null); + b.expect(logLevel, fullMessage, null); } UnitTestTreeLogger logger = b.createLogger(); TypeOracle oracle = buildOracle(buggyCode, extraCode, logger); logger.assertCorrectLogEntries(); - if (message != null) { - assertEquals("Buggy compilation unit not removed from type oracle", null, + if (message != null && logLevel == TreeLogger.ERROR) { + assertNull("Buggy compilation unit not removed from type oracle", + oracle.findType("Buggy")); + } else { + assertNotNull("Buggy compilation unit removed with only a warning", oracle.findType("Buggy")); } } + private void shouldGenerateError(CharSequence buggyCode, + CharSequence extraCode, int line, String message) { + shouldGenerate(buggyCode, extraCode, line, TreeLogger.ERROR, "Errors", + message); + } + private void shouldGenerateError(CharSequence buggyCode, int line, String message) { shouldGenerateError(buggyCode, null, line, message); @@ -310,5 +341,16 @@ private void shouldGenerateNoError(CharSequence code, CharSequence extraCode) { shouldGenerateError(code, extraCode, -1, null); + } + + private void shouldGenerateWarning(CharSequence buggyCode, + CharSequence extraCode, int line, String message) { + shouldGenerate(buggyCode, extraCode, line, TreeLogger.WARN, "Warnings", + message); + } + + private void shouldGenerateWarning(CharSequence buggyCode, int line, + String message) { + shouldGenerateWarning(buggyCode, null, line, message); } } --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---