[Help-smalltalk] ParseTreeRewriter tests & behavior

[Stephen Compall]

http://scompall.nocandysw.com/gst/ptrtests.st

This is a file of SUnit tests for "PTR" (ParseTreeRewriter).  (It's been 
updated since I last mentioned it.)  File it in (fix up 
RBSmallDictionary first, as described in list thread "copying 
RBSmallDictionary") to load and execute the tests.

There are two behavioral changes I'm interested in making; both current 
behaviors are tested in testCascadeCornerCases.  They are well described 
by their comments:

    "If replacement isn't a cascade or message, it drops.  Oddly, PTS 

     doesn't count this as a 'not found'; it doesn't descend into 

     arguments of the original node in this case, and, as a result, it 

     won't descend to the receiver.  I am considering changing this 

     behavior, in which case use this shouldBe: content: 



        obj. 

        z display: x; display: y; nextPut: $q" 

    self rewrite: 'stream display: obj. 

                   (stream display: z) display: (stream display: x); 

                       display: y; nextPut: $q' 

         from: 'address@hidden display: address@hidden' 

         to: 'address@hidden' 

         shouldBe: 'obj. 

                    (stream display: z) display: (stream display: x); 

                        display: y; nextPut: $q'.

I have a fix to make PTR (oops) count dropped replacements as 'not 
found', so the PTR will descend into the originals' arguments.

Next up:

    "lookForMoreMatchesInContext: doesn't copy its values.  As a 

     result, replacement in successful replacements later rejected by 

     acceptCascadeNode: (after lookForMoreMatchesInContext: is already 

     sent, after all) depends on where in the subtree a match 

     happened.  This is why selective recursion into successful 

     matches before giving outer contexts the opportunity to reject 

     them isn't so great.  It can be 'fixed' by #copy-ing each value 

     in the context before descending into it.  I would prefer 

     removing that 'feature' altogether, and my own 'trampoline' 

     rewriter does just this. 



     This replacement test depends on the non-message rejection oddity 

     described above, though fixing that won't entirely fix this 

     issue.  If that issue is fixed, this test will require adding 

     another pattern that successfully transforms one of the cascade's 

     messages." 

    self rewrite: 'qqq display: (qqq display: sss); 

                       display: [qqq display: sss]' 

         from: 'address@hidden display: address@hidden' 

         to: 'address@hidden' 

         shouldBe: 'qqq display: (qqq display: sss); 

                        display: [[sss]]'.

The whole recusivelySearchInContext mess bothers me to some degree.  In 
this small but important way, I can fight the insanity by simply copying 
context nodes before descent.  This will make [[sss]] in the replacement 
above be the less sensible [qqq display: sss], but only because of my 
dislike of PTR's selective recursion -- it would seem to be more 
consistent with the intent of selective recursion.

These behavioral changes are solely in the interest of making PTR more 
consistent; my refactoring of PTR supports the current behavior, and can 
be adjusted with even less change to fit the behavior I've described 
above.  If you don't agree, it's not a big deal; my rewriter subclass 
works around both issues by crudely cutting down on behavior complexity.

Strictly speaking, these changes will mean that PTR becomes 
behavior-incompatible with the stock ParseTreeRewriter.

What do you think?

--
Stephen Compall
http://scompall.nocandysw.com/blog