This patch adds a modular testing framework for adding ApiChecker tests. In
addition, it fixes the following 3 cases where the ApiChecker was wrong and
adds tests for each case.
(i) A api class is made FINAL and simultaneously the FINAL keyword is
removed from its methods.
(ii) An api class's methods are overloaded and the overloading is such that the
methods are "compatible."
(iii) A api class's fields are moved to its superclass.
Patch by: amitmanjhi
Review by: ecc (desk review)
git-svn-id: https://google-web-toolkit.googlecode.com/svn/releases/1.6@4514 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/tools/api-checker/config/gwt15_16userApi.conf b/tools/api-checker/config/gwt15_16userApi.conf
index a5be761..4c179ec 100644
--- a/tools/api-checker/config/gwt15_16userApi.conf
+++ b/tools/api-checker/config/gwt15_16userApi.conf
@@ -96,6 +96,5 @@
# to be hard. Come back and fix this issue, if more such cases come up.
java.lang.StringBuilder::append(Ljava/lang/StringBuffer;) OVERRIDABLE_METHOD_ARGUMENT_TYPE_CHANGE
com.google.gwt.user.client.ui.Button::Button(Ljava/lang/String;Lcom/google/gwt/user/client/ui/ClickListener;) OVERLOADED_METHOD_CALL
-com.google.gwt.user.client.ui.MultiWordSuggestOracle::addAll(Ljava/util/Collection;) FINAL_ADDED
com.google.gwt.user.client.ui.ToggleButton::ToggleButton(Lcom/google/gwt/user/client/ui/Image;Lcom/google/gwt/user/client/ui/Image;Lcom/google/gwt/user/client/ui/ClickListener;) OVERLOADED_METHOD_CALL
com.google.gwt.user.client.ui.Tree::setImageBase(Ljava/lang/String;) MISSING
diff --git a/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiClass.java b/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiClass.java
index 4e04f5d..616a3f0 100644
--- a/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiClass.java
+++ b/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiClass.java
@@ -225,8 +225,7 @@
JField fields[] = getAccessibleFields();
for (JField field : fields) {
if (isApiMember(field)) {
- apiFields.put(ApiField.computeApiSignature(field), new ApiField(field,
- this));
+ apiFields.put(field.getName(), new ApiField(field, this));
} else {
notAddedFields.add(field.toString());
}
diff --git a/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiClassDiffGenerator.java b/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiClassDiffGenerator.java
index 9a2c5b1..59a65df 100644
--- a/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiClassDiffGenerator.java
+++ b/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiClassDiffGenerator.java
@@ -190,21 +190,57 @@
return collection;
}
- private boolean isIncompatibileDueToMethodOverloading(
+ /**
+ * Attempts to find out if a methodName(null) call previously succeeded, and
+ * would fail with the new Api. Currently, this method is simple.
+ * TODO(amitmanjhi): generalize this method.
+ *
+ * @param methodsInNew Candidate methods in the new Api
+ * @param methodsInExisting Candidate methods in the existing Api.
+ * @return the possible incompatibilities due to method overloading.
+ */
+ private Map<ApiAbstractMethod, ApiChange> getOverloadedMethodIncompatibility(
Set<ApiAbstractMethod> methodsInNew,
Set<ApiAbstractMethod> methodsInExisting) {
if (!ApiCompatibilityChecker.API_SOURCE_COMPATIBILITY
|| methodsInExisting.size() != 1 || methodsInNew.size() <= 1) {
- return false;
+ return Collections.emptyMap();
}
- String signature = methodsInExisting.toArray(new ApiAbstractMethod[0])[0].getCoarseSignature();
- int numMatchingSignature = 0;
+ ApiAbstractMethod existingMethod = methodsInExisting.toArray(new ApiAbstractMethod[0])[0];
+ String signature = existingMethod.getCoarseSignature();
+ List<ApiAbstractMethod> matchingMethods = new ArrayList<ApiAbstractMethod>();
for (ApiAbstractMethod current : methodsInNew) {
if (current.getCoarseSignature().equals(signature)) {
- ++numMatchingSignature;
+ matchingMethods.add(current);
}
}
- return numMatchingSignature > 1;
+ if (isPairwiseCompatible(matchingMethods)) {
+ return Collections.emptyMap();
+ }
+ Map<ApiAbstractMethod, ApiChange> incompatibilities = new HashMap<ApiAbstractMethod, ApiChange>();
+ incompatibilities.put(existingMethod, new ApiChange(existingMethod,
+ ApiChange.Status.OVERLOADED_METHOD_CALL,
+ "Many methods in the new API with similar signatures. Methods = "
+ + methodsInNew + " This might break API source compatibility"));
+ return incompatibilities;
+ }
+
+ /**
+ * @return true if each pair of methods within the list is compatibile.
+ */
+ private boolean isPairwiseCompatible(List<ApiAbstractMethod> methods) {
+ int length = methods.size();
+ for (int i = 0; i < length - 1; i++) {
+ for (int j = i + 1; j < length; j++) {
+ ApiAbstractMethod firstMethod = methods.get(i);
+ ApiAbstractMethod secondMethod = methods.get(j);
+ if (!firstMethod.isCompatible(secondMethod)
+ && !secondMethod.isCompatible(firstMethod)) {
+ return false;
+ }
+ }
+ }
+ return true;
}
/**
@@ -230,12 +266,11 @@
elementName, methodType);
onlyInNew.addAll(methodsInNew);
onlyInExisting.addAll(methodsInExisting);
- if (isIncompatibileDueToMethodOverloading(methodsInNew, methodsInExisting)) {
- ApiAbstractMethod methodInExisting = methodsInExisting.toArray(new ApiAbstractMethod[0])[0];
- addProperty(intersectingElements, methodInExisting, new ApiChange(
- methodInExisting, ApiChange.Status.OVERLOADED_METHOD_CALL,
- "Many methods in the new API with similar signatures. Methods = "
- + methodsInNew + " This might break API source compatibility"));
+ Map<ApiAbstractMethod, ApiChange> incompatibilityMap = getOverloadedMethodIncompatibility(
+ methodsInNew, methodsInExisting);
+ for (ApiAbstractMethod existingMethod : incompatibilityMap.keySet()) {
+ addProperty(intersectingElements, existingMethod,
+ incompatibilityMap.get(existingMethod));
}
/*
diff --git a/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiMethod.java b/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiMethod.java
index d911d3a..1667a2d 100644
--- a/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiMethod.java
+++ b/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiMethod.java
@@ -175,7 +175,8 @@
+ getApiSignature());
}
List<ApiChange.Status> statuses = new ArrayList<ApiChange.Status>();
- if (!oldjmethod.isFinal() && newjmethod.isFinal()) {
+ if (!oldjmethod.isFinal() && !apiClass.getClassObject().isFinal()
+ && newjmethod.isFinal()) {
statuses.add(ApiChange.Status.FINAL_ADDED);
}
if (!oldjmethod.isAbstract() && newjmethod.isAbstract()) {
diff --git a/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityTest.java b/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityTest.java
index 54117a9..5a2357e 100644
--- a/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityTest.java
+++ b/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityTest.java
@@ -38,6 +38,10 @@
* to both ApiClass, apiMethods.
*
* test white-list support.
+ *
+ * TODO(amitmanjhi): (1) Re-factor this code as much as possible into smaller
+ * separate components similar to the ApiCompatibilityUnit class. (2) Use
+ * MockApiElement instead of String comparisons.
*/
public class ApiCompatibilityTest extends TestCase {
@@ -76,8 +80,8 @@
StringBuffer sb = new StringBuffer();
sb.append("package test.apicontainer;\n");
sb.append("class NonApiClass extends java.lang.Object {\n");
- sb.append("\tpublic void methodInNonApiClass(java.lang.Object o) { };\n");
- sb.append("\tpublic void methodInNonApiClass(test.apicontainer.NonApiClass t) { };\n");
+ sb.append("\tpublic void methodInNonApiClass(ApiClassInNonApiClass o) { };\n");
+ sb.append("\tpublic void methodInNonApiClass(NonApiClass t) { };\n");
sb.append("\tpublic int fieldInNonApiClass = 3;\n");
sb.append("\tprotected abstract class ApiClassInNonApiClass {\n");
sb.append("\tprotected ApiClassInNonApiClass() { }\n");
@@ -246,7 +250,7 @@
assertEquals(2, countPresence(finalMethodSignature, strWithoutDuplicates));
// test method_overloading
- finalMethodSignature = "methodInNonApiClass(Ljava/lang/Object;)";
+ finalMethodSignature = "methodInNonApiClass(Ltest/apicontainer/NonApiClass;)";
assertEquals(1, countPresence(finalMethodSignature + delimiter
+ ApiChange.Status.OVERLOADED_METHOD_CALL, strWithoutDuplicates));
@@ -264,9 +268,6 @@
strWithoutDuplicates));
}
- assertEquals(1, countPresence(methodSignature + delimiter
- + ApiChange.Status.OVERLOADED_METHOD_CALL, strWithoutDuplicates));
-
// the method should be satisfied by the method in the super-class
methodSignature = "test.apicontainer.OneMoreApiClass::checkOverloadedMethodAccounted(Ltest/apicontainer/OneMoreApiClass;)";
assertEquals(0, countPresence(methodSignature + delimiter
diff --git a/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityUnitTest.java b/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityUnitTest.java
new file mode 100644
index 0000000..d17a8eb
--- /dev/null
+++ b/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiCompatibilityUnitTest.java
@@ -0,0 +1,300 @@
+/*
+ * Copyright 2008 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.tools.apichecker;
+
+import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.core.ext.typeinfo.NotFoundException;
+import com.google.gwt.dev.javac.CompilationUnit;
+import com.google.gwt.dev.util.log.AbstractTreeLogger;
+import com.google.gwt.dev.util.log.PrintWriterTreeLogger;
+import com.google.gwt.tools.apichecker.ApiCompatibilityChecker.StaticCompilationUnit;
+
+import junit.framework.TestCase;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Base class for all the ApiCompatibility Testing. Encapsulates two api
+ * containers and a test method.
+ *
+ */
+public class ApiCompatibilityUnitTest extends TestCase {
+
+ /**
+ * Mock class to test if the correct ApiChange(s) are being returned.
+ *
+ */
+ static class MockApiElement implements ApiElement {
+
+ final String signature;
+
+ public MockApiElement(String signature) {
+ this.signature = signature;
+ }
+
+ public String getRelativeSignature() {
+ return signature;
+ }
+ }
+
+ private static class FinalKeywordRefactoring extends ApiCompatibilityUnitTest {
+ private static String getFirstApiSourceForObject() {
+ StringBuffer sb = new StringBuffer();
+ sb.append("package java.lang;\n");
+ sb.append("public final class Object {\n");
+ sb.append("\tpublic Object foo;\n");
+ sb.append("\tpublic void bar() {}\n");
+ sb.append("}\n");
+ return sb.toString();
+ }
+
+ private static String getSecondApiSourceForObject() {
+ StringBuffer sb = new StringBuffer();
+ sb.append("package java.lang;\n");
+ sb.append("public class Object {\n");
+ sb.append("\tpublic final Object foo;\n");
+ sb.append("\tpublic final void bar() {}\n");
+ sb.append("}\n");
+ return sb.toString();
+ }
+
+ void testBothWays() throws NotFoundException, UnableToCompleteException {
+ Map<String, String> firstApi = new HashMap<String, String>();
+ firstApi.put("java.lang.Object", getFirstApiSourceForObject());
+ Map<String, String> secondApi = new HashMap<String, String>();
+ secondApi.put("java.lang.Object", getSecondApiSourceForObject());
+
+ // firstApi is the reference Api
+ Collection<ApiChange> apiChanges = getApiChanges(firstApi, secondApi);
+ assertEquals(
+ Arrays.asList(new ApiChange[] {new ApiChange(new MockApiElement(
+ "java.lang.Object::foo"), ApiChange.Status.FINAL_ADDED),}), apiChanges);
+
+ // secondApi is the reference Api
+ apiChanges = getApiChanges(secondApi, firstApi);
+ assertEquals(
+ Arrays.asList(new ApiChange[] {new ApiChange(new MockApiElement(
+ "java.lang.Object"), ApiChange.Status.FINAL_ADDED),}), apiChanges);
+ }
+ }
+
+ /**
+ * Test when method overloading results in Api incompatibilities.
+ * <p>
+ * Imagine a class Foo had a method foo(String ..). If in the new Api, a
+ * method foo(Integer ..) is added, ApiChecker should output a
+ * OVERLOADED_METHOD_CALL Api change (because foo(null) cannot be compiled).
+ * However, if foo(Object ..) is added, it should be okay since JLS matches
+ * from the most specific to the least specific.
+ */
+ private static class MethodOverloadingTest extends ApiCompatibilityUnitTest {
+ private static String getFirstApiSourceForObject() {
+ StringBuffer sb = new StringBuffer();
+ sb.append("package java.lang;\n");
+ sb.append("public class Object {\n");
+ sb.append("\tstatic class Foo extends java.lang.Object {\n");
+ sb.append("\t}\n");
+ sb.append("\tstatic class Bar extends java.lang.Object {\n");
+ sb.append("\t}\n");
+ sb.append("\tpublic void fooObject(Foo x){}\n");
+ sb.append("\tpublic void fooBar(Foo y){}\n");
+ sb.append("}\n");
+ return sb.toString();
+ }
+
+ private static String getSecondApiSourceForObject() {
+ StringBuffer sb = new StringBuffer();
+ sb.append("package java.lang;\n");
+ sb.append("public class Object {\n");
+ sb.append("\tstatic class Foo extends java.lang.Object {\n");
+ sb.append("\t}\n");
+ sb.append("\tstatic class Bar extends java.lang.Object {\n");
+ sb.append("\t}\n");
+ sb.append("\tpublic void fooObject(Foo x){}\n");
+ sb.append("\tpublic void fooObject(Object x){}\n");
+ sb.append("\tpublic void fooBar(Foo y){}\n");
+ sb.append("\tpublic void fooBar(Bar y){}\n");
+ sb.append("}\n");
+ return sb.toString();
+ }
+
+ void testBothWays() throws NotFoundException, UnableToCompleteException {
+ Map<String, String> firstApi = new HashMap<String, String>();
+ firstApi.put("java.lang.Object", getFirstApiSourceForObject());
+ Map<String, String> secondApi = new HashMap<String, String>();
+ secondApi.put("java.lang.Object", getSecondApiSourceForObject());
+
+ // firstApi is the reference Api
+ Collection<ApiChange> apiChanges = getApiChanges(firstApi, secondApi);
+ assertEquals(
+ Arrays.asList(new ApiChange[] {new ApiChange(new MockApiElement(
+ "java.lang.Object::fooBar(Ljava/lang/Object$Foo;)"),
+ ApiChange.Status.OVERLOADED_METHOD_CALL),}), apiChanges);
+
+ // secondApi is the reference Api
+ apiChanges = getApiChanges(secondApi, firstApi);
+ assertEquals(Arrays.asList(new ApiChange[] {
+ new ApiChange(new MockApiElement(
+ "java.lang.Object::fooBar(Ljava/lang/Object$Bar;)"),
+ ApiChange.Status.MISSING),
+ new ApiChange(new MockApiElement(
+ "java.lang.Object::fooObject(Ljava/lang/Object;)"),
+ ApiChange.Status.MISSING),}), apiChanges);
+ }
+ }
+
+ /**
+ * Test whether the ApiChecker correctly identifies moving fields and methods
+ * to a super class.
+ *
+ */
+ private static class SuperClassRefactoring extends ApiCompatibilityUnitTest {
+ private static String getFirstApiSourceForObject() {
+ StringBuffer sb = new StringBuffer();
+ sb.append("package java.lang;\n");
+ sb.append("public class Object {\n");
+ sb.append("\t\tpublic static void staticMethod(){}\n");
+ sb.append("\t\tpublic static int staticField = 1;\n");
+ sb.append("\t\tpublic int instanceField = 2;\n");
+ sb.append("\tpublic static class Foo extends java.lang.Object {\n");
+ sb.append("\t}\n");
+ sb.append("}\n");
+ return sb.toString();
+ }
+
+ private static String getSecondApiSourceForObject() {
+ StringBuffer sb = new StringBuffer();
+ sb.append("package java.lang;\n");
+ sb.append("public class Object {\n");
+ sb.append("\tpublic static class Foo extends java.lang.Object {\n");
+ sb.append("\t\tpublic static void staticMethod(){}\n");
+ sb.append("\t\tpublic static int staticField = 1;\n");
+ sb.append("\t\tpublic int instanceField = 2;\n");
+ sb.append("\t}\n");
+ sb.append("}\n");
+ return sb.toString();
+ }
+
+ void testBothWays() throws NotFoundException, UnableToCompleteException {
+ Map<String, String> firstApi = new HashMap<String, String>();
+ firstApi.put("java.lang.Object", getFirstApiSourceForObject());
+ Map<String, String> secondApi = new HashMap<String, String>();
+ secondApi.put("java.lang.Object", getSecondApiSourceForObject());
+
+ // firstApi is the reference Api
+ Collection<ApiChange> apiChanges = getApiChanges(firstApi, secondApi);
+ assertEquals(Arrays.asList(new ApiChange[] {
+ new ApiChange(new MockApiElement("java.lang.Object::instanceField"),
+ ApiChange.Status.MISSING),
+ new ApiChange(new MockApiElement("java.lang.Object::staticField"),
+ ApiChange.Status.MISSING),
+ new ApiChange(new MockApiElement("java.lang.Object::staticMethod()"),
+ ApiChange.Status.MISSING),}), apiChanges);
+
+ // secondApi is the reference Api
+ apiChanges = getApiChanges(secondApi, firstApi);
+ assertEquals(0, apiChanges.size());
+ }
+ }
+
+ /**
+ * Assert that two ApiChanges are equal.
+ */
+ static void assertEquals(ApiChange apiChange1, ApiChange apiChange2) {
+ assert apiChange1 != null;
+ assert apiChange2 != null;
+ assertEquals(apiChange1.getStatus(), apiChange2.getStatus());
+ assertEquals(apiChange1.getApiElement().getRelativeSignature(),
+ apiChange2.getApiElement().getRelativeSignature());
+ }
+
+ /**
+ * Assert that two sets of ApiChanges are equal.
+ */
+ static void assertEquals(Collection<ApiChange> collection1,
+ Collection<ApiChange> collection2) {
+ assertEquals(collection1.size(), collection2.size());
+
+ List<ApiChange> list1 = new ArrayList<ApiChange>();
+ list1.addAll(collection1);
+ Collections.sort(list1);
+
+ List<ApiChange> list2 = new ArrayList<ApiChange>();
+ list2.addAll(collection2);
+ Collections.sort(list2);
+
+ for (int i = 0; i < list1.size(); i++) {
+ assertEquals(list1.get(i), list2.get(i));
+ }
+ }
+
+ /**
+ * Returns the apiChanges from moving to an existing api to a new Api.
+ *
+ * @param existingTypesToSourcesMap existing Api
+ * @param newTypesToSourcesMap new Api
+ * @return A collection of ApiChange
+ */
+ static Collection<ApiChange> getApiChanges(
+ Map<String, String> existingTypesToSourcesMap,
+ Map<String, String> newTypesToSourcesMap)
+ throws UnableToCompleteException, NotFoundException {
+
+ AbstractTreeLogger logger = new PrintWriterTreeLogger();
+ logger.setMaxDetail(TreeLogger.ERROR);
+
+ Set<CompilationUnit> set1 = new HashSet<CompilationUnit>();
+ for (String type : existingTypesToSourcesMap.keySet()) {
+ set1.add(new StaticCompilationUnit(type,
+ existingTypesToSourcesMap.get(type)));
+ }
+ Set<String> emptyList = Collections.emptySet();
+ Set<CompilationUnit> set2 = new HashSet<CompilationUnit>();
+ for (String type : existingTypesToSourcesMap.keySet()) {
+ set2.add(new StaticCompilationUnit(type, newTypesToSourcesMap.get(type)));
+ }
+
+ ApiContainer existingApi = new ApiContainer("existingApi", set1, emptyList,
+ logger);
+ ApiContainer newApi = new ApiContainer("newApi", set2, emptyList, logger);
+ return ApiCompatibilityChecker.getApiDiff(newApi, existingApi, emptyList);
+ }
+
+ public void testFinalKeywordRefactoring() throws NotFoundException,
+ UnableToCompleteException {
+ new FinalKeywordRefactoring().testBothWays();
+ }
+
+ public void testMethodOverloading() throws NotFoundException,
+ UnableToCompleteException {
+ new MethodOverloadingTest().testBothWays();
+ }
+
+ public void testSuperClassRefactoring() throws NotFoundException,
+ UnableToCompleteException {
+ new SuperClassRefactoring().testBothWays();
+ }
+
+}
diff --git a/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiContainerTest.java b/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiContainerTest.java
index 90deed9..f21a63b 100644
--- a/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiContainerTest.java
+++ b/tools/api-checker/test/com/google/gwt/tools/apichecker/ApiContainerTest.java
@@ -95,7 +95,7 @@
StringBuffer sb = new StringBuffer();
sb.append("package test.apicontainer;\n");
sb.append("class NonApiClass extends java.lang.Object {\n");
- sb.append("\tpublic void methodInNonApiClass(java.lang.Object a) { };\n");
+ sb.append("\tpublic void methodInNonApiClass(NonApiClass a) { };\n");
sb.append("\tpublic int fieldInNonApiClass = 3;\n");
sb.append("\tprotected class ApiClassInNonApiClass {\n");
sb.append("\tprotected ApiClassInNonApiClass() { }\n");
