Revision: 10415
Author: [email protected]
Date: Thu Jun 30 05:59:35 2011
Log: Updates the module name validation check for using CompileModule
on a project.
1) Loosen the requirement that the last part of the module name
be a Java identifier. Users were creating modules with non-java identifer
names but since they weren't top level modules, they were never checked.
Those
modules were then disallowed when presented to the
compiler as top level modules for CompileModule.
2) Perform the loosened name validation check on all modules.
Adds some unit tests on the module naming.
Review at http://gwt-code-reviews.appspot.com/1467810
http://code.google.com/p/google-web-toolkit/source/detail?r=10415
Added:
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/7Foo.gwt.xml
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/Foo-test.gwt.xml
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/Invalid..Foo.gwt.xml
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/Nested7Foo.gwt.xml
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/NestedInvalid.gwt.xml
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/client
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/client/Mock.java
Modified:
/trunk/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
/trunk/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java
/trunk/dev/core/test/com/google/gwt/dev/cfg/ModuleDefLoaderTest.java
/trunk/dev/core/test/com/google/gwt/dev/cfg/ModuleDefTest.java
/trunk/dev/core/test/com/google/gwt/dev/util/UnitTestTreeLogger.java
=======================================
--- /dev/null
+++
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/7Foo.gwt.xml
Thu Jun 30 05:59:35 2011
@@ -0,0 +1,3 @@
+<module>
+ <source path="client" />
+</module>
=======================================
--- /dev/null
+++
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/Foo-test.gwt.xml
Thu Jun 30 05:59:35 2011
@@ -0,0 +1,3 @@
+<module>
+ <source path="client" />
+</module>
=======================================
--- /dev/null
+++
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/Invalid..Foo.gwt.xml
Thu Jun 30 05:59:35 2011
@@ -0,0 +1,3 @@
+<module>
+ <source path="client" />
+</module>
=======================================
--- /dev/null
+++
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/Nested7Foo.gwt.xml
Thu Jun 30 05:59:35 2011
@@ -0,0 +1,3 @@
+<module>
+ <inherits name="com.google.gwt.dev.cfg.testdata.naming.7Foo"/>
+</module>
=======================================
--- /dev/null
+++
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/NestedInvalid.gwt.xml
Thu Jun 30 05:59:35 2011
@@ -0,0 +1,3 @@
+<module>
+ <inherits name="com.google.gwt.dev.cfg.testdata.naming.Invalid..Foo"/>
+</module>
=======================================
--- /dev/null
+++
/trunk/dev/core/test/com/google/gwt/dev/cfg/testdata/naming/client/Mock.java
Thu Jun 30 05:59:35 2011
@@ -0,0 +1,4 @@
+package com.google.gwt.dev.cfg.testdata.naming.client;
+
+public class Mock {
+}
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java Wed Jun 8
16:44:32 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java Thu Jun 30
05:59:35 2011
@@ -78,8 +78,13 @@
};
public static boolean isValidModuleName(String moduleName) {
+ // Check for an empty string between two periods.
+ if (moduleName.contains("..")) {
+ return false;
+ }
+ // Insure the package name components are a valid Java ident.
String[] parts = moduleName.split("\\.");
- for (int i = 0; i < parts.length; i++) {
+ for (int i = 0; i < parts.length - 1; i++) {
String part = parts[i];
if (!Util.isValidJavaIdent(part)) {
return false;
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java Mon Jun
20 09:36:25 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java Thu Jun
30 05:59:35 2011
@@ -239,6 +239,12 @@
+ moduleName + "'", null);
alreadyLoadedModules.add(moduleName);
+ if (!ModuleDef.isValidModuleName(moduleName)) {
+ logger.log(TreeLogger.ERROR, "Invalid module name: '" + moduleName
+ "'",
+ null);
+ throw new UnableToCompleteException();
+ }
+
// Find the specified module using the classpath.
//
String slashedModuleName = moduleName.replace('.', '/');
@@ -308,11 +314,6 @@
*/
private ModuleDef doLoadModule(TreeLogger logger, String moduleName)
throws UnableToCompleteException {
- if (!ModuleDef.isValidModuleName(moduleName)) {
- logger.log(TreeLogger.ERROR, "Invalid module name: '" + moduleName
+ "'",
- null);
- throw new UnableToCompleteException();
- }
ModuleDef moduleDef = new ModuleDef(moduleName);
Event moduleLoadEvent =
SpeedTracerLogger.start(CompilerEventType.MODULE_DEF,
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/cfg/ModuleDefLoaderTest.java
Tue Aug 17 08:14:13 2010
+++ /trunk/dev/core/test/com/google/gwt/dev/cfg/ModuleDefLoaderTest.java
Thu Jun 30 05:59:35 2011
@@ -16,6 +16,8 @@
package com.google.gwt.dev.cfg;
import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.dev.util.UnitTestTreeLogger;
import junit.framework.TestCase;
@@ -52,4 +54,59 @@
assertNull(three.findSourceFile("com/google/gwt/dev/cfg/testdata/merging/client/Toxic.java"));
}
-}
+ /**
+ * The top level module has an invalid name.
+ */
+ public void testModuleNamingInvalid() {
+ UnitTestTreeLogger.Builder builder = new UnitTestTreeLogger.Builder();
+ builder.setLowestLogLevel(TreeLogger.ERROR);
+ builder.expectError("Invalid module
name: 'com.google.gwt.dev.cfg.testdata.naming.Invalid..Foo'", null);
+ UnitTestTreeLogger logger = builder.createLogger();
+ try {
+ ModuleDefLoader.loadFromClassPath(logger,
+ "com.google.gwt.dev.cfg.testdata.naming.Invalid..Foo", false);
+ fail("Expected exception from invalid module name.");
+ } catch (UnableToCompleteException expected) {
+ }
+ logger.assertLogEntriesContainExpected();
+ }
+
+ public void testModuleNamingValid() throws Exception {
+ TreeLogger logger = TreeLogger.NULL;
+ //TreeLogger logger = new PrintWriterTreeLogger();
+
+ ModuleDef module;
+ module = ModuleDefLoader.loadFromClassPath(logger,
+ "com.google.gwt.dev.cfg.testdata.naming.Foo-test", false);
+
assertNotNull(module.findSourceFile("com/google/gwt/dev/cfg/testdata/naming/client/Mock.java"));
+
+ module = ModuleDefLoader.loadFromClassPath(logger,
+ "com.google.gwt.dev.cfg.testdata.naming.7Foo", false);
+
assertNotNull(module.findSourceFile("com/google/gwt/dev/cfg/testdata/naming/client/Mock.java"));
+
+ module = ModuleDefLoader.loadFromClassPath(logger,
+ "com.google.gwt.dev.cfg.testdata.naming.Nested7Foo", false);
+
assertNotNull(module.findSourceFile("com/google/gwt/dev/cfg/testdata/naming/client/Mock.java"));
+
+ module = ModuleDefLoader.loadFromClassPath(logger,
+ "com.google.gwt.dev.cfg.testdata.naming.Nested7Foo", false);
+
assertNotNull(module.findSourceFile("com/google/gwt/dev/cfg/testdata/naming/client/Mock.java"));
+ }
+
+ /**
+ * The top level module has a valid name, but the inherited one does not.
+ */
+ public void testModuleNestedNamingInvalid() {
+ UnitTestTreeLogger.Builder builder = new UnitTestTreeLogger.Builder();
+ builder.setLowestLogLevel(TreeLogger.ERROR);
+ builder.expectError("Invalid module
name: 'com.google.gwt.dev.cfg.testdata.naming.Invalid..Foo'", null);
+ UnitTestTreeLogger logger = builder.createLogger();
+ try {
+ ModuleDefLoader.loadFromClassPath(logger,
+ "com.google.gwt.dev.cfg.testdata.naming.NestedInvalid", false);
+ fail("Expected exception from invalid module name.");
+ } catch (UnableToCompleteException expected) {
+ }
+ logger.assertLogEntriesContainExpected();
+ }
+}
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/cfg/ModuleDefTest.java Thu Mar
25 12:00:47 2010
+++ /trunk/dev/core/test/com/google/gwt/dev/cfg/ModuleDefTest.java Thu Jun
30 05:59:35 2011
@@ -237,4 +237,24 @@
assertEquals(Arrays.asList(expectedClasses),
new ArrayList<Class<? extends Linker>>(def.getActiveLinkers()));
}
-}
+
+ public void testValidModuleName() {
+ // Package names must contain valid Java identifiers.
+ assertFalse(ModuleDef.isValidModuleName("com.foo.."));
+ assertFalse(ModuleDef.isValidModuleName("com..Foo"));
+ assertFalse(ModuleDef.isValidModuleName("com.7.Foo"));
+ assertFalse(ModuleDef.isValidModuleName("com.7foo.Foo"));
+
+ assertTrue(ModuleDef.isValidModuleName("com.foo.Foo"));
+ assertTrue(ModuleDef.isValidModuleName("com.$foo.Foo"));
+ assertTrue(ModuleDef.isValidModuleName("com._foo.Foo"));
+ assertTrue(ModuleDef.isValidModuleName("com.foo7.Foo"));
+
+ // For legacy reasons, allow the last part of the name is not
+ // required to be a valid ident. In the past, naming rules
+ // were enforced for top level modules, but not nested modules.
+ assertTrue(ModuleDef.isValidModuleName("com.foo.F-oo"));
+ assertTrue(ModuleDef.isValidModuleName("com.foo.7Foo"));
+ assertTrue(ModuleDef.isValidModuleName("com.foo.+Foo"));
+ }
+}
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/util/UnitTestTreeLogger.java
Mon Mar 21 05:20:32 2011
+++ /trunk/dev/core/test/com/google/gwt/dev/util/UnitTestTreeLogger.java
Thu Jun 30 05:59:35 2011
@@ -45,8 +45,7 @@
return new UnitTestTreeLogger(expected, loggableTypes);
}
- public void expect(TreeLogger.Type type, String msg,
- Class<? extends Throwable> caught) {
+ public void expect(TreeLogger.Type type, String msg, Class<? extends
Throwable> caught) {
expected.add(new LogEntry(type, msg, caught));
}
@@ -102,8 +101,7 @@
private final String msg;
private final Type type;
- public LogEntry(TreeLogger.Type type, String msg,
- Class<? extends Throwable> caught) {
+ public LogEntry(TreeLogger.Type type, String msg, Class<? extends
Throwable> caught) {
assert (type != null);
this.type = type;
this.msg = msg;
@@ -139,28 +137,37 @@
}
return sb.toString();
}
+
+ private boolean matches(LogEntry other) {
+ if (!type.equals(other.type)) {
+ return false;
+ }
+ if (!msg.equals(other.msg)) {
+ return false;
+ }
+ if ((caught == null) != (other.caught == null)) {
+ return false;
+ }
+ if (caught != null && !caught.isAssignableFrom(other.caught)) {
+ return false;
+ }
+ return true;
+ }
}
private static void assertCorrectLogEntry(LogEntry expected, LogEntry
actual) {
- Assert.assertEquals("Log types do not match", expected.getType(),
- actual.getType());
- Assert.assertEquals("Log messages do not match", expected.getMessage(),
- actual.getMessage());
+ Assert.assertEquals("Log types do not match", expected.getType(),
actual.getType());
+ Assert.assertEquals("Log messages do not match",
expected.getMessage(), actual.getMessage());
if (expected.getCaught() == null) {
- Assert.assertNull("Actual log exception type should have been null",
- actual.getCaught());
+ Assert.assertNull("Actual log exception type should have been null",
actual.getCaught());
} else {
- Assert.assertNotNull(
- "Actual log exception type should not have been null",
- actual.getCaught());
- Assert.assertTrue("Actual log exception type ("
- + actual.getCaught().getName()
+ Assert.assertNotNull("Actual log exception type should not have been
null", actual
+ .getCaught());
+ Assert.assertTrue("Actual log exception type (" +
actual.getCaught().getName()
+ ") cannot be assigned to expected log exception type ("
- + expected.getCaught().getName() + ")",
- expected.getCaught().isAssignableFrom(actual.getCaught()));
- }
- Assert.assertEquals("Log types do not match", expected.getType(),
- actual.getType());
+ + expected.getCaught().getName() + ")",
expected.getCaught().isAssignableFrom(
+ actual.getCaught()));
+ }
}
private final List<LogEntry> actualEntries = new ArrayList<LogEntry>();
@@ -168,20 +175,22 @@
private final EnumSet<TreeLogger.Type> loggableTypes;
- public UnitTestTreeLogger(List<LogEntry> expectedEntries,
- EnumSet<TreeLogger.Type> loggableTypes) {
+ public UnitTestTreeLogger(List<LogEntry> expectedEntries,
EnumSet<TreeLogger.Type> loggableTypes) {
this.expectedEntries.addAll(expectedEntries);
this.loggableTypes = loggableTypes;
// Sanity check that all expected entries are actually loggable.
for (LogEntry entry : expectedEntries) {
Type type = entry.getType();
- Assert.assertTrue("Cannot expect an entry of a non-loggable type!",
- isLoggable(type));
+ Assert.assertTrue("Cannot expect an entry of a non-loggable type!",
isLoggable(type));
loggableTypes.add(type);
}
}
+ /**
+ * Asserts that all expected log entries were logged in the correct
order and
+ * no other entries were logged.
+ */
public void assertCorrectLogEntries() {
if (expectedEntries.size() != actualEntries.size()) {
Assert.fail("Wrong log count: expected=" + expectedEntries + ",
actual=" + actualEntries);
@@ -190,10 +199,28 @@
assertCorrectLogEntry(expectedEntries.get(i), actualEntries.get(i));
}
}
+
+ /**
+ * A more loose check than {@link #assertCorrectLogEntries} that just
checks
+ * to see that the expected log messages are somewhere in the actual
logged
+ * messages.
+ */
+ public void assertLogEntriesContainExpected() {
+ for (LogEntry expectedEntry : expectedEntries) {
+ boolean found = false;
+ for (LogEntry actualEntry : actualEntries) {
+ if (expectedEntry.matches(actualEntry)) {
+ found = true;
+ break;
+ }
+ }
+ Assert.assertTrue("No match for expected=" + expectedEntry + " in
actual=" + actualEntries,
+ found);
+ }
+ }
@Override
- public TreeLogger branch(Type type, String msg, Throwable caught,
- HelpInfo helpInfo) {
+ public TreeLogger branch(Type type, String msg, Throwable caught,
HelpInfo helpInfo) {
log(type, msg, caught);
return this;
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
