Revision: 6932
Author: rj...@google.com
Date: Mon Nov 16 15:06:51 2009
Log: Fixes bad code gen when css class names in a ui:style block have
dashes in them,
by adopting a convention that {foo.bar-baz} == {foo.barBaz}.
Addresses issue 4052, issue 4053
Review by jasonparekh
http://code.google.com/p/google-web-toolkit/source/detail?r=6932
Added:
/trunk/user/src/com/google/gwt/uibinder/attributeparsers/CssNameConverter.java
/trunk/user/test/com/google/gwt/uibinder/attributeparsers/CssNameConverterTest.java
Modified:
/trunk/user/src/com/google/gwt/uibinder/attributeparsers/FieldReferenceConverter.java
/trunk/user/src/com/google/gwt/uibinder/rebind/BundleWriter.java
/trunk/user/src/com/google/gwt/uibinder/rebind/CssResourceWriter.java
/trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
/trunk/user/test/com/google/gwt/uibinder/UiBinderJreSuite.java
/trunk/user/test/com/google/gwt/uibinder/attributeparsers/FieldReferenceConverterTest.java
/trunk/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
=======================================
--- /dev/null
+++
/trunk/user/src/com/google/gwt/uibinder/attributeparsers/CssNameConverter.java
Mon Nov 16 15:06:51 2009
@@ -0,0 +1,81 @@
+/*
+ * 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.uibinder.attributeparsers;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Converts css class names to a form safe to use as a Java identifier.
+ */
+public class CssNameConverter {
+ /**
+ * Thrown by {...@link CssNameConverter#convertSet(Set)} on name collision.
+ */
+ public static class Failure extends Exception {
+ Failure(String message, Object... params) {
+ super(String.format(message, params));
+ }
+ }
+
+ /**
+ * @param className a css class name
+ * @return the same name in a form safe to use as a Java identifier
+ */
+ public String convertName(String className) {
+ String[] bits = className.split("\\-");
+ StringBuilder b = new StringBuilder();
+ for (String bit : bits) {
+ if (b.length() == 0) {
+ b.append(bit);
+ } else {
+ b.append(bit.substring(0, 1).toUpperCase());
+ if (bit.length() > 1) {
+ b.append(bit.substring(1));
+ }
+ }
+ }
+ String converted = b.toString();
+ return converted;
+ }
+
+ /**
+ * @param classNames css class names to convert
+ * @return map of the same class names in a form safe for use as Java
+ * identifiers, with the order of the input set preserved
+ * @throws Failure on collisions due to conversions
+ */
+ public Map<String, String> convertSet(Set<String> classNames)
+ throws Failure {
+ Map<String, String> rawToConverted = new LinkedHashMap<String,
String>();
+ Map<String, String> convertedToRaw = new LinkedHashMap<String,
String>();
+ for (String className : classNames) {
+ String converted = convertName(className);
+ String already = convertedToRaw.get(converted);
+ if (already != null) {
+ throw new Failure("CSS class name collision: \"%s\" and \"%s\"",
already,
+ className);
+ }
+ if (!converted.equals(className)) {
+ convertedToRaw.put(converted, className);
+ }
+ rawToConverted.put(className, converted);
+ }
+ return rawToConverted;
+ }
+
+}
=======================================
--- /dev/null
+++
/trunk/user/test/com/google/gwt/uibinder/attributeparsers/CssNameConverterTest.java
Mon Nov 16 15:06:51 2009
@@ -0,0 +1,110 @@
+/*
+ * 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.uibinder.attributeparsers;
+
+import com.google.gwt.dev.util.collect.Sets;
+
+import junit.framework.TestCase;
+
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Eponymous test class.
+ */
+public class CssNameConverterTest extends TestCase {
+ static class Pair {
+ final String before;
+ final String after;
+
+ Pair(String before, String after) {
+ this.before = before;
+ this.after = after;
+ }
+ }
+
+ private final CssNameConverter converter = new CssNameConverter();
+
+ public void testCollision() {
+ Set<String> in = Sets.create("charlie-delta", "baker", "charlieDelta",
+ "echo");
+
+ try {
+ converter.convertSet(in);
+ } catch (CssNameConverter.Failure e) {
+ assertContains(e.getMessage(), "charlie-delta");
+ assertContains(e.getMessage(), "charlieDelta");
+ }
+ }
+
+ public void testNameConversion() {
+ Pair[] beforeAndAfter = {
+ new Pair("able", "able"), new Pair("-baker-", "baker"),
+ new Pair("charlie-delta", "charlieDelta"), new
Pair("echo", "echo"),
+ new Pair("foxtrot-Tango", "foxtrotTango")};
+
+ for (Pair pair : beforeAndAfter) {
+ assertEquals(pair.after, converter.convertName(pair.before));
+ }
+ }
+
+ public void testNoOp() throws CssNameConverter.Failure {
+ Set<String> in = Sets.create("able", "baker", "charlieDelta", "echo");
+ Map<String, String> out = converter.convertSet(in);
+
+ for (Map.Entry<String, String> entry : out.entrySet()) {
+ String key = entry.getKey();
+ assertTrue(in.remove(key));
+ assertEquals(key, entry.getValue());
+ }
+ assertTrue(in.isEmpty());
+ }
+
+ public void testReverseCollision() {
+ Set<String> in = Sets.create("charlieDelta", "baker", "charlie-delta",
+ "echo");
+
+ try {
+ converter.convertSet(in);
+ } catch (CssNameConverter.Failure e) {
+ assertContains(e.getMessage(), "charlie-delta");
+ assertContains(e.getMessage(), "charlieDelta");
+ }
+ }
+
+ public void testSomeOp() throws CssNameConverter.Failure {
+ Set<String> in =
Sets.create("able", "-baker-", "charlie-delta", "echo",
+ "foxtrot-Tango");
+ Pair[] expected = {
+ new Pair("able", "able"), new Pair("-baker-", "baker"),
+ new Pair("charlie-delta", "charlieDelta"), new
Pair("echo", "echo"),
+ new Pair("foxtrot-Tango", "foxtrotTango")};
+
+ Map<String, String> out = converter.convertSet(in);
+
+ for (Pair pair : expected) {
+ String convert = out.remove(pair.before);
+ assertEquals(pair.after, convert);
+ }
+
+ assertTrue(out.isEmpty());
+ }
+
+ private void assertContains(String string, String fragment) {
+ assertTrue(String.format("%s should contain %s", string, fragment),
+ string.contains(fragment));
+ }
+}
=======================================
---
/trunk/user/src/com/google/gwt/uibinder/attributeparsers/FieldReferenceConverter.java
Wed Nov 11 22:08:47 2009
+++
/trunk/user/src/com/google/gwt/uibinder/attributeparsers/FieldReferenceConverter.java
Mon Nov 16 15:06:51 2009
@@ -34,6 +34,10 @@
* A field reference starts with '{' and is followed immediately by a
character
* that can legally start a java identifier—that is a letter, $, or
* underscore. Braces not followed by such a character are left in place.
+ * <p>
+ * For convenience when dealing with generated CssResources, field
segments with
+ * dashes are converted to camel case. That is, {able.baker-charlie} is
the same
+ * as {able.bakerCharlie}
* <p>
* Opening braces may be escape by doubling them. That is, "{{foo}" will
* converted to "{foo}", with no field reference detected.
@@ -45,32 +49,6 @@
@SuppressWarnings("serial")
public static class IllegalFieldReferenceException extends
RuntimeException {
}
-
- /**
- * Used by {...@link #hasFieldReferences}. Passthrough implementation that
notes
- * when handleReference has been called.
- */
- private static final class Telltale implements
- FieldReferenceConverter.Delegate {
- boolean hasComputed = false;
-
- public JType getType() {
- return null;
- }
-
- public String handleFragment(String fragment) {
- return fragment;
- }
-
- public String handleReference(String reference) {
- hasComputed = true;
- return reference;
- }
-
- public boolean hasComputed() {
- return hasComputed;
- }
- }
/**
* Responsible for the bits around and between the field references. May
throw
@@ -103,10 +81,36 @@
throws IllegalFieldReferenceException;
}
- private static final Pattern BRACES = Pattern.compile("[{]([^}]*)[}]");
-
+ /**
+ * Used by {...@link #hasFieldReferences}. Passthrough implementation that
notes
+ * when handleReference has been called.
+ */
+ private static final class Telltale implements
+ FieldReferenceConverter.Delegate {
+ boolean hasComputed = false;
+
+ public JType getType() {
+ return null;
+ }
+
+ public String handleFragment(String fragment) {
+ return fragment;
+ }
+
+ public String handleReference(String reference) {
+ hasComputed = true;
+ return reference;
+ }
+
+ public boolean hasComputed() {
+ return hasComputed;
+ }
+ }
+
+ private static final Pattern BRACES = Pattern.compile("[{]([^}]*)[}]");
private static final Pattern LEGAL_FIRST_CHAR =
Pattern.compile("^[$_a-zA-Z].*");
+
/**
* @return true if the given string holds one or more field references
*/
@@ -116,6 +120,7 @@
return telltale.hasComputed();
}
+ private final CssNameConverter cssConverter = new CssNameConverter();
private final FieldManager fieldManager;
/**
@@ -162,6 +167,7 @@
String[] segments = value.split("[.]");
for (String segment : segments) {
+ segment = cssConverter.convertName(segment);
if (b.length() == 0) {
b.append(segment); // field name
} else {
=======================================
--- /trunk/user/src/com/google/gwt/uibinder/rebind/BundleWriter.java Wed
Oct 28 09:10:53 2009
+++ /trunk/user/src/com/google/gwt/uibinder/rebind/BundleWriter.java Mon
Nov 16 15:06:51 2009
@@ -40,7 +40,8 @@
private final ImplicitClientBundle bundleClass;
private final IndentedWriter writer;
private final PrintWriterManager writerManager;
- private final TypeOracle oracle;
+ private final TypeOracle types;
+ private final MortalLogger logger;
private final JClassType clientBundleType;
private final JClassType dataResourceType;
@@ -50,27 +51,28 @@
private final JClassType importAnnotationType;
public BundleWriter(ImplicitClientBundle bundleClass,
- PrintWriterManager writerManager, TypeOracle oracle,
- PrintWriterManager printWriterProvider) {
+ PrintWriterManager writerManager, TypeOracle types, MortalLogger
logger) {
this.bundleClass = bundleClass;
this.writer = new IndentedWriter(
writerManager.makePrintWriterFor(bundleClass.getClassName()));
this.writerManager = writerManager;
- this.oracle = oracle;
-
- clientBundleType = oracle.findType(ClientBundle.class.getName());
- dataResourceType =
oracle.findType(DataResource.class.getCanonicalName());
- imageOptionType =
oracle.findType(ImageOptions.class.getCanonicalName());
- imageResourceType =
oracle.findType(ImageResource.class.getCanonicalName());
- repeatStyleType =
oracle.findType(RepeatStyle.class.getCanonicalName());
- importAnnotationType =
oracle.findType(Import.class.getCanonicalName());
+ this.types = types;
+ this.logger = logger;
+
+ clientBundleType = types.findType(ClientBundle.class.getName());
+ dataResourceType =
types.findType(DataResource.class.getCanonicalName());
+ imageOptionType =
types.findType(ImageOptions.class.getCanonicalName());
+ imageResourceType =
types.findType(ImageResource.class.getCanonicalName());
+ repeatStyleType = types.findType(RepeatStyle.class.getCanonicalName());
+ importAnnotationType = types.findType(Import.class.getCanonicalName());
}
public void write() throws UnableToCompleteException {
writeBundleClass();
for (ImplicitCssResource css : bundleClass.getCssMethods()) {
- new CssResourceWriter(css, oracle,
- writerManager.makePrintWriterFor(css.getClassName())).write();
+ new CssResourceWriter(css, types,
+ writerManager.makePrintWriterFor(css.getClassName()),
+ logger).write();
}
}
@@ -94,7 +96,7 @@
writer.write("public interface %s extends ClientBundle {",
bundleClass.getClassName());
writer.indent();
-
+
// Write css methods
for (ImplicitCssResource css : bundleClass.getCssMethods()) {
writeCssSource(css);
@@ -102,7 +104,7 @@
writer.write("%s %s();", css.getClassName(), css.getName());
writer.newline();
}
-
+
// Write data methods
for (ImplicitDataResource data : bundleClass.getDataMethods()) {
writer.write("@Source(\"%s\")", data.getSource());
=======================================
--- /trunk/user/src/com/google/gwt/uibinder/rebind/CssResourceWriter.java
Wed Oct 28 09:10:53 2009
+++ /trunk/user/src/com/google/gwt/uibinder/rebind/CssResourceWriter.java
Mon Nov 16 15:06:51 2009
@@ -1,12 +1,12 @@
/*
* 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
@@ -21,30 +21,33 @@
import com.google.gwt.core.ext.typeinfo.JType;
import com.google.gwt.core.ext.typeinfo.TypeOracle;
import com.google.gwt.resources.client.CssResource;
+import com.google.gwt.uibinder.attributeparsers.CssNameConverter;
import com.google.gwt.uibinder.rebind.model.ImplicitCssResource;
import java.io.PrintWriter;
+import java.util.Map;
import java.util.Set;
/**
* Writes the source to implement an {...@link ImplicitCssResource} interface.
*/
public class CssResourceWriter {
- /**
- *
- */
private static final JType[] NO_PARAMS = new JType[0];
private final ImplicitCssResource css;
private final IndentedWriter writer;
private final JClassType cssResourceType;
private final JClassType stringType;
-
- public CssResourceWriter(ImplicitCssResource css, TypeOracle oracle,
- PrintWriter writer) {
+ private final CssNameConverter nameConverter;
+ private final MortalLogger logger;
+
+ public CssResourceWriter(ImplicitCssResource css, TypeOracle types,
+ PrintWriter writer, MortalLogger logger) {
this.css = css;
this.writer = new IndentedWriter(writer);
- this.cssResourceType = oracle.findType(CssResource.class.getName());
- this.stringType = oracle.findType(String.class.getName());
+ this.cssResourceType = types.findType(CssResource.class.getName());
+ this.stringType = types.findType(String.class.getName());
+ this.nameConverter = new CssNameConverter();
+ this.logger = logger;
}
public void write() throws UnableToCompleteException {
@@ -75,16 +78,35 @@
writer.write("}");
}
- private void writeCssMethods(JClassType superType) throws
UnableToCompleteException {
- Set<String> classNames = css.getCssClassNames();
-
- /*
- * Only write names that we are not overriding from super, or else
we'll
- * re-obfuscate any @Shared ones
- */
- for (String className : classNames) {
- JMethod method = superType.findMethod(className, NO_PARAMS);
- if (method == null || !stringType.equals(method.getReturnType())) {
+ private boolean isOverride(String methodName, JClassType superType) {
+ JMethod method = superType.findMethod(methodName, NO_PARAMS);
+ if (method != null && stringType.equals(method.getReturnType())) {
+ return true;
+ }
+ return false;
+ }
+
+ private void writeCssMethods(JClassType superType)
+ throws UnableToCompleteException {
+ Set<String> rawClassNames = css.getCssClassNames();
+ Map<String, String> convertedClassNames = null;
+
+ try {
+ convertedClassNames = nameConverter.convertSet(rawClassNames);
+ } catch (CssNameConverter.Failure e) {
+ logger.die(e.getMessage());
+ }
+
+ for (Map.Entry<String, String> entry : convertedClassNames.entrySet())
{
+ String className = entry.getValue();
+ /*
+ * Only write names that we are not overriding from super, or else
we'll
+ * re-obfuscate any @Shared ones
+ */
+ if (!isOverride(className, superType)) {
+ if (!rawClassNames.contains(className)) {
+ writer.write("@ClassName(\"%s\")", entry.getKey());
+ }
writer.write("String %s();", className);
}
}
=======================================
--- /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
Wed Nov 11 22:08:47 2009
+++ /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
Mon Nov 16 15:06:51 2009
@@ -130,7 +130,7 @@
}
ImplicitClientBundle bundleClass = uiBinderWriter.getBundleClass();
- new BundleWriter(bundleClass, writerManager, oracle,
writerManager).write();
+ new BundleWriter(bundleClass, writerManager, oracle, logger).write();
writerManager.commit();
}
=======================================
--- /trunk/user/test/com/google/gwt/uibinder/UiBinderJreSuite.java Tue Nov
10 12:33:30 2009
+++ /trunk/user/test/com/google/gwt/uibinder/UiBinderJreSuite.java Mon Nov
16 15:06:51 2009
@@ -15,6 +15,7 @@
*/
package com.google.gwt.uibinder;
+import com.google.gwt.uibinder.attributeparsers.CssNameConverterTest;
import
com.google.gwt.uibinder.attributeparsers.FieldReferenceConverterTest;
import com.google.gwt.uibinder.attributeparsers.IntAttributeParserTest;
import com.google.gwt.uibinder.attributeparsers.StrictAttributeParserTest;
@@ -55,6 +56,7 @@
suite.addTestSuite(OwnerFieldTest.class);
// attributeparsers
+ suite.addTestSuite(CssNameConverterTest.class);
suite.addTestSuite(IntAttributeParserTest.class);
suite.addTestSuite(FieldReferenceConverterTest.class);
suite.addTestSuite(StrictAttributeParserTest.class);
=======================================
---
/trunk/user/test/com/google/gwt/uibinder/attributeparsers/FieldReferenceConverterTest.java
Wed Nov 11 22:08:47 2009
+++
/trunk/user/test/com/google/gwt/uibinder/attributeparsers/FieldReferenceConverterTest.java
Mon Nov 16 15:06:51 2009
@@ -52,6 +52,13 @@
assertEquals(expected, converter.convert(before, frDelegate));
}
+ public void testOne() {
+ String before = "{baker}";
+ String expected = "** & baker & **";
+
+ assertEquals(expected, converter.convert(before, frDelegate));
+ }
+
public void testReplaceSimple() {
String before = "able {baker} charlie";
String expected = "*able * & baker & * charlie*";
@@ -59,13 +66,20 @@
assertEquals(expected, converter.convert(before, frDelegate));
}
+ public void testDashes() {
+ String before = "{foo-bar.baz-bangZoom.zip-zap}";
+ String expected = "** & fooBar.bazBangZoom().zipZap() & **";
+
+ assertEquals(expected, converter.convert(before, frDelegate));
+ }
+
public void testReplaceSeveral() {
String before = "{foo.bar.baz} baker {bang.zoom} delta {zap}";
String expected = "** & foo.bar().baz() & * baker * & bang.zoom() & *
delta * & zap & **";
assertEquals(expected, converter.convert(before, frDelegate));
}
-
+
public void testEscaping() {
String before = "Well {{Hi mom}!";
String expected = "*Well {Hi mom}!*";
=======================================
---
/trunk/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
Mon Nov 16 10:04:26 2009
+++
/trunk/user/test/com/google/gwt/uibinder/test/client/WidgetBasedUi.ui.xml
Mon Nov 16 15:06:51 2009
@@ -100,7 +100,7 @@
<ui:style field='myTotallyPrivateStyle'>
/* An inline style tied to no public type */
- .superPrivateColor {
+ .super-private-color {
background-color: Gold;
}
</ui:style>
@@ -422,7 +422,7 @@
</p>
<p ui:field='reallyPrivateStyleParagraph'
class='{myOtherStyle.privateColor}'>
And this style is defined right here inside the ui.xml file.
- <span ui:field='totallyPrivateStyleSpan'
class='{myTotallyPrivateStyle.superPrivateColor}'>
+ <span ui:field='totallyPrivateStyleSpan'
class='{myTotallyPrivateStyle.super-private-color}'>
(This one too, but even more so.)
</span>
</p>
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
