eXist-db / eXide

A web-based XQuery IDE for eXist-db
GNU General Public License v3.0
45 stars 32 forks source link

xqlint error causes "run test" to silently fail #224

Open joewiz opened 5 years ago

joewiz commented 5 years ago

When running a test with the spec-compliant map constructor syntax, eXide silently fails, with this console error:

Error while parsing XQuery: syntax error, found ':'
while expecting ':='
at line 9, column 14:
...: "Sunday",
        "Mo" : "Monday",
        "Tu" : "Tuesday",
 ...

Correcting the xqlint parser at https://github.com/wolfgangmm/xqlint/blob/master/lib/XQueryParser.ebnf#L426 should solve this immediate issue, though it's unfortunate that the parsing error isn't reported explicitly to the user in the application.

We should probably test atom-existdb to see if its xqlint fork also shares this issue. https://github.com/eXist-db/atom-existdb/blob/master/package.json#L47 suggests that it's using this fork: https://github.com/eXistSolutions/xqlint/tree/master/lib. I can't actually find any trace of support for the map constructor in this repo...

joewiz commented 5 years ago

As noticed by @dizzzz and me, xqlint parsing errors still will prevent eXide from even executing xqsuite tests via the "XQuery > Run as test" menu. I have a very rough workaround in https://github.com/joewiz/eXide/tree/disable-xqlint-for-xqsuite, but I hesitate to even propose this as a PR, since we lose functionality, as I describe in the commit. That said, the branch does allow me to resume use of eXide for performing xqsuite tests that contain syntax that, for some reason, offends xqlint (or rather the ancient fork of it that eXide is stuck on).

joewiz commented 5 years ago

@dizzzz I understand from our conversation in Slack that the https://github.com/joewiz/eXide/tree/disable-xqlint-for-xqsuite branch isn't making the xqlint-for-xqsuite problem go away. Could you please post a link to the test module you're using, or post the module here as an attachment? Then I can test it on my system and try to prevent xqlint from firing on your module?

dizzzz commented 5 years ago

store as test.xq :

xquery version "3.1";

module namespace m = "aaaaaaaaaaa"; 

declare namespace test = "http://exist-db.org/xquery/xqsuite";

declare variable $m:one := map {
"a" : "b",
"c" : "d"
};

declare variable $m:two := map {
"a" : "b",
"c" : "d"
};

declare
%test:assertEquals("this should fail")
function m:testSimple() as xs:string {
    "aaaa"
};
joewiz commented 5 years ago

@dizzzz Your test works for me in eXide, built from my branch. I copied and pasted your query into eXide, saved it as /db/test.xq, and then selected XQuery > Run as test. The query result was:

<testsuites>
    <testsuite package="aaaaaaaaaaa" timestamp="2019-09-29T18:29:22.679-04:00" tests="1" failures="1" errors="0" pending="0" time="PT0.006S">
        <testcase name="testSimple" class="m:testSimple">
            <failure message="assertEquals failed." type="failure-error-code-1">this should fail</failure>
            <output>aaaa</output>
        </testcase>
    </testsuite>
</testsuites>

I've attached my build of eXide here, since it must be that something is preventing your system from picking up the changes in my branch.

eXide-2.4.9.zip - disable-xqlint-for-xqsuite build

After downloading, just change zip to xar, and install via Package Manager; then in eXide, shift-click on the reload button to ensure that the browser doesn't load stale copies of eXide.min.js. Then:

  1. Copy & paste your sample test query into a new query window in eXide
  2. Save the new query window as /db/test.xq
  3. Select XQuery > Run as test
  4. Results should appear as above.
joewiz commented 5 years ago

Another totally valid approach to running xqsuite tests in eXide (or anywhere) is to invoke the xqsuite module manually. All you have to do is write a query like this:

xquery version "3.1";

import module namespace test="http://exist-db.org/xquery/xqsuite" at "resource:org/exist/xquery/lib/xqsuite/xqsuite.xql";

test:suite(
    inspect:module-functions(xs:anyURI("/db/test.xq"))
)

This approach is documented in https://exist-db.org/exist/apps/doc/xqsuite#running-testsuite.

joewiz commented 2 years ago

@ccheraa @plutonik-a While some of the original tests given here do now pass, the issue can still be triggered if you use syntax that xqlint chokes on. For an example, follow the steps in the linked issue above: https://github.com/eXist-db/exist/issues/3417. I'll add the test here for convenience.

Steps to reproduce

  1. Save the following XQSuite test query to the database as /db/test.xq

    xquery version "3.1";
    
    module namespace t="http://exist-db.org/xquery/test";
    
    declare namespace test="http://exist-db.org/xquery/xqsuite";
    
    declare variable $t:XML := document {
        <test>
           <strange xml:id="foo"/>
        </test>
    };
    
    declare variable $t:XSL := document {
        <xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="3.0">
            <xsl:mode on-no-match="shallow-copy"/>
        </xsl:stylesheet>
    };
    
    declare variable $t:CONTROLLER := ``[xquery version "3.1";
    
    if (ends-with($exist:resource, ".xml")) then
         <dispatch xmlns="http://exist.sourceforge.net/NS/exist">
             <view>
                 <forward servlet="XSLTServlet">
                     <set-attribute name="xslt.stylesheet" value="{$exist:root}{$exist:controller}/test.xsl"/>
                 </forward>
             </view>
             <cache-control cache="no"/>
         </dispatch>
    
    else
        ()
    ]``;
    
    declare
        %test:setUp
    function t:setup() {
        let $testCol := xmldb:create-collection("/db/apps", "test")
        return
            (
                xmldb:store("/db/apps/test", "test.xml", $t:XML),
                xmldb:store("/db/apps/test", "test.xsl", $t:XSL),
                xmldb:store("/db/apps/test", "controller.xql", $t:CONTROLLER),
                sm:chmod(xs:anyURI("/db/apps/test/controller.xql"), "o+x")
            )
    };
    
    declare
        %test:tearDown
    function t:tearDown() {
        xmldb:remove("/db/apps/test")
    };
    
    declare
        %test:assertEquals('foo')
    function t:pass-through-controller() {
        doc("http://localhost:8080/exist/apps/test/test.xml")//@xml:id/string()
    };
    
    declare
        %test:assertEquals('foo')
    function t:directly-transform() {
        let $node-tree := doc("/db/apps/test/test.xml")
        let $stylesheet := doc("/db/apps/test/test.xsl")
        let $parameters := ()
        return
            transform:transform($node-tree, $stylesheet, $parameters)//@xml:id/string()
    };
  2. In eXide, run XQuery > Run as test. The expected test results are not shown. No user-facing error is shown. Examine the Javascript console and notice the error (pasted in below the query).

    Running xqlint...
    util.js:368 Error while parsing XQuery: lexical analysis failed
    while expecting [Wildcard, EQName, IntegerLiteral, DecimalLiteral, DoubleLiteral, StringLiteral, S, '$', '%', '(', '(#', '(:', '+', '-', '.', '..', '/', '//', '<', '<!--', '<?', '@', '[', 'after', 'allowing', 'ancestor', 'ancestor-or-self', 'and', 'append', 'array', 'as', 'ascending', 'at', 'attribute', 'base-uri', 'before', 'boundary-space', 'break', 'case', 'cast', 'castable', 'catch', 'child', 'collation', 'comment', 'constraint', 'construction', 'context', 'continue', 'copy', 'copy-namespaces', 'count', 'decimal-format', 'declare', 'default', 'delete', 'descendant', 'descendant-or-self', 'descending', 'div', 'document', 'document-node', 'element', 'else', 'empty', 'empty-sequence', 'encoding', 'end', 'eq', 'every', 'except', 'exit', 'external', 'first', 'following', 'following-sibling', 'for', 'ft-option', 'function', 'ge', 'group', 'gt', 'idiv', 'if', 'import', 'in', 'index', 'insert', 'instance', 'integrity', 'intersect', 'into', 'is', 'item', 'json-item', 'last', 'lax', 'le', 'let', 'loop', 'lt', 'map', 'mod', 'modify', 'module', 'namespace', 'namespace-node', 'ne', 'node', 'nodes', 'object', 'only', 'option', 'or', 'order', 'ordered', 'ordering', 'parent', 'preceding', 'preceding-sibling', 'processing-instruction', 'rename', 'replace', 'return', 'returning', 'revalidation', 'satisfies', 'schema', 'schema-attribute', 'schema-element', 'score', 'self', 'sliding', 'some', 'stable', 'start', 'strict', 'switch', 'text', 'to', 'treat', 'try', 'tumbling', 'type', 'typeswitch', 'union', 'unordered', 'updating', 'validate', 'value', 'variable', 'version', 'where', 'while', 'with', 'xquery', '{', '{|']
    at line 19, column 35:
    ...``[xquery version "3.1";
    
    if (ends-with($exist:resource, ".xml")...
  3. To confirm that the test can be run, use the query posted above

    xquery version "3.1";
    
    import module namespace test="http://exist-db.org/xquery/xqsuite" at "resource:org/exist/xquery/lib/xqsuite/xqsuite.xql";
    
    test:suite(
        inspect:module-functions(xs:anyURI("/db/test.xq"))
    )

The Javascript console error shows that, as described in the original post, an xqlint error can cause eXide's "Run as test" to silently fail.

Thus, part 1 of the fix to this issue is to apply a necessary band aid for cases when xqlint's parser chokes on a query. Specifically, eXide should catch xqlint errors like this and report the xqlint parsing error to the user. Something like this from the console above should be shown:

Error while parsing XQuery: lexical analysis failed

It should probably be shown where other eXide errors are shown - in red text in the lower right corner of the editor pane.

Judging by the location of the parsing error - at line 19 column 35 of the query - xqlint is tripping on test.xq's valid use of the XQuery 3.1 String Constructor syntax, i.e., ``[ ]``.

Looking through the EBNF in the parser, the version of xqlint being used by eXide doesn't have a definition for the String Constructor.

Thus, part 2 of the fix is to update the EBNF to add the String Constructor and update the parser used by eXide. For one such PR, see https://github.com/eXist-db/xqlint/pull/1.

For additional context, Note that the xqlint used by eXide is a very old fork of https://github.com/wcandillon/xqlint. That project has since updated its tooling to use more modern Javascript, but in a way that @wolfgangmm found difficult to update eXide to use. But he did use a newer fork of the upstream xqlint repository for his atom and vscode plugins. The xqlint fork used for those plugins is now 38 commits behind the original fork. It doesn't have the needed EBNF additions for the string constructor. But this is all to say that one alternative approach to "part 2" here is to switch from the old fork to the current upstream master. The EBNF additions would still be needed (hopefully, they could simply be copied from the XQuery spec linked above), but at least the tooling for building xqlint would be much newer.

joewiz commented 2 years ago

@marmoure A simpler test xqsuite module to reproduce the same issue:

xquery version "3.1";

module namespace t="http://exist-db.org/xquery/test";

declare namespace test="http://exist-db.org/xquery/xqsuite";

declare variable $t:foo := ``[foo]``;

declare
    %test:assertEquals("foo")
function t:simple-test() {
    $t:foo
};

Just replace the query in the 1st step of my previous post with this.

joewiz commented 2 years ago

@ccheraa I see you mentioned this issue in a comment on a PR submitted to a forked copy of this repository: https://github.com/evolvedbinary/eXide/pull/1. Is that PR part of the fix to this issue? If so, should that PR be resubmitted to this repository?

ccheraa commented 2 years ago

@ccheraa I see you mentioned this issue in a comment on a PR submitted to a forked copy of this repository: evolvedbinary#1. Is that PR part of the fix to this issue? If so, should that PR be resubmitted to this repository?

This is not a fix for this issue, it only displays a comprehensive error message when eXide's and eXist-db's XQuery parsers are not the same version, instead of failing silently.

joewiz commented 2 years ago

As discussed and noted in #529, that PR was an important but partial fix. A full fix to this issue requires updating xqlint to accept valid XQuery 3.1 syntax.

ccheraa commented 2 years ago

It turns out updating eXist-db/xqlint to support XQuery 3.1 is not enough, as eXide currently relies on an old commit of that repo, and the current version of XQlint will not work with eXide. This means after updating XQlint, we will need to update eXide too to support the latest XQlint. Another approach that may take less time but is not necessarily good is to work on the old comment that eXide currently use and update that version to support XQuery 3.1. @joewiz @adamretter @marmoure

adamretter commented 2 years ago

as eXide currently relies on an old commit of that repo

@ccheraa Could you provide a bit more detail please? What commit and what feature is it specifically that eXide relies on in the older https://github.com/eXist-db/xqlint that is not present in a more modern xqlint?

ccheraa commented 1 year ago

https://github.com/eXist-db/xqlint has some changes that helps connect it to eXide, which are missing from https://github.com/wcandillon/xqlint I don't know yet if a simple rebase/merge would fix that, or if more work is needed.