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

Jump to bottom

encoding/json: strange isEmptyValue behavior with arrays #31189

Closed
sorenvonsarvort opened this issue Apr 1, 2019 · 2 comments
Closed

encoding/json: strange isEmptyValue behavior with arrays #31189

sorenvonsarvort opened this issue Apr 1, 2019 · 2 comments
Labels
FrozenDueToAge

Comments

@sorenvonsarvort
Copy link

sorenvonsarvort commented Apr 1, 2019 •
edited by andybons

What version of Go are you using?

$ go version
go version go1.12.1 linux/amd64

Does this issue reproduce with the latest release?

Yes, it does.

What operating system and processor architecture are you using?

go env Output
$ go env

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/sorenvonsarvort/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/sorenvonsarvort/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build582915182=/tmp/go-build -gno-record-gcc-switches"

What did you do?

package main

import (
	"encoding/json"
	"fmt"
)

type EncodingBreaker [12]byte

type Article struct {
	ID EncodingBreaker `json:"id,omitempty"`
}

func main() {
	articleBytes, err := json.MarshalIndent(Article{}, "", "	")
	if err != nil {
		// ...
	}

	fmt.Println("item:", string(articleBytes))
	fmt.Println("error:", err)
}

https://play.golang.org/p/JTZn-qSksgw

What did you expect to see?

item: {}
error: <nil>

What did you see instead?

item: {
        "id": [
                0,
                0,
                0,
                0,
                0,
                0,
                0,
                0,
                0,
                0,
                0,
                0
        ]
}
error: <nil>

Possible solution

Is this behavior is correct? If no:

In encoding/json the encode.go file contains:

case reflect.Array, reflect.Map, reflect.Slice, reflect.String:
		return v.Len() == 0

Since that, any array will never be empty.

Used to fork the package and change it to:

	case reflect.Array:
		for i := 0; i < v.Len(); i++ {
			if !isEmptyValue(v.Index(i)) {
				return false
			}
		}

		return true
		
	case reflect.Map, reflect.Slice, reflect.String:
		return v.Len() == 0
@andybons andybons changed the title Strange encoding/json isEmptyValue behavior encoding/json: strange isEmptyValue behavior Apr 1, 2019
@andybons andybons changed the title encoding/json: strange isEmptyValue behavior encoding/json: strange isEmptyValue behavior with arrays Apr 1, 2019
@andybons
Copy link
Member

andybons commented Apr 1, 2019

I believe the behavior you're seeing is correct.

Since that, any array will never be empty.

A zero-length array is empty:

type EncodingBreaker [0]byte

would have the desired output you expect, however having a zero-length array is likely not what you'd want. I would use slices instead if that works for your use-case.

Changing this behavior would break people who rely on the current encoding semantics with arrays, so it's likely not going to happen.

@dsnet @mvdan in case they have any thoughts.

@andybons andybons closed this as completed Apr 1, 2019
@sorenvonsarvort
Copy link
Author

Thank You for the answer, since some people rely on the current implementation, now I am sure it is okay to have a separate package.

@golang golang locked and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@andybons @gopherbot @sorenvonsarvort