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
Labels
Documentation FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@bep
Copy link
Contributor

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 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 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 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 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 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 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 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
@mvdan mvdan added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 22, 2019
@robpike
Copy link
Contributor

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 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 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

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

@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.
Labels
Documentation FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

5 participants