Bugzilla – Full Text Bug Listing
| Summary: | [xpath2] [patch] parsing error, while using language keywords as element and attribute names | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP Source Editing | Reporter: | Mukul Gandhi <mukul.gandhi> | ||||||||
| Component: | wst.xpath | Assignee: | Jesper Moller <jesper> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | David Carver <d_a_carver> | ||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | d_a_carver, emarcotte, jesper | ||||||||
| Version: | unspecified | Flags: | d_a_carver:
review+
|
||||||||
| Target Milestone: | 3.2.1 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Mukul Gandhi
(In reply to comment #0) > Build Identifier: I20090313-0100 > > XPath expressions like following, currently give parsing error with PsychoPath > engine: > > @attribute eq 'xyz' > attribute eq 'xyz' > > @element eq 'xyz' > element eq 'xyz' > > It seems, that PsychoPath engine considers words 'attribute' & 'element' in the > above tests, as language keywords & does not treat these names as valid element > & attribute names. > > Although, I think good XML design would not use the above names, as names for > elements & attributes, but I think, this is still a bug & we should fix it. > > I could be a bit slow in solving this :( Perhaps, somebody else could > investigate this. Yeah, this is a known issue with the parser. We have to add them into a separate QName list in the parser generator to allow it to work correctly. I'll look at targeting for RC1 and attach a patch for review here. (In reply to comment #1) > Yeah, this is a known issue with the parser. We have to add them into a > separate QName list in the parser generator to allow it to work correctly. > I'll look at targeting for RC1 and attach a patch for review here. Thanks, Dave. Regards, Mukul Mukul I've taken a shot at this, but unfortunately the changes I made we causing prior tests to fail that were originally passing. I'm going to CC Jesper on this as well to take a look into it. We'll try for RC1, but this may have to be pushed until WTP 3.2.1. BTW, jesper we'll need PMC approval if we do find a fix for this. Created attachment 167785 [details]
Unit Test for the element/attribute issue as nodes
Retargeting for 3.2.1. Ultimately I think we are going to need to look at the newer versions of Java Cup, or a different parser/lexer combination. JFlex isn't our problem, but javacup has some limitations. Supposedly the new Javacup 11 addresses many of these limitations. (In reply to comment #6) > Retargeting for 3.2.1. > Ultimately I think we are going to need to look at the newer versions of Java > Cup, or a different parser/lexer combination. JFlex isn't our problem, but > javacup has some limitations. Supposedly the new Javacup 11 addresses many of > these limitations. Thanks, Dave for assesment of this bug. I hope that new CUP version, could resolve this issue. It would be great, if you could please share results of migration to, CUP 11. Regards, Mukul Quite close to fixing this. There were other errors as well. We don't need CUP 11, we just need to read the grammar and remember our CS lessons... Created attachment 171786 [details]
Patch for the grammar issue
Also: as, eq, ne, lt, lq, gt, and ge are now allowed as names.
Hi Jesper, (In reply to comment #9) > Created an attachment (id=171786) [details] > Patch for the grammar issue > > Also: as, eq, ne, lt, lq, gt, and ge are now allowed as names. The attachment "Patch for the grammar issue" seem to contain only the test-cases for this issue. Is the source-code of the patch (to be applied to "org.eclipse.wst.xml.xpath2.processor) also available in your Jira post? I can't seem to find it presently :( Or is this still a work in progress. Regards, Mukul This appears to be a failure of the CVS Create Patch wizard in the Helios release candidate I'm using. I will investigate. Created attachment 171800 [details]
Patch for the parser
This patch has the parser changes.
If I add the sym file, there's no diff. I must investigate that against the CVS component.
Hi Jesper, (In reply to comment #12) > Created an attachment (id=171800) [details] > Patch for the parser > > This patch has the parser changes. > > If I add the sym file, there's no diff. I must investigate that against the CVS > component. I generated parser Java file from the grammar patch you've attached here, and integrated that into PsychoPath engine code-base. my preliminary tests show that, all of the problems I found in this issue are solved by your patch. The overall test success rate, for PsychoPath test-suite is same as before (which is great :). I therefore consider that, this patch has resolved this issue & I'm marking it resolved. In-case I find any test failures with new scenarios, I'll report them here. Thanks a lot for looking at it :) PS: after the WTP CVS server has resumed commits, kindly commit the changes to the code base on HEAD please. Regards, Mukul Great to hear the patch is working. > PS: after the WTP CVS server has resumed commits, kindly commit the changes to > the code base on HEAD please. I'm thinking that this one, the JDK 1.4 patch (bug 280798), and also the one concerning 'instance of' (bug 312191) should go into WTP 3.2.1 service release. I'm guessing you'll want to release Xerces-J 2.10 with that release, right? I'm reopening this bug so we can track that it hasn't been committed yet! (In reply to comment #14) > I'm thinking that this one, the JDK 1.4 patch (bug 280798), and also the one > concerning 'instance of' (bug 312191) should go into WTP 3.2.1 service release. I think, that will be great :) > I'm guessing you'll want to release Xerces-J 2.10 with that release, right? I think, Xerces-J 2.10.0 in it's documentation wouldn't recommend any specific WTP release, to use a PsychoPath Jar. Xerces user's would probably be advised to download the latest PsychoPath Jar from eclipse.org WTP site. Regards, Mukul I think Eclipse Helios got released today, so perhaps we might be able to commit changes to CVS HEAD. @Jesper: could you please do the needful to commit the changes you proposed in "comment 14". Copying to Dave too. Regards, Mukul (In reply to comment #16) > I think Eclipse Helios got released today, so perhaps we might be able to > commit changes to CVS HEAD. > > @Jesper: could you please do the needful to commit the changes you proposed in > "comment 14". > Yes, this should now be applied to both HEAD and the 3.2.1 branch. I need to do some tweaks for the build to get it going again. Will hopefully have time this weekend.' We need review, don't we, for inclusion to 3.2.1 ? Nope no review necessary for 3.2.1. Only when we start getting down near the release canidate stage for 3.2.1 will we need reviews again. Jesper just to be on the safe side, add me for code review on this, and I'll +1 it. Looks good to me. Committed in trunk, but not in branch yet Committed to branch as well. Released as v201006250052 in HEAD and v201006250054 in 3.2 branch Broke the build somehow, will investigate?? (In reply to comment #23) > Committed to branch as well. > Released as v201006250052 in HEAD and v201006250054 in 3.2 branch > > Broke the build somehow, will investigate?? the PsychoPath specific build has been broken for a while. If the XSL build runs to completion correctly, it also runs all the Psychopath tests. I'm going to work on getting the PsychoPath build setup with the new Maven based approach I'm using with the other incubators. It'll be much faster as well. (In reply to comment #23) > Committed to branch as well. > Released as v201006250052 in HEAD and v201006250054 in 3.2 branch > > Broke the build somehow, will investigate?? took an update of code from HEAD, and found it's working fine with the changes we did, for this bug report :) I've few questions please, I can still see that PsychoPath code-base on HEAD is at 1.5 level. When are we expecting to make to 1.4 level (i.e, apply the PsychoPath JDK 1.4 patch that we have)? What is the significance (& it's policy vs HEAD location) of 3.2.1 branch? What is it's path on WTP CVS tree? Regards, Mukul (In reply to comment #25) > I can still see that PsychoPath code-base on HEAD is at 1.5 level. When are we > expecting to make to 1.4 level (i.e, apply the PsychoPath JDK 1.4 patch that we > have)? I would prefer to have our continuous build succeed before we make such a big change, otherwise it may get tricky to catch up. > What is the significance (& it's policy vs HEAD location) of 3.2.1 branch? What > is it's path on WTP CVS tree? Same path, a branch on the files "R3_2_maintenance". To work with the branch in CVS, checkout the projects and then right click on each project, choose Team -> Switch to Another Branch or Version... and just follow the signs. Thanks, Jesper. (In reply to comment #26) > I would prefer to have our continuous build succeed before we make such a big > change, otherwise it may get tricky to catch up. I agree. > Same path, a branch on the files "R3_2_maintenance". > To work with the branch in CVS, checkout the projects and then right click on > each project, choose Team -> Switch to Another Branch or Version... and just > follow the signs. Thanks a lot. I'll try this. Regards, Mukul (In reply to comment #26) > I would prefer to have our continuous build succeed before we make such a big > change, otherwise it may get tricky to catch up. The CI Build from HEAD is running again. It is now using Maven3/Tycho. Note if you break API, you'll need to up the major version numbers in both the POM and the Manifest files otherwise the build will fail. *** Bug 323266 has been marked as a duplicate of this bug. *** |