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

builtin: len godoc can lead the reader to believe that len(nil) is valid #30349

Closed
bep opened this issue Feb 22, 2019 · 11 comments
Closed

builtin: len godoc can lead the reader to believe that len(nil) is valid #30349

bep opened this issue Feb 22, 2019 · 11 comments

Comments

@bep
Copy link
Contributor

@bep bep commented Feb 22, 2019

package main

import (
	"log"
	"os"
	"text/template"
)

func main() {
	log.SetFlags(0)
	
	data := make(map[string]interface{})

	tpl, err := template.New("").Parse(`{{ len .Foo  }}`)
	if err != nil {
		log.Fatal(err)
	}
	err = tpl.Execute(os.Stdout, data)
	if err != nil {
		log.Fatal(err)
	}
}

This fails with:

template: :1:3: executing "" at <len .Foo>: error calling len: len of untyped nil

This behaviour does not match the documented behaviour at https://golang.org/pkg/builtin/#len

if v is nil, len(v) is zero.

gohugoio/hugo#5708

@robpike
Copy link
Contributor

@robpike robpike commented Feb 22, 2019

https://play.golang.org/p/3-iF20WnP2I demonstrates the same behavior in Go without templates.

It might indeed need a tweak to the spec. I'm not sure. Let's ask @griesemer

@bep
Copy link
Contributor Author

@bep bep commented Feb 22, 2019

I think you should adjust the implementation of len to be in line with its specification. Go templates can never mirror the Go spec (this should be a good example). Untyped nils are way less obvious in a dynamic, non-static runtime.

@bep
Copy link
Contributor Author

@bep bep commented Feb 22, 2019

But then again, I assume that a range over that same nil would fail, so I'm not sure ...

@mvdan
Copy link
Member

@mvdan mvdan commented Feb 22, 2019

Reading the builtin godoc linked above:

Array: the number of elements in v.
Pointer to array: the number of elements in *v (even if v is nil).
Slice, or map: the number of elements in v; if v is nil, len(v) is zero.
String: the number of bytes in v.
Channel: the number of elements queued (unread) in the channel buffer;
if v is nil, len(v) is zero.

The way I'm reading it is that "if v is one of the types above and its value is nil, len(v) is zero". Perhaps that should be made clearer. In any case, I don't think it makes sense to allow len(nil) in Go or templates.

@mvdan
Copy link
Member

@mvdan mvdan commented Feb 22, 2019

Actually, I misread the punctuation. It's actually saying:

  • Array: the number of elements in v.
  • Pointer to array: the number of elements in *v (even if v is nil).
  • Slice, or map: the number of elements in v; if v is nil, len(v) is zero.
  • String: the number of bytes in v.
  • Channel: the number of elements queued (unread) in the channel buffer; if v is nil, len(v) is zero.

So this seems like just a formatting issue in the rendered godoc.

@antong
Copy link
Contributor

@antong antong commented Feb 22, 2019

#7873 has a proposal for lists. The preformatted style intended for source code is very awkward for lists like this.

@mvdan
Copy link
Member

@mvdan mvdan commented Feb 22, 2019

Thanks for the link, that's quite interesting. Though I think we can improve the situation here without waiting for a resolution to that issue. For example, we could make the channel line longer, or we could indent its continuation:

String: the number of bytes in v.
Channel: the number of elements queued (unread) in the channel buffer;
        if v is nil, len(v) is zero.

@mvdan mvdan changed the title text/template: error calling len: len of untyped nil builtin: len godoc can lead the reader to believe that len(nil) is valid Feb 22, 2019
@robpike
Copy link
Contributor

@robpike robpike commented Feb 23, 2019

I do not think there is a compelling need to make len(nil) be zero. Remember an untyped nil is literally the constant nil; asking its length is pointless.

Other than possibly reformatting the text as suggested, I see nothing here worth doing.

@ghost
Copy link

@ghost ghost commented Feb 25, 2019

Other than possibly reformatting the text as suggested, I see nothing here worth doing.

Makes sense. I think the confusion is that Hugo users may call range on an untyped nil and IIRC it doesn't error whereas a len check will error. Handling pointer references in go templates is a gotcha devs will need to learn unless there's some kind of affordance made to len in templates to make the following (example from Hugo) work without the use of default:

{{ range first 1 (default slice .Site.Params.images) }}
  <image>
    <url>{{ . }}</url>
  </image>
{{ end }}

guitmz pushed a commit to guitmz/after-dark-green that referenced this issue Mar 12, 2019
pointer assignments return nil by design golang/go#30349

closes #136
updates #135
@mvdan
Copy link
Member

@mvdan mvdan commented Mar 13, 2019

@robpike note that the apparent consensus we reached is not about allowing len(nil), but rather about making len's documentation clearer. I'll send a CL shortly.

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 13, 2019

Change https://golang.org/cl/167403 mentions this issue: builtin: make len's godoc less ambiguous

@gopherbot gopherbot closed this in 1a24bf1 Mar 13, 2019
@golang golang locked and limited conversation to collaborators Mar 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants