w3c / data-shapes

RDF Data Shapes WG repo
89 stars 33 forks source link

path syntax too liberal #75

Closed pfps closed 7 years ago

pfps commented 7 years ago

The syntax for property paths in SHACL is both too liberal and too strict.

On the too-liberal side, paths that are lists can also look like other kinds of paths. Here are two examples of well-formed SHACL shapes that should instead be ill-formed.

@prefix rdf: http://www.w3.org/1999/02/22-rdf-syntax-ns# . @prefix rdfs: http://www.w3.org/2000/01/rdf-schema# . @prefix sh: http://www.w3.org/ns/shacl# . @prefix ex: http://example.org/shacl-test/ .

ex:s1 a sh:PropertyShape ; sh:targetNode ex:i ; sh:path [ rdf:first ex:p ; rdf:rest [ rdf:first ex:q ; rdf:rest rdf:nil ] ] ; sh:inversePath ex:q ] ; sh:class ex:C .

ex:s2 a sh:PropertyShape ; sh:targetNode ex:i ; sh:path [ rdf:first ex:p ; rdf:rest [ rdf:first ex:q ; rdf:rest rdf:nil ] ] ; sh:inversePath () ] ; sh:class ex:C .

It is quite possible that SHACL implementations do not process these shapes correctly so if these shapes remain well-formed there should be tests in the test suite to ensure that SHACL implementations implement them correctly.

HolgerKnublauch commented 7 years ago

This is not valid Turtle. Could you fix and clean up the formatting with proper indentation?

pfps commented 7 years ago

Yeah the perils of editing for presentation after syntax checking. I have no idea how to get the indentation that is in the text that I pasted to show up in the presentation. Here is a fixed up example:

@prefix rdf: http://www.w3.org/1999/02/22-rdf-syntax-ns# . @prefix rdfs: http://www.w3.org/2000/01/rdf-schema# . @prefix sh: http://www.w3.org/ns/shacl# . @prefix ex: http://example.org/shacl-test/ .

ex:s1 a sh:PropertyShape ; sh:targetNode ex:i ; sh:path [ rdf:first ex:p ; rdf:rest [ rdf:first ex:q ; rdf:rest rdf:nil ] ; sh:inversePath ex:q ] ; sh:class ex:C .

ex:s2 a sh:PropertyShape ; sh:targetNode ex:i ; sh:path [ rdf:first ex:p ; rdf:rest [ rdf:first ex:q ; rdf:rest rdf:nil ] ; sh:inversePath ( ex:p ) ] ; sh:class ex:C .

HolgerKnublauch commented 7 years ago

For the record, the point here seems to be that sequence paths only require to be well-formed lists, but list nodes may have another triple such as sh:inversePath.

I consider this to be a user error - there are many ways to mess up your shapes graph if you'r really trying, making things look like something else. Given that there is an infinite number of potentially misleading shape definitions, I see no reason why this particular corner case deserves any special test cases.

Overall I am getting the impression that the reported issues are becoming less and less critical.

simonstey commented 7 years ago

I have no idea how to get the indentation that is in the text that I pasted to show up in the presentation.

fwiw: https://help.github.com/articles/creating-and-highlighting-code-blocks/

e.g.:

```turtle ex:s ex:p ex:o . ``` becomes

ex:s ex:p ex:o .

->

@prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> .
@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
@prefix sh: <http://www.w3.org/ns/shacl#> .
@prefix ex: <http://example.org/shacl-test/> .

ex:s1 a sh:PropertyShape ;
  sh:targetNode ex:i ;
  sh:path [ rdf:first ex:p ; rdf:rest [ rdf:first ex:q ; rdf:rest rdf:nil ] ;
        sh:inversePath ex:q ] ;
  sh:class ex:C .

ex:s2 a sh:PropertyShape ;
  sh:targetNode ex:i ;
  sh:path [ rdf:first ex:p ; rdf:rest [ rdf:first ex:q ; rdf:rest rdf:nil ] ;
        sh:inversePath ( ex:p ) ] ;
  sh:class ex:C .
pfps commented 7 years ago

Thanks

pfps commented 7 years ago

I ran into this problem when I was implementing a mode in my implementation that did precise syntax checking for paths. My first effort did not work correctly and I had to reverse the order of some code to get it to work right.

So this was an actual problem for at least one implementation. That's excellent support for there being test cases for the feature.

However, there is no need at all for this kind of support for a test case. More test cases are better than fewer.

HolgerKnublauch commented 7 years ago

The fact that you had to change the order of lines in your implementation does not indicate a problem with the specification. The spec is clear that sh:inversePath must be the only triple of such a path node, therefore it excludes the well-formed list.

So what do you actually propose here we should change? You have removed the Editorial flag so it sounds like you suggest normative changes?

pfps commented 7 years ago

There at least needs to be test cases.

HolgerKnublauch commented 7 years ago

The two tests that you had submitted were accepted.

irenetq commented 7 years ago

So what is the status here? Test are added to the suite and successfully passed. On the other hand, commenter submitted a formal objection for the path syntax.

irenetq commented 7 years ago

Adding a link to the wiki page for the formal objection https://www.w3.org/2014/data-shapes/wiki/PRFO2