Revision: 7033 Author: [email protected] Date: Thu Nov 19 13:48:07 2009 Log: Fixes an NPE in JsniChecker; improves test coverage on JsniCollector and JsniChecker.
Review by: bobv http://code.google.com/p/google-web-toolkit/source/detail?r=7033 Added: /trunk/dev/core/test/com/google/gwt/dev/javac/JsniCollectorTest.java Modified: /trunk/dev/core/src/com/google/gwt/dev/javac/JsniChecker.java /trunk/dev/core/test/com/google/gwt/dev/javac/CompilationStateTestBase.java /trunk/dev/core/test/com/google/gwt/dev/javac/JavaCompilationSuite.java /trunk/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java /trunk/dev/core/test/com/google/gwt/dev/javac/impl/StaticJavaResource.java ======================================= --- /dev/null +++ /trunk/dev/core/test/com/google/gwt/dev/javac/JsniCollectorTest.java Thu Nov 19 13:48:07 2009 @@ -0,0 +1,142 @@ +/* + * Copyright 2009 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.gwt.dev.javac; + +import com.google.gwt.dev.javac.impl.StaticJavaResource; +import com.google.gwt.dev.jjs.SourceInfo; +import com.google.gwt.dev.js.ast.JsContext; +import com.google.gwt.dev.js.ast.JsExpression; +import com.google.gwt.dev.js.ast.JsNameRef; +import com.google.gwt.dev.js.ast.JsVisitor; + +import org.eclipse.jdt.core.compiler.CategorizedProblem; + +import java.util.ArrayList; +import java.util.List; + +/** + * Tests {...@link JsniCollector}. + */ +public class JsniCollectorTest extends CompilationStateTestBase { + + /** + * TODO: currently JSNI does not parse character position. Turn this on (and + * delete it, actually) when it does. + */ + public static final boolean JSNI_PARSES_SOURCE_POSITION = false; + + public void testErrorPosition() { + StringBuffer code = new StringBuffer(); + code.append("class Foo {\n"); + code.append(" native void m(Object o) /*-{\n"); + code.append(" o...@foo::m(Ljava/lang/String);\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + String source = code.toString(); + + CategorizedProblem[] problems = getProblems("Foo", source); + assertEquals(1, problems.length); + CategorizedProblem problem = problems[0]; + if (JSNI_PARSES_SOURCE_POSITION) { + assertEquals(source.indexOf('@'), problem.getSourceStart()); + } + assertEquals(3, problem.getSourceLineNumber()); + assertTrue(problem.isWarning()); + assertEquals( + "Referencing method 'Foo.m(Ljava/lang/String)': unable to resolve method, expect subsequent failures", + problem.getMessage()); + } + + public void testMalformedJsniRefPosition() { + StringBuffer code = new StringBuffer(); + code.append("class Foo {\n"); + code.append(" native void m() /*-{\n"); + code.append(" @Bar;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + String source = code.toString(); + CategorizedProblem[] problems = getProblems("Foo", source); + assertEquals(1, problems.length); + CategorizedProblem problem = problems[0]; + assertEquals(source.indexOf('@') + "Bar".length(), problem.getSourceStart()); + assertEquals(3, problem.getSourceLineNumber()); + assertTrue(problem.isError()); + assertEquals("Expected \":\" in JSNI reference", problem.getMessage()); + } + + public void testMalformedJsniRefPositionWithExtraLines() { + StringBuffer code = new StringBuffer(); + code.append("class Foo {\n"); + code.append(" native\nvoid\nm()\n\n\n/*-{\n\n"); + code.append(" @Bar;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + String source = code.toString(); + CategorizedProblem[] problems = getProblems("Foo", source); + assertEquals(1, problems.length); + CategorizedProblem problem = problems[0]; + assertEquals(source.indexOf('@') + "Bar".length(), problem.getSourceStart()); + assertEquals(9, problem.getSourceLineNumber()); + assertTrue(problem.isError()); + assertEquals("Expected \":\" in JSNI reference", problem.getMessage()); + } + + public void testSourcePosition() { + StringBuffer code = new StringBuffer(); + code.append("class Foo {\n"); + code.append(" native void m(Object o) /*-{\n"); + code.append(" o...@foo::m(Ljava/lang/Object);\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + String source = code.toString(); + + List<JsNameRef> foundRefs = findJsniRefs("Foo", source); + assertEquals(1, foundRefs.size()); + JsNameRef ref = foundRefs.get(0); + SourceInfo info = ref.getSourceInfo(); + if (JSNI_PARSES_SOURCE_POSITION) { + assertEquals(source.indexOf('@'), info.getStartPos()); + } + assertEquals(3, info.getStartLine()); + } + + private List<JsNameRef> findJsniRefs(String typeName, final String source) { + addGeneratedUnits(new StaticJavaResource(typeName, source)); + CompilationUnit unit = state.getCompilationUnitMap().get(typeName); + assertNotNull(unit); + List<JsniMethod> jsniMethods = unit.getJsniMethods(); + assertEquals(1, jsniMethods.size()); + JsniMethod jsniMethod = jsniMethods.get(0); + final List<JsNameRef> foundRefs = new ArrayList<JsNameRef>(); + new JsVisitor() { + @Override + public void endVisit(JsNameRef x, JsContext<JsExpression> ctx) { + if (x.getIdent().startsWith("@")) { + foundRefs.add(x); + } + } + }.accept(jsniMethod.function()); + return foundRefs; + } + + private CategorizedProblem[] getProblems(String typeName, String source) { + addGeneratedUnits(new StaticJavaResource(typeName, source)); + CompilationUnit unit = state.getCompilationUnitMap().get(typeName); + assertNotNull(unit); + CategorizedProblem[] problems = unit.getProblems(); + return problems; + } +} ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/javac/JsniChecker.java Fri Nov 13 07:52:59 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/JsniChecker.java Thu Nov 19 13:48:07 2009 @@ -84,7 +84,9 @@ checkDecl(meth, scope); } JsniMethod jsniMethod = jsniMethods.get(meth); - new JsniRefChecker(meth, hasUnsafeLongsAnnotation).check(jsniMethod.function()); + if (jsniMethod != null) { + new JsniRefChecker(meth, hasUnsafeLongsAnnotation).check(jsniMethod.function()); + } } } @@ -156,6 +158,9 @@ } FieldBinding target = getField(clazz, jsniRef); if (target == null) { + emitWarning("jsni", "Referencing field '" + jsniRef.className() + "." + + jsniRef.memberName() + + "': unable to resolve field, expect subsequent failures"); return; } if (target.isDeprecated()) { @@ -177,6 +182,9 @@ assert jsniRef.isMethod(); MethodBinding target = getMethod(clazz, jsniRef); if (target == null) { + emitWarning("jsni", "Referencing method '" + jsniRef.className() + "." + + jsniRef.memberSignature() + + "': unable to resolve method, expect subsequent failures"); return; } if (target.isDeprecated()) { @@ -262,7 +270,7 @@ } } else { emitWarning("jsni", "Referencing class '" + className - + ": unable to resolve class, expect subsequent failures"); + + "': unable to resolve class, expect subsequent failures"); } } ======================================= --- /trunk/dev/core/test/com/google/gwt/dev/javac/CompilationStateTestBase.java Thu Nov 12 06:32:35 2009 +++ /trunk/dev/core/test/com/google/gwt/dev/javac/CompilationStateTestBase.java Thu Nov 19 13:48:07 2009 @@ -17,7 +17,6 @@ import com.google.gwt.core.ext.TreeLogger; import com.google.gwt.dev.javac.impl.JavaResourceBase; -import com.google.gwt.dev.javac.impl.MockJavaResource; import com.google.gwt.dev.javac.impl.MockResource; import com.google.gwt.dev.javac.impl.MockResourceOracle; import com.google.gwt.dev.resource.Resource; @@ -48,7 +47,7 @@ boolean reallyLog = false; if (reallyLog) { AbstractTreeLogger logger = new PrintWriterTreeLogger(); - logger.setMaxDetail(TreeLogger.ALL); + logger.setMaxDetail(TreeLogger.WARN); return logger; } return TreeLogger.NULL; @@ -102,7 +101,7 @@ protected CompilationState state = isolatedBuilder.doBuildFrom( createTreeLogger(), oracle.getResources()); - protected void addGeneratedUnits(MockJavaResource... sourceFiles) { + protected void addGeneratedUnits(MockResource... sourceFiles) { state.addGeneratedCompilationUnits(createTreeLogger(), getGeneratedUnits(sourceFiles)); } ======================================= --- /trunk/dev/core/test/com/google/gwt/dev/javac/JavaCompilationSuite.java Tue Nov 10 20:40:43 2009 +++ /trunk/dev/core/test/com/google/gwt/dev/javac/JavaCompilationSuite.java Thu Nov 19 13:48:07 2009 @@ -39,6 +39,7 @@ suite.addTestSuite(JdtCompilerTest.class); suite.addTestSuite(JSORestrictionsTest.class); suite.addTestSuite(JsniCheckerTest.class); + suite.addTestSuite(JsniCollectorTest.class); suite.addTestSuite(TypeOracleMediatorTest.class); suite.addTestSuite(CollectClassDataTest.class); ======================================= --- /trunk/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java Tue Nov 10 20:42:30 2009 +++ /trunk/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java Thu Nov 19 13:48:07 2009 @@ -61,6 +61,29 @@ shouldGenerateError(code, 10, "Referencing class \'Buggy$1.A: " + "JSNI references to anonymous classes are illegal"); } + + public void testArrayBadMember() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" native void jsniMethod() /*-{\n"); + code.append(" @Buggy[][]::blah;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + shouldGenerateError( + code, + 3, + "Referencing member 'Buggy[][].blah': 'class' is the only legal reference for array types"); + } + + public void testArrayClass() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" native void jsniMethod() /*-{\n"); + code.append(" @Buggy[][]::class;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + shouldGenerateNoWarning(code); + } public void testCyclicReferences() { { @@ -248,6 +271,16 @@ shouldGenerateError(code, 2, "Type 'long' may not be returned from a JSNI method"); } + + public void testMalformedJsniRef() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" native void jsniMethod() /*-{\n"); + code.append(" @Buggy;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + shouldGenerateError(code, 3, "Expected \":\" in JSNI reference"); + } public void testMethodArgument() { StringBuffer code = new StringBuffer(); @@ -324,6 +357,29 @@ 5, "Referencing method 'Buggy.m': return type 'long' is not safe to access in JSNI code"); } + + public void testPrimitiveBadMember() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" native void jsniMethod() /*-{\n"); + code.append(" @Z::blah;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + shouldGenerateError( + code, + 3, + "Referencing member 'Z.blah': 'class' is the only legal reference for primitive types"); + } + + public void testPrimitiveClass() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" native void jsniMethod() /*-{\n"); + code.append(" @Z::class;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + shouldGenerateNoWarning(code); + } public void testRefInString() { { @@ -337,6 +393,43 @@ shouldGenerateNoError(code); } } + + public void testUnresolvedClass() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" native void jsniMethod() /*-{\n"); + code.append(" @Foo::x;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + shouldGenerateWarning(code, 3, + "Referencing class 'Foo': unable to resolve class, expect subsequent failures"); + } + + public void testUnresolvedField() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" native void jsniMethod() /*-{\n"); + code.append(" @Buggy::x;\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + shouldGenerateWarning( + code, + 3, + "Referencing field 'Buggy.x': unable to resolve field, expect subsequent failures"); + } + + public void testUnresolvedMethod() { + StringBuffer code = new StringBuffer(); + code.append("class Buggy {\n"); + code.append(" native void jsniMethod() /*-{\n"); + code.append(" @Buggy::x(Ljava/lang/String);\n"); + code.append(" }-*/;\n"); + code.append("}\n"); + shouldGenerateWarning( + code, + 3, + "Referencing method 'Buggy.x(Ljava/lang/String)': unable to resolve method, expect subsequent failures"); + } public void testUnsafeAnnotation() { { ======================================= --- /trunk/dev/core/test/com/google/gwt/dev/javac/impl/StaticJavaResource.java Tue Nov 10 20:40:43 2009 +++ /trunk/dev/core/test/com/google/gwt/dev/javac/impl/StaticJavaResource.java Thu Nov 19 13:48:07 2009 @@ -15,14 +15,12 @@ */ package com.google.gwt.dev.javac.impl; -import com.google.gwt.dev.javac.Shared; - -public class StaticJavaResource extends MockResource { +public class StaticJavaResource extends MockJavaResource { private final CharSequence source; - public StaticJavaResource(String typeName, CharSequence source) { - super(Shared.toPath(typeName)); + public StaticJavaResource(String qualifiedTypeName, CharSequence source) { + super(qualifiedTypeName); this.source = source; } -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
