Revision: 6356 Author: [email protected] Date: Tue Oct 13 10:44:52 2009 Log: Stricter XMLElement#consumeSingleChild, fixes NPE in DockLayoutPanelParser. Also improves testing of XMLElement
Reviewed by jgw http://code.google.com/p/google-web-toolkit/source/detail?r=6356 Modified: /trunk/user/src/com/google/gwt/uibinder/parsers/CellPanelParser.java /trunk/user/src/com/google/gwt/uibinder/parsers/DockPanelParser.java /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java /trunk/user/src/com/google/gwt/uibinder/rebind/XMLElement.java /trunk/user/test/com/google/gwt/uibinder/rebind/XMLElementTest.java ======================================= --- /trunk/user/src/com/google/gwt/uibinder/parsers/CellPanelParser.java Tue Sep 1 15:14:29 2009 +++ /trunk/user/src/com/google/gwt/uibinder/parsers/CellPanelParser.java Tue Oct 13 10:44:52 2009 @@ -87,9 +87,6 @@ if (ns.equals(elem.getNamespaceUri()) && tagName.equals(CELL_TAG)) { // It's a cell element, so parse its single child as a widget. XMLElement widget = child.consumeSingleChildElement(); - if (widget == null) { - writer.die("Cell must contain a single child widget"); - } String childFieldName = writer.parseElementToField(widget); writer.addStatement("%1$s.add(%2$s);", fieldName, childFieldName); ======================================= --- /trunk/user/src/com/google/gwt/uibinder/parsers/DockPanelParser.java Thu Oct 8 18:15:53 2009 +++ /trunk/user/src/com/google/gwt/uibinder/parsers/DockPanelParser.java Tue Oct 13 10:44:52 2009 @@ -68,9 +68,6 @@ // And they can only have a single child widget. XMLElement widget = child.consumeSingleChildElement(); - if (widget == null) { - writer.die("Dock must contain a single child widget."); - } String childFieldName = writer.parseElementToField(widget); writer.addStatement("%1$s.add(%2$s, %3$s);", fieldName, childFieldName, translated); ======================================= --- /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java Wed Oct 7 13:08:06 2009 +++ /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java Tue Oct 13 10:44:52 2009 @@ -1,12 +1,12 @@ /* * 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 @@ -21,6 +21,7 @@ import com.google.gwt.core.ext.typeinfo.JPackage; import com.google.gwt.core.ext.typeinfo.JParameter; import com.google.gwt.core.ext.typeinfo.TypeOracle; +import com.google.gwt.dev.shell.log.TreeItemLogger; import com.google.gwt.dom.client.TagName; import com.google.gwt.uibinder.client.UiBinder; import com.google.gwt.uibinder.parsers.AttributeMessageParser; @@ -60,10 +61,10 @@ /** * Writer for UiBinder generated classes. - * + * * TODO(rdamazio): Refactor this, extract model classes, improve ordering * guarantees, etc. - * + * * TODO(rjrjr): Improve error messages */ @SuppressWarnings("deprecation") @@ -160,7 +161,7 @@ /** * Returns a list of the given type and all its superclasses and implemented * interfaces in a breadth-first traversal. - * + * * @param type the base type * @return a breadth-first collection of its type hierarchy */ @@ -193,7 +194,7 @@ /** * Class names of parsers for values of attributes with no namespace prefix, * keyed by method parameter signatures. - * + * * TODO(rjrjr) Seems like the attribute parsers belong in BeanParser, which is * the only thing that uses them. */ @@ -281,12 +282,31 @@ handlerEvaluator = new HandlerEvaluator(ownerClass, logger, oracle); fieldManager = new FieldManager(logger); } + + /** + * Hack for testing. Die method works, nothing else does + */ + UiBinderWriter() { + this.baseClass = null; + this.implClassName = null; + this.oracle = null; + this.logger = new MortalLogger(new TreeItemLogger()); + this.templatePath = null; + this.messages = null; + uiRootType = null; + uiOwnerType = null; + + ownerClass = null; + bundleClass = null; + handlerEvaluator = null; + fieldManager = null; + } /** * Statements to be excuted right after the current attached element is * detached. This is useful for doing things that might be expensive while the * element is attached to the DOM. - * + * * @param format * @param args */ @@ -313,7 +333,7 @@ /** * Begin a section where a new attachable element is being parsed. Note that * attachment is only done when actually needed. - * + * * @param element to be attached for this section */ public void beginAttachedSection(String element) { @@ -330,7 +350,7 @@ * generate a unique dom id at runtime. Further code will be generated to be * run after widgets are instantiated, to use that dom id in a getElementById * call and assign the Element instance to its field. - * + * * @param fieldName The name of the field being declared * @param parentElementExpression an expression to be evaluated at runtime, * which will return an Element that is an ancestor of this one @@ -351,7 +371,7 @@ /** * Declare a variable that will be filled at runtime with a unique id, safe * for use as a dom element's id attribute. - * + * * @return that variable's name. */ public String declareDomIdHolder() throws UnableToCompleteException { @@ -391,7 +411,7 @@ * If this element has a gwt:field attribute, create a field for it of the * appropriate type, and return the field name. If no gwt:field attribute is * found, do nothing and return null - * + * * @return The new field name, or null if no field is created */ public String declareFieldIfNeeded(XMLElement elem) @@ -449,7 +469,7 @@ /** * Ensure that the specified element is attached to the DOM. - * + * * @param element variable name of element to be attached */ public void ensureAttached(String element) { @@ -466,7 +486,7 @@ /** * Ensure that the specified field is attached to the DOM. - * + * * @param field variable name of the field to be attached */ public void ensureFieldAttached(String field) { @@ -476,7 +496,7 @@ /** * Finds the JClassType that corresponds to this XMLElement, which must be a * Widget or an Element. - * + * * @throws UnableToCompleteException If no such widget class exists * @throws RuntimeException if asked to handle a non-widget, non-DOM element */ @@ -561,7 +581,7 @@ /** * Finds an attribute {...@link BundleAttributeParser} for the given xml * attribute, if any, based on its namespace uri. - * + * * @return the parser or null * @deprecated exists only to support {...@link BundleAttributeParser}, which * will be leaving us soon. @@ -641,7 +661,7 @@ * name of the field (possibly private) that will hold it. The element is * likely to make recursive calls back to this method to have its children * parsed. - * + * * @param elem the xml element to be parsed * @return the name of the field containing the parsed widget */ @@ -675,7 +695,7 @@ /** * Gives the writer the initializer to use for this field instead of the * default GWT.create call. - * + * * @throws IllegalStateException if an initializer has already been set */ public void setFieldInitializer(String fieldName, String factoryMethod) { @@ -699,7 +719,7 @@ * token, surrounded by plus signs. This is useful in strings to be handed to * setInnerHTML() and setText() calls, to allow a unique dom id attribute or * other runtime expression in the string. - * + * * @param expression */ public String tokenForExpression(String expression) { @@ -823,7 +843,7 @@ /** * Ensures that all of the internal data structures are cleaned up correctly * at the end of parsing the document. - * + * * @throws UnableToCompleteException */ private void ensureAttachmentCleanedUp() throws UnableToCompleteException { @@ -859,8 +879,8 @@ } /** - * Use this method to format code. It forces the use of the en-US locale, - * so that things like decimal format don't get mangled. + * Use this method to format code. It forces the use of the en-US locale, so + * that things like decimal format don't get mangled. */ private String formatCode(String format, Object... params) { String r = String.format(Locale.US, format, params); @@ -870,7 +890,7 @@ /** * Inspects this element for a gwt:field attribute. If one is found, the * attribute is consumed and its value returned. - * + * * @return The field name declared by an element, or null if none is declared */ private String getFieldName(XMLElement elem) throws UnableToCompleteException { @@ -927,7 +947,7 @@ /** * Find a set of element parsers for the given ui type. - * + * * The list of parsers will be returned in order from most- to least-specific. */ private Iterable<ElementParser> getParsersForClass(JClassType type) { @@ -936,7 +956,7 @@ /* * Let this non-widget parser go first (it finds <m:attribute/> elements). * Any other such should land here too. - * + * * TODO(rjrjr) Need a scheme to associate these with a namespace uri or * something? */ @@ -1015,7 +1035,7 @@ /** * Parses a package uri (i.e. package://com.google...). - * + * * @throws UnableToCompleteException on bad package name */ private JPackage parseNamespacePackage(String ns) @@ -1190,7 +1210,7 @@ * gwt:field in the template. For those that have not had constructor * generation suppressed, emit GWT.create() calls instantiating them (or die * if they have no default constructor). - * + * * @throws UnableToCompleteException on constructor problem */ private void writeGwtFields(IndentedWriter niceWriter) ======================================= --- /trunk/user/src/com/google/gwt/uibinder/rebind/XMLElement.java Fri Oct 9 14:47:58 2009 +++ /trunk/user/src/com/google/gwt/uibinder/rebind/XMLElement.java Tue Oct 13 10:44:52 2009 @@ -382,9 +382,9 @@ /** * Consumes a single child element, ignoring any text nodes and throwing an - * exception if more than one child element is found. + * exception if no child is found, or more than one child element is found. * - * @throws UnableToCompleteException + * @throws UnableToCompleteException on no children, or too many */ public XMLElement consumeSingleChildElement() throws UnableToCompleteException { @@ -397,6 +397,10 @@ ret = child; } + + if (ret == null) { + writer.die("%s must have a single child element", this); + } return ret; } ======================================= --- /trunk/user/test/com/google/gwt/uibinder/rebind/XMLElementTest.java Fri Oct 9 14:47:58 2009 +++ /trunk/user/test/com/google/gwt/uibinder/rebind/XMLElementTest.java Tue Oct 13 10:44:52 2009 @@ -36,74 +36,100 @@ * Tests XMLElement. */ public class XMLElementTest extends TestCase { - /** - * - */ private static final String STRING_WITH_DOUBLEQUOTE = "I have a \" quote in me"; - private static final String dom = - "<doc><elm attr1=\"attr1Value\" attr2=\"attr2Value\"/></doc>"; private Document doc; private Element item; private XMLElement elm; - + @Override protected void setUp() throws Exception { super.setUp(); - doc = UiBinderTesting.documentForString(dom); - item = (Element) doc.getDocumentElement().getElementsByTagName( - "elm").item(0); - elm = new XMLElement(item, null); - + init("<doc><elm attr1=\"attr1Value\" attr2=\"attr2Value\"/></doc>"); } public void testConsumeAttribute() { assertEquals("attr1Value", elm.consumeAttribute("attr1")); assertEquals("", elm.consumeAttribute("attr1")); } - + public void testConsumeAttributeWithDefault() { assertEquals("attr1Value", elm.consumeAttribute("attr1", "default")); assertEquals("default", elm.consumeAttribute("attr1", "default")); - assertEquals("otherDefault", elm.consumeAttribute("unsetthing", "otherDefault")); - } - + assertEquals("otherDefault", elm.consumeAttribute("unsetthing", + "otherDefault")); + } + public void testConsumeRequired() throws UnableToCompleteException { assertEquals("attr1Value", elm.consumeRequiredAttribute("attr1")); - - // TODO(rjrjr) Can't test this until UiBinderWriter can be mocked -// try { -// elm.consumeRequiredAttribute("unsetthing"); -// fail("Should have thrown UnableToCompleteException"); -// } catch (UnableToCompleteException e) { -// /* pass */ -// } - } - - public void testConsumeInnerTextEscapedAsHtmlStringLiteral() throws UnableToCompleteException { + try { + elm.consumeRequiredAttribute("unsetthing"); + fail("Should have thrown UnableToCompleteException"); + } catch (UnableToCompleteException e) { + /* pass */ + } + } + + public void testConsumeInnerTextEscapedAsHtmlStringLiteral() + throws UnableToCompleteException { appendText(STRING_WITH_DOUBLEQUOTE); - assertEquals(UiBinderWriter.escapeTextForJavaStringLiteral(STRING_WITH_DOUBLEQUOTE), + assertEquals( + UiBinderWriter.escapeTextForJavaStringLiteral(STRING_WITH_DOUBLEQUOTE), elm.consumeInnerTextEscapedAsHtmlStringLiteral(new NullInterpreter<String>())); } - - public void testConsumeInnerTextEscapedAsHtmlStringLiteralEmpty() throws UnableToCompleteException { - assertEquals("", + + public void testConsumeInnerTextEscapedAsHtmlStringLiteralEmpty() + throws UnableToCompleteException { + assertEquals( + "", elm.consumeInnerTextEscapedAsHtmlStringLiteral(new NullInterpreter<String>())); } - + + public void testConsumeSingleChildElementEmpty() + throws ParserConfigurationException, SAXException, IOException, + UnableToCompleteException { + try { + elm.consumeSingleChildElement(); + fail("Should throw on single child element"); + } catch (UnableToCompleteException e) { + /* pass */ + } + + init("<doc><elm><child>Hi.</child></elm></doc>"); + assertEquals("Hi.", + elm.consumeSingleChildElement().consumeUnescapedInnerText()); + + init("<doc><elm id='elm'><child>Hi.</child><child>Ho.</child></elm></doc>"); + try { + elm.consumeSingleChildElement(); + fail("Should throw on too many children"); + } catch (UnableToCompleteException e) { + /* pass */ + } + } + + private void init(final String domString) + throws ParserConfigurationException, SAXException, IOException { + doc = UiBinderTesting.documentForString(domString); + item = (Element) doc.getDocumentElement().getElementsByTagName("elm").item( + 0); + elm = new XMLElement(item, new UiBinderWriter()); + } + private void appendText(final String text) { Text t = doc.createTextNode(STRING_WITH_DOUBLEQUOTE); item.appendChild(t); } - + public void testConsumeUnescapedInnerText() throws UnableToCompleteException { appendText(STRING_WITH_DOUBLEQUOTE); assertEquals(STRING_WITH_DOUBLEQUOTE, elm.consumeUnescapedInnerText()); } - - public void testConsumeUnescapedInnerTextEmpty() throws UnableToCompleteException { + + public void testConsumeUnescapedInnerTextEmpty() + throws UnableToCompleteException { assertEquals("", elm.consumeUnescapedInnerText()); } - + public void testEmptyStringOnMissingAttribute() throws ParserConfigurationException, SAXException, IOException { assertEquals("", elm.consumeAttribute("fnord")); --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
