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

text/template: handling of nil keyword #58326

Closed
mxxk opened this issue Feb 4, 2023 · 5 comments
Closed

text/template: handling of nil keyword #58326

mxxk opened this issue Feb 4, 2023 · 5 comments
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mxxk
Copy link

mxxk commented Feb 4, 2023

What version of Go are you using (go version)?

$ go version
go version go1.19.5 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/user/gLibrary/Caches/go-build"
GOENV="/Users/user/gLibrary/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/user/g.asdf/installs/golang/1.19.5/packages/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/user/g.asdf/installs/golang/1.19.5/packages"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/user/g.asdf/installs/golang/1.19.5/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/user/g.asdf/installs/golang/1.19.5/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.19.5"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/kq/zqxnkcz94n97q3qrpvtl16l00000gn/T/go-build3844177246=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

These templates were evaluated to demonstrate how the argument nil is handled:

  • {{nil}}
  • {{print nil}}
  • {{nil | print}}
Source (go.dev/play/p/ZUCywuoOq7E)
package main

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

var templates = []string{
	// Examples using `nil`
	"{{nil}}",
	"{{print nil}}",
	"{{nil | print}}",
	// Examples using `true`, for comparison
	"{{true}}",
	"{{print true}}",
	"{{true | print}}",
}

func main() {
	for i, templateSource := range templates {
		name := fmt.Sprintf("Template %d", i)
		tmpl := template.Must(template.New(name).Parse(templateSource))
		fmt.Printf("# %s\n", name)
		if err := tmpl.Execute(os.Stdout, nil); err != nil {
			fmt.Println(err)
			continue
		}
		fmt.Println()
	}
}

What did you expect to see?

According to the text/template docs, nil is a valid standalone argument:

Arguments

An argument is a simple value, denoted by one of the following.

  • A boolean, string, character, integer, floating-point, imaginary or complex constant in Go syntax. [...]
  • The keyword nil, representing an untyped Go nil.

Based on the above, these are expected to render:

Template Expected result
{{nil}} <no value> (how nil value renders)
{{print nil}} <nil>
{{nil | print}} <nil>

What did you see instead?

Template Actual result
{{nil}} Error: "Template 0": at <nil>: nil is not a command
{{print nil}} <nil>
{{nil | print}} Error "Template 2": at <nil>: nil is not a command

If the observed behavior is actually correct/intentional, then should the docs be updated to reflect this?

@ianlancetaylor
Copy link
Contributor

I agree that the behavior does not seem to match the documentation, but as the current behavior seems reasonable I think we should probably just adjust the documentation.

CC @robpike

@ianlancetaylor ianlancetaylor added Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 5, 2023
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Feb 5, 2023
@mxxk
Copy link
Author

mxxk commented Feb 5, 2023

Thanks @ianlancetaylor. Would you mind sharing your thoughts on how the current behavior seems reasonable? Or perhaps it is simply easier to adjust the docs rather than the logic for compatibility purposes?

Personally, the handling of nil (as opposed to other keyword constants, such as true) seems a bit confusing/surprising, but perhaps I'm missing something here.

Stepping back a bit, the current documentation seems to suggest that {{nil}} should renders to <no value> based on the following excerpts:

Documentation snippets
  1. An action may be a pipeline:

    Actions

    {{pipeline}}
           The default textual representation (the same as would be
           printed by fmt.Print) of the value of the pipeline is copied
           to the output.
    
  2. A pipeline may be an argument:

    Pipelines

    A pipeline is a possibly chained sequence of "commands". A command is a simple value (argument) or a function or method call, possibly with multiple arguments:

    Argument
    	The result is the value of evaluating the argument.
    
  3. An argument may be the keyword nil:

    Arguments

    An argument is a simple value, denoted by one of the following.

    • The keyword nil, representing an untyped Go nil.

Presently, the cases below seem non-trivial to justify, for example:

Template Result Justification
{{print nil}} <nil> nil is a valid argument to a function
{{nil}} Error nil is not valid as a standalone argument
{{print true}} {{true}} Non-nil constants are valid in both of the above contexts

@ianlancetaylor
Copy link
Contributor

It seems reasonable to me because {{nil}} doesn't have a type, and because there is code in text/template that is clearly implementing this behavior and there are tests that are checking for it. My experience with text/template is that changing it causes people's existing templates to break, though I admit I don't see how that could happen in this case.

I'm not strongly opposed to changing the behavior, but I would not be at all surprised if we have to roll it back.

Perhaps I should ask: what is your use case here?

@mxxk
Copy link
Author

mxxk commented Feb 5, 2023

Appreciate the clarification!

Perhaps I should ask: what is your use case here?

I actually came across this behavior on accident, without a concrete use case to stake on. I was experimenting with {{define "helper"}}...{{end}} followed by {{template "helper}} and {{template "helper" nil}}, and was surprised that the former passed nil to "helper" while the latter errored.

The question in this issue appeared by isolating the behavior to a minimal reproducible example. 🙂

@robpike
Copy link
Contributor

robpike commented May 11, 2023

See my closing comment in #57773. Infeasible.

@robpike robpike closed this as completed May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants