Skip to content
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

Sjsonnet does not error when duplicate fields are present in an object #69

Closed
mgyucht opened this issue Jun 3, 2020 · 0 comments · Fixed by #86
Closed

Sjsonnet does not error when duplicate fields are present in an object #69

mgyucht opened this issue Jun 3, 2020 · 0 comments · Fixed by #86
Assignees

Comments

@mgyucht
Copy link
Contributor

mgyucht commented Jun 3, 2020

Reproduction:

$ cat test.jsonnet
{ a: 1, a: 2 }
$ ./sjsonnet.jar test.jsonnet
{
   "a": 2
}
$ jsonnet test.jsonnet
STATIC ERROR: test.jsonnet:1:9: duplicate field: a
@szeiger szeiger self-assigned this Aug 24, 2020
szeiger added a commit to szeiger/sjsonnet that referenced this issue Aug 24, 2020
- The error message has a rather unhelpful position but this is consistent with the way other static errors are currently handled by Sjsonnet. The original Jsonnet parser does it better.

- There are two different places in the Jsonnet codebase where duplicate fields are detected: Statically in the parser and dynamically in the VM. It's not clear to me without digging much deeper into their C++ implementation when the second one is supposed to kick in. There is no specification or test case for the dynamic detection.

Fixes databricks#69
szeiger added a commit to szeiger/sjsonnet that referenced this issue Aug 24, 2020
- The error message has a rather unhelpful position but this is consistent with the way other static errors are currently handled by Sjsonnet. The original Jsonnet parser does it better.

- There are two different places in the Jsonnet codebase where duplicate fields are detected: Statically in the parser and dynamically in the VM. It's not clear to me without digging much deeper into their C++ implementation when the second one is supposed to kick in. There is no specification or test case for the dynamic detection.

Fixes databricks#69
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants