Support named nodes in property paths

[LaurensRietveld]

This is a fix for #69

Added a unit test to cover the situation from #69. All unit tests pass.

Happy to get some feedback and change this when needed

[tpluscode]

@martinmaillard the initial proposal was to have this as an opt-in feature since it deviates from SHACL spec (regardless of the merits of such a restrictions). I think I'm favour of that

[LaurensRietveld]

Yep, I was unsure whether everybody agreed on that. happy to add a flag though

[LaurensRietveld]

@tpluscode Do you have suggestions on how to propagate such a flag from the index.js file down to the property-path.js file? The original shacl-validate options are not accessible in the scope of extractPropertyPath

[tpluscode]

Not off the top of my head. I've worked little with tis codebase.

Is the maxErrors option any inspiration?

[LaurensRietveld]

I checked that out, but it's used on a higher level so easier to pass on

[LaurensRietveld]

If the main challenge lies in propagating options to lower-level parts of the codebase, would it be an idea to:

• Merge/release this without adding it behind a flag. Given that the behavior will be a superset of the spec, it wont cause third-party uses to fail
• Follow up in a different issue where we focus on configuring the validator on a lower level

Obviously the above is only applicable under the assumption that it's tricky to propagate these options cleanly to lower parts of the codebase. Any thoughts @martinmaillard ?

[martinmaillard]

I would personally be very careful when adding a feature that is not part of the spec. You mention that "the behavior will be a superset of the spec", but I don't think that's necessarily true. Nothing prevents me from saying that schema:name a rdf:List. How do I know if the path is the list or the named node then? The example is a bit contrived, but you get the point.

I will let @bergos and @tpluscode chime in on that. But I don't think we want to spend a lot of time figuring out how to add features flags and complexity to this lib for such a feature.

If you really have to deal with shapes where blank nodes have been replaced with IRIs, one solution would be to wrap the library to replace the paths with blank nodes again before validation.

[LaurensRietveld]

Wrapping the library is problematic when postprocessing the reports, as you'll have to convert the reports back again. It's a pity if we have to resort to overcomplicated wrappers or having to maintain a fork for this.

[bergos]

Everyone is happy to merge a PR for your requested feature on our site. But as Martin described, it's actually not a superset. I'm aware the described scenario is an edge case that probably nobody will ever have to deal with. But to keep the library spec compliant for everyone else who wants to try all kinds of crazy things, having a flag and disabling this feature by default is a requirement to merge the PR.

[LaurensRietveld]

@bergos That's understandable. Took another look, and turned out it was relatively straight forward after all to make this behaviour configurable. Let me know if there is any feedback on the latest changes

[martinmaillard]

Thanks! I'll do a release soon