This looks great. My only comment is that rather than using $tag$ as the prefix for the non-LZX classes, perhaps use something more descriptive, say $lfc$ (indicating that it is an internal LFC class.
On 2011-04-22, at 11:50, Donald Anderson wrote: > Tucker, I'm resending this review after 'svn up'. Do you have time to review > this? > - Don > > Change dda-20110407-qa4 by [email protected] on 2011-04-07 09:46:30 EDT > in /Users/dda/laszlo/src/svn/openlaszlo/trunk-c > for http://svn.openlaszlo.org/openlaszlo/trunk > > Summary: Emit reference classes for all LFC classes that do not have tags > > New Features: > > Bugs Fixed: LPP-9873 : with(this) emitted for in class extending LzEventable > > Technical Reviewer: ptw (pending) > QA Reviewer: (pending) > Doc Reviewer: (pending) > > Documentation: > > Release Notes: > > Overview: > In previous commits, the issue of having reference classes for LFC > classes that were not associated with tags was deferred. This > commit fixes that deficiency. The primary mechanism is for the > js2doc 'schemabuilder' to create dummy lzx tags for any public LFC > javascript class that does not have an lzx tag. For example > LzEventable is given tag "$tag$LzEventable". A tag name starting > with '$' is illegal to use in XML, so it amounts to a private > name, used internally only to identify inheritance. This approach > firmly establishes a 1-1 relationship between tags and Javascript > classes, and allows these classes to be emitted as reference classes > while keeping the tag compiler clean. > > The end result is that: > - methods of classes written in script that extend (previously) > non-tagged classes, such as LzEventable, will now be > considered for 'with(this) removal' > - The LFC class heirarchy is now completely known, except for > method arg types, and can be used for other purposes: helping > to identify globals, helping to evaluate 'is' expressions, etc. > > Details: > test/lztest/lztest-class-impl.lzx > - added test case that shows the error in LPP-9873. > If a with(this) is generated for that case, > the 'pragma errorWithThis' results in a compile error > (that behavior was tested too). > - Added 'pragma errorWithThis' to several existing tests to > ensure that with(this) is not emitted as a regression > in the future. > > test/smoke/compiler.lzl > - disabled some test functions that now fail given the 'complete > knowledge'. > For these, with(this) removal is now errantly done, even though a new > transformation is needed, see > http://jira.openlaszlo.org/jira/browse/LPP-9822 . > - added tests for Aliasing using a non-LFC class to parallel tests > that use > an LFC class. These remain partially disabled due to LPP-9822 . > > WEB-INF/lps/schema/lfc-undeclared.lzx > - Removed 'setAttribute' and 'ondestroy' from the list of > added methods/events for <node>. These are from LzEventable > (which node inherits from), and had been inserted previously > to cover for this class missing from the reference class hierarchy. > > WEB-INF/lps/server/src/org/openlaszlo/js2doc/SchemaBuilder.java > - For classes that do not have an lzxname, the class name > prefixed by "$tag$" is used. > - Guaranteeing that all emitted classes have an lzxname cleans > up some code. > > WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassCompiler.java > - Removed special case code for a class with null tagName and non-null > jsname. > - Changed addClassModel() signature to pass through the jsname, if it > exists, > into the ClassModel. > > WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewCompiler.java > WEB-INF/lps/server/src/org/openlaszlo/compiler/TypeCompiler.java > WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewSchema.java > WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java > - Changed addClassModel() signature to pass through the jsname, if it > exists, > into the ClassModel. > > WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassModel.java > - className is now a constructor arg, if it exists, it will be the JS > name. > > > Tests: > - smoke, lztest, runlzunit > (lztest incorporates a test that shows the original problem) > > - Examined .js produced in test/lztest, all but 20 with(this) occurrences > have gone away (there were about 100 before, most attributable to > inheritance from LzEventable). Of the remaining 20, 1 of these is > an intentional test case that uses with(somevariable), the others > result from an explicit with(this) used in: > lps/components/lztest/lztestmanager.lzx > that is included in all the test cases. > > Files: > M test/lztest/lztest-class-impl.lzx > M test/smoke/compiler.lzl > M WEB-INF/lps/schema/lfc-undeclared.lzx > M WEB-INF/lps/server/src/org/openlaszlo/js2doc/SchemaBuilder.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassCompiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewCompiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/TypeCompiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewSchema.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassModel.java > M build-tools/runlzunit.sh > > Changeset: http://svn.openlaszlo.org/openlaszlo/patches/dda-20110407-qa4.tar > > > > -- > > Don Anderson > Java/C/C++, Berkeley DB, systems consultant > > voice: 617-306-2057 > email: [email protected] > www: http://www.ddanderson.com > blog: http://libdb.wordpress.com > > > > >
