Error in rendering empty JSON array #4399

Closed
andreaferretti opened this Issue Jun 23, 2016 · 11 comments

Projects

None yet

4 participants

@andreaferretti
Contributor

The following

import json

let y = %[]

echo y

prints {}. This is particularly inconvenient when using the %* macro, where the empty sequence could be nested arbitrarilly

@Araq
Member
Araq commented Jun 23, 2016

What should it produce instead? []?

@andreaferretti
Contributor
andreaferretti commented Jun 23, 2016 edited

Yes. There are easy workaround (such as using newSeq[...]()) but [] is definitely what I would expect.

On the other hand, there should be a way to get an empty object - such as {:}

@jangko
Contributor
jangko commented Jun 25, 2016

the compiler confused between this two procs

proc `%`*(keyVals: openArray[tuple[key: string, val: JsonNode]]): JsonNode =
  ## Generic constructor for JSON data. Creates a new `JObject JsonNode`
  result = newJObject()
  for key, val in items(keyVals): result.fields[key] = val

proc `%`*[T](elements: openArray[T]): JsonNode =
  ## Generic constructor for JSON data. Creates a new `JArray JsonNode`
  result = newJArray()
  for elem in elements: result.add(%elem)

and the compiler choose the the first one, the one wrong one.
looks like a semcheck issue too.

@jangko
Contributor
jangko commented Jun 25, 2016

this is a duplicate of #3950

@Araq
Member
Araq commented Jun 25, 2016 edited

looks like a semcheck issue too.

No, the compiler adheres to the spec which says that in this case the non-generic implementation is to be preferred.

@Araq
Member
Araq commented Jun 25, 2016

The problem here is that {:} is a synonym for [] and while %[] should produce an empty JSON list, %{:} should produce an empty object. %{:} is not used out there I bet (since it's not known), so the right fix is to make the % for openArray special casing the len==0.

@jangko
Contributor
jangko commented Jun 25, 2016

I've tried it, and it fix the issue altough a bit weird

proc `%`*(keyVals: openArray[tuple[key: string, val: JsonNode]]): JsonNode =
  ## Generic constructor for JSON data. Creates a new `JObject JsonNode`
  if keyVals.len == 0: return newJArray()
  result = newJObject()
  for key, val in items(keyVals): result.fields[key] = val
@Araq
Member
Araq commented Jun 26, 2016

PR please!

@jangko
Contributor
jangko commented Jun 26, 2016

the above solution not work for %{:} because now it's produce [] too, which is wrong.
need to find another solution.

@Varriount
Contributor

Hm. Would a term-rewriting macro work? Of course, that would only affect 'literal' values, not computed ones...

@Araq
Member
Araq commented Jun 27, 2016

the above solution not work for %{:} because now it's produce [] too, which is wrong.

I said so, but I don't think it's a problem, few people know about %{:}

@Araq Araq added a commit that closed this issue Jul 8, 2016
@Araq Araq fixes #4399 019ee22
@Araq Araq closed this in 019ee22 Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment