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

Reply via email to