Add tests for invalid injection into tables after-the-fact via dotted keys

[marzer]

see toml-lang/toml#846

[hukkin]

👍

I'd suggest postponing the merge until toml-lang/toml#848 is merged, released, and toml-test updated to claim compatibilty with the new spec version.

[marzer]

released

toml-lang/toml#848 isn't adding anything new/different to the language, though, so ideally it wouldn't depend on any release. The PR seems to be just a clarification of the way it has always been, so the language spec shouldn't need a version bump.

[hukkin]

The PR is just a clarification of the way it has always been

This I disagree with.

There is a difference between actual written out spec and author's intended spec. TOML v1.0.0 is and will forever be this even if intended otherwise.

[marzer]

Eh. I disagree.

[hukkin]

Would you mind elaborating? IMO only things that are in the source tree under the release tag can be considered part of the spec. This excludes things like authors' issue comments, thoughts in their head, notes lying on their desk etc. 😄

This is how it works in software. If I release v2.0.0 of {my-application} and find out that there's a bug, I'll have to fix it and release v2.0.1. I can't just tell users that "yeah, stick to v2.0.0, the bug wasn't intended so it isn't there".

[marzer]

I disagree on the basis that I don't think it is wrong to prohibit this behaviour in v1.0.0, but in fact it was always prohibited and you simply interpreted it incorrectly. That the language in the spec was not explicit enough to prevent this misinterpretation was unfortunate, but I believe the overall intention and direction of the spec has always been clear regarding the self-contained nature of [tables] (hence why I understood it to work that way, and only came to doubt it based on assertions to the contrary after the bug report on toml++).

So, if it were clarified as a retroactive wording defect, not a bug fix, then that makes sense to me, and thus wouldn't require a version bump because nothing about the language semantics had changed.

[marzer]

Of course if others disagree with that point and insist on a version bump then so be it, but there's plenty of precedent for this logic in software versioning, despite "this is how it works in software". The ISO C++ comittee has a "Defect Report" system for applying clarifications and fixes retroactively without requiring a version bump, for example.

[arp242]

Could you add a brief comment clarifying what exactly this tests and why it should be rejected? It's not immediately obvious from the test case itself, and will make things easier for implementers down the line who might be confused why this is invalid TOML.

Or actually, maybe even better, encode it in the test itself:

[a.b.c]
  z = "a"
[a]
  b.c.t = "overriding the 'a' table from above is not allowed"

Also, I think they can be stored as key/injection-{1,2}.toml? I moved everything in to subdirectories some time ago. There is also redefine.toml already, so I think we can rename that to redefine-1.toml and then add these two new ones?

As for versioning: I thought about this before, and decided it wasn't really worth spending too much time on. The target audience of toml-test is very small with a very limited number of users (dozens, at the most). If your library doesn't implement a specific feature or some such then you can already skip these with the -skip flag, which is one reason I try to keep test names consistent since it accepts glob patterns so you can use -skip invalid/somesuch*.

Could add invalid/1.1/[...], but IMHO it's just more complexity for little benefit.

[arp242]

Also, I think they can be stored as key/injection-{1,2}.toml?

Actually, make that table/.... Can probably also move key/dotted-redefine-table.toml to table/redefine-n.toml.

[marzer]

Sure, done

Can probably also move key/dotted-redefine-table.toml to table/redefine-n.toml.

Was that for me to do, or just thinking out loud? I'm happy to include it with this PR, though renaming it will break workflows for people explicitly skipping that particular test...