w3c / rdf-star

RDF-star specification
https://w3c.github.io/rdf-star/
Other
120 stars 23 forks source link

Add missing objects in `sparql-star-syntax-bad-ann-path-*.rq` tests #251

Closed rodentrabies closed 2 years ago

rodentrabies commented 2 years ago

Object parts are missing in the patterns in tests

sparql-star-syntax-bad-ann-path-2.rq
sparql-star-syntax-bad-ann-path-3.rq
sparql-star-syntax-bad-ann-path-4.rq
sparql-star-syntax-bad-ann-path-5.rq
sparql-star-syntax-bad-ann-path-6.rq

This makes a correct SPARQL-star parser fail the tests as intended, but for a different reason.

gkellogg commented 2 years ago

These test files are intentionally bad examples and a parser should raise an error, as indicated by the type of the test entry in the manifest.

rodentrabies commented 2 years ago

Isn't the intention of these tests to show that it doesn't make sense to have annotation syntax if the verb in the pattern is a property path (same as in sparql-star-syntax-bad-ann-path-1.rq)?

Expanding the syntactic sugar, :s (:p1|:p2) ?o {| :p3 :o |} is the same as :s (:p1|:p2) ?o. <<:s (:p1|:p2) ?o>> :p3 :o. which should not be allowed.

So I assumed that this is what the tests should be testing, as in sparql-star-syntax-bad-ann-path-1.rq, since missing object is too obvious for a bad SPARQL.

gkellogg commented 2 years ago

The grammar allows annotations of patterns using property paths, so that in itself would parse okay. But @afs wrote the tests originally so is in the best position to comment on the intention of the tests.

pchampin commented 2 years ago

The grammar allows annotations of patterns using property paths, so that in itself would parse okay.

I think that @rodentrabies' point is precisely that: these test files do not respect the grammar, because they miss an object after the property path.

hartig commented 2 years ago

The grammar allows annotations of patterns using property paths, so that in itself would parse okay.

While the grammar allows such annotations, they are still invalid. We have a restriction related to these cases, which is in the text below the grammar in Section 4.2 of the spec.

https://www.w3.org/2021/12/rdf-star.html#sparql-star-grammar

pchampin commented 2 years ago

@hartig exactly, and those tests are meant, IIUC, to check that SPARQL-parsers abide with this restriction. So they should comply with the grammar but not with the restriction (and fail for the latter reason).

Currently, they do not comply with the grammar, so their purpose is not fulfilled.

gkellogg commented 2 years ago

Good points, then these test updates would be appropriate.

afs commented 2 years ago

Yes, the tests are not testing the right thing. That's th trouble with "bad syntax" tests; they can pass (i.e. parser failure) for other reasons!

We are adding to the additional conditions of SPARQL 1.1 and for SPARQL-star, the test input should never be seen as parsed SPARQL-star.

rodentrabies commented 2 years ago

No problem! Do I need to do anything else for this to get merged? I received email from iprbot about W3C account, but I think this change goes into "Typos" category.

afs commented 2 years ago

Thanks for coming back on this. I don't believe there is anythign else needed for you but we'll check.

The test are under the W3C software license which is an open source license.

I'm happy to treat as "typo" (i.e. there is no functional changes and it reflects the original intent; it has no copyright or patent implications.)

w3cbot commented 2 years ago

pchampin marked as non substantive for IPR from ash-nazg.

pchampin commented 2 years ago

this was discussed during today's call: https://w3c.github.io/rdf-star/Minutes/2022-02-11.html#x131