qt4cg / qtspecs

QT4 specifications
https://qt4cg.org/
Other
28 stars 15 forks source link

### Errors in `misc/BuiltinKeywords` #1461

Closed johnlumley closed 1 month ago

johnlumley commented 1 month ago
          ### Errors in `misc/BuiltinKeywords`

Test cases Keywords-fn-parse-html-1 and Keywords-fn-build-uri-1 contain invalid function signatures, caused I think by a bug in generate-keyword-test-set.xsl. Both seem to omit the SequenceType of one of the arguments:

 fn:parse-html(html := ?, options := ?) instance of 
    function((xs:string | xs:hexBinary | xs:base64Binary)?, ) as document-node(element(*:html))?
fn:build-uri(parts := ?, options := ?) instance of function(, map(*)?) as xs:string

Examining generate-keyword-test-set.xsl shows:

<xsl:for-each select="arg">
      <xsl:if test="position() != 1">, </xsl:if>
      <xsl:text>{@type}</xsl:text>
</xsl:for-each>

but for these two function definitions, type information is indirected via a @type-ref attribute:

<fos:proto name="parse-html" return-type="document-node(element(*:html))?">
        <fos:arg name="html" type="(xs:string | xs:hexBinary | xs:base64Binary)?"/>
        <fos:arg name="options" type-ref="parse-html-options"
                     default="{
                                 &quot;method&quot;: &quot;html&quot;,
                                 &quot;html-version&quot;: &quot;5&quot;
                              }"/>
 </fos:proto>

and

<fos:proto name="build-uri" return-type="xs:string">
            <fos:arg name="parts" type-ref="uri-structure-record"
               example='{
               "scheme": "https",
               "host": "qt4cg.org",
               "port": (),
               "path": "/specifications/index.html"
               }'/>
            <fos:arg name="options" type="map(*)?" default="{}"/>
</fos:proto>

Originally posted by @johnlumley in https://github.com/qt4cg/qtspecs/issues/1451#issuecomment-2358082401

michaelhkay commented 1 month ago

So, how should we fix it?

Option 1:

generate the test as

declare record fos:parse-html-options( method as xs:string, *);
parse-html(html := ?, options := ?) 
    instance of function(...., options as fos:parse-html-options) as document-node();

Drawback: (a) that syntax is XQuery-only (b) it's not actually approved yet.

Option 2:

Same, but move the record declaration into the <environment> object. Drawback: (a) relies on extensions to the test suite framework (b) only works in XPath if implementations provide an API to declare record types in the static context of an XPath expression.

Option 3

Expand the record definition in place, so it becomes

parse-html(html := ?, options := ?) 
    instance of function(...., options as record ( method as xs:string, *)) as document-node();

I think that's probably the least troublesome. But it only works if the record definition isn't recursive.

michaelhkay commented 1 month ago

I've now run into a further problem, which is that the two record types in question, parse-html-options and uri-structure-record, seem to have dummy definitions in the function catalog, while the true definitions appear (violating the DTD...) in xpath-functions.xml. So it looks as if we need to fix bug #1336 at the same time.

ndw commented 1 month ago

Apologies for not getting to #1336 more promptly, but my schedule is a bit disrupted this week.

michaelhkay commented 1 month ago

Note in passing: for csv-structure record the spec says "The record has four parts, which are always present (though potentially empty)", but the names of the fields are then incorrectly followed with "?" indicating they may be absent. The description also uses the obsolete syntax union(xs:string, xs:integer)

michaelhkay commented 1 month ago

Closed; the PR has been applied