Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
joewiz opened this issue Jul 23, 2019 · 13 comments · Fixed by wolfgangmm/xqlint#2, eXist-db/xqlint#1 or #529
Open

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

joewiz opened this issue Jul 23, 2019 · 13 comments · Fixed by wolfgangmm/xqlint#2, eXist-db/xqlint#1 or #529
Labels

Comments

@joewiz
Copy link
Member

joewiz commented Jul 23, 2019

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
Copy link
Member Author

joewiz commented Sep 27, 2019

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
Copy link
Member Author

joewiz commented Sep 29, 2019

@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
Copy link
Member

dizzzz commented Sep 29, 2019

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
Copy link
Member Author

joewiz commented Sep 29, 2019

@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
Copy link
Member Author

joewiz commented Sep 30, 2019

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
Copy link
Member Author

joewiz commented Jul 15, 2022

@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: eXist-db/exist#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 eXist-db/xqlint#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
Copy link
Member Author

joewiz commented Jul 20, 2022

@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
Copy link
Member Author

joewiz commented Aug 13, 2022

@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?

@ccheraa
Copy link
Contributor

ccheraa commented Sep 28, 2022

@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 joewiz reopened this Oct 4, 2022
@joewiz
Copy link
Member Author

joewiz commented Oct 4, 2022

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
Copy link
Contributor

ccheraa commented Oct 17, 2022

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
Copy link
Contributor

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
Copy link
Contributor

ccheraa commented Oct 20, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment