Skip to content

Warn or error if non-keyword strings having "@" are encountered #16

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

Closed
gkellogg opened this issue Jun 30, 2018 · 13 comments
Closed

Warn or error if non-keyword strings having "@" are encountered #16

gkellogg opened this issue Jun 30, 2018 · 13 comments
Labels
satisfied Requirement Satisfied spec:editorial

Comments

@gkellogg
Copy link
Member

gkellogg commented Jun 30, 2018

Looking at a schema.org example for ItemList, there is a JSON-LD example which includes the fictitious @url keyword, where they likely meant @id.

{
  "@context": "http://schema.org",
  "@type": "ItemList",
  "@url": "http://en.wikipedia.org/wiki/Billboard_200",
  "name": "Top music artists",
  "description": "The artists with the most cumulative weeks at number one according to Billboard 200",
  "itemListElement": [
    {
      "@type": "ListItem",
      "position": 1,
      "item": {
        "@type": "MusicGroup",
        "name": "Beatles"
      }
    },
    {
      "@type": "ListItem",
      "position": 2,
      "item": {
        "@type": "MusicGroup",
        "name": "Elvis Presley"
      }
    },
    {
      "@type": "ListItem",
      "position": 3,
      "item": {
        "@type": "MusicGroup",
        "name": "Michael Jackson"
      }
    },
    {
      "@type": "ListItem",
      "position": 3,
      "item": {
        "@type": "MusicGroup",
        "name": "Garth Brooks"
      }
    }
  ]
}

There's nothing to signal an issue, and a JSON-LD processor will happily resolve this relative to @vocab as http://schema.org/@url. This creates a potential forward-compatibility issue if new keywords are introduced, as they are in 1.1. We might want to describe normative or suggested behavior if a processor encounters a string which could hold a keyword, but holds something else starting with @.

cc/ @danbri

Original issue: Warn or error if non-keyword strings having "@" are encountered #598

@ajs6f
Copy link
Member

ajs6f commented Aug 10, 2018

Is it possible to choose to ignore such new @ keywords in the algorithms? I.e. create no triples from them and drop them in expansion?

@gkellogg
Copy link
Member Author

Is it possible to choose to ignore such new @ keywords in the algorithms? I.e. create no triples from them and drop them in expansion?

That's what 1.0 does, it just ignores them. The issue was an example in schema.org (ItemList example #3) which uses @url instead of @id, which would seem to be an error. Thus the danger of ignoring such things.

  "@context": "http://schema.org",
  "@type": "ItemList",
  "@url": "http://en.wikipedia.org/wiki/Billboard_200",
  "name": "Top music artists",
  "description": "The artists with the most cumulative weeks at number one according to Billboard 200",
  "itemListElement": [
    {
      "@type": "ListItem",
      "position": 1,
      "item": {
        "@type": "MusicGroup",
        "name": "Beatles"
      }
    },
    {
      "@type": "ListItem",
      "position": 2,
      "item": {
        "@type": "MusicGroup",
        "name": "Elvis Presley"
      }
    },
    {
      "@type": "ListItem",
      "position": 3,
      "item": {
        "@type": "MusicGroup",
        "name": "Michael Jackson"
      }
    },
    {
      "@type": "ListItem",
      "position": 3,
      "item": {
        "@type": "MusicGroup",
        "name": "Garth Brooks"
      }
    }
  ]
}

@ajs6f
Copy link
Member

ajs6f commented Aug 10, 2018

I'm not sure if I'm misunderstanding, but the example doesn't seem to have ignored it at all-- it expanded it to http://schema.org/@url. I'm asking whether it would be possible to ignore it in expansion entirely, i.e. @url would be dropped entirely, no expanded form, no triples. That might be more surprising than helpful, but it would seem to be a little more forward-compatible. Or perhaps I'm still missing the point (wouldn't be the first time!)?

@gkellogg
Copy link
Member Author

the example doesn't seem to have ignored it at all

Good point, that's because of the use of @vocab; otherwise, it would have been dropped. Another reason we might flag this. If you really wanted @url as a relative IRI, you could always use ./@url.

@dlongley
Copy link
Contributor

This seems like a feature implementations could offer (a special mode to check for this sort of thing). I don't think we'd want this extra burden (or any similar extra burdens) added to the standard processing rules.

@ajs6f
Copy link
Member

ajs6f commented Aug 17, 2018

@dlongley I don't maintain an impl (so take a grain of salt with this question), but would a configurable mode just shift the burden to the clients of impls instead of the maintainers of impls?

@dlongley
Copy link
Contributor

@ajs6f,

Yes, but I think that's where it should be. In fact, I think I'm speaking more with my client hat on than my implementer hat when I say that I do not want my production systems to have to deal with the extra burden of checking non-error cases to express warnings.

I think it should be a goal to have a "fast lane" for JSON-LD processing to keep things as efficient as possible. I would prefer that the bottleneck in a complex software system never be JSON-LD processing. One way to help support that effort is to keep corner cases like raising warnings out of the standard processing mode.

@azaroth42
Copy link
Contributor

I'm convinced by @dlongley's argument.

I'm in favor of close won't fix for the following rationale:

  • Per @dlongley, raising a warning but otherwise continuing is a burden for processing that in the regular case of it being a part of a much larger automated system will never be seen anyway, so just adds complexity and no value.
  • It could be an option to turn on when debugging your JSON-LD, and turned on by default in the playgrounds. But that's an implementation issue, not syntax or API.
  • It would break the (admittedly incorrect) existing usage in the schema example, but I wouldn't be surprised if someone had intentionally mapped @label to rdf:label to go along with @type, and it would certainly break them.
  • There is already a non-normative note at the end of 4.8 warning against making your own keys that start with @, and I think from a spec perspective that is the right approach.

@ajs6f
Copy link
Member

ajs6f commented Aug 24, 2018

@azaroth42 I'm actually fine with close-won't-fix, but I do want to address one point you make:

Per @dlongley, raising a warning but otherwise continuing is a burden for processing that in the regular case of it being a part of a much larger automated system will never be seen anyway, so just adds complexity and no value.

I disagree strongly. It's exactly in the case of a large fully-automated system where I want warnings from my libraries (instead of silent acquiescence), so that I can accumulate information that lets me improve workflows without them having to break down first. So although I agree that this ticket isn't a good case at all for emitting warnings, I disagree strongly that we could find no such cases.

Just my 2¢!

@azaroth42
Copy link
Contributor

WG resolved to add a non-normative note to the API specification to recommend that implementations generate a warning, however is appropriate for the implementation's language, when @ signs are encountered outside of specified keywords.

@gkellogg
Copy link
Member Author

Fixed in 760b32fd396e47f5ae0ff720c3f3c9a2080592eb.

@azaroth42
Copy link
Contributor

Reopening from TPAC discussion

@iherman
Copy link
Member

iherman commented Sep 21, 2019

This issue was discussed in a meeting.

  • RESOLVED: Reopen w3c/json-ld-syntax#16 and require processors to ignore terms beginning with @ that are not defined keywords
View the transcript Ivan Herman: See Issue Syntax#260
Gregg Kellogg: what we’re most worried about is other specs using @ and we want them to be compatible with us
… but at this point hopefully everyone’s aware of our use of @
Manu Sporny: what does changing it to MUST NOT buy us?
Gregg Kellogg: it means that if we add things like @direction in the future, the processors would choke on the new term but they also wouldn’t try to turn it into a URI
Ralph Swick: so, could you just say SHOULD NOT?
Rob Sanderson: it does
Ralph Swick: ah, so you are leaving this open for future compatibility
Gregg Kellogg: and I think @direction is an example of why we want this
Rob Sanderson: there was an earlier issue which we closed about this
… and it was mentioned that if we found new info we might reopen it
… this seems like new infromation
Gregg Kellogg: so, some future @direction type thing could be setup to just disappear
Rob Sanderson: or attempt to turn into a crazy relative IRI
Benjamin Young: so, as long as we just ignore the new values when processing–and don’t choke–that should leave the door open to the future without tripping up old clients
Gregg Kellogg: authors should not use these keywords, but processors must ignore them
… but with a warning
Proposed resolution: Reopen #16 and require processors to ignore terms beginning with @ that are not defined keywords (Rob Sanderson)
Ivan Herman: See Issue syntax#16
Gregg Kellogg: +1
Manu Sporny: +1
Rob Sanderson: +1
Benjamin Young: +1
Simon Steyskal: +1
Pierre-Antoine Champin: +1
Ivan Herman: +1
Resolution #5: Reopen #16 and require processors to ignore terms beginning with @ that are not defined keywords
Rob Sanderson: Have we made this clear, it’s resolved?
Gregg Kellogg: Yes, the editors need to do their thing now.
Rob Sanderson: so we’ll close #260 in favor of opening #16.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
satisfied Requirement Satisfied spec:editorial
Projects
Archived in project
Development

No branches or pull requests

5 participants