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

proposal: log: Add a generic Must method #58280

Closed
imjasonh opened this issue Feb 2, 2023 · 3 comments
Closed

proposal: log: Add a generic Must method #58280

imjasonh opened this issue Feb 2, 2023 · 3 comments

Comments

@imjasonh
Copy link

imjasonh commented Feb 2, 2023

I'd like to propose adding methods that take advantage of generics to make the Must idiom more widely used across the ecosystem.

The text/template package has a Must method for example, which takes a *Template and an error. If the error is not nil, it panics. If the error is nil, the template is returned.

This enables a more readable form of parsing and using a template if you expect it to succeed, as in the example:

var t = template.Must(template.New("name").Parse("text"))

Because Parse returns (*Template, error), these are passed to Must, and if successful, the *Template is returned and stored as a var.

This is commonly done because templates are defined as constants in code or maybe loaded at init-time, and are expected to be parseable. If they're not, it's programmer error and not user error, and the code should panic.


This idiom can be made generic with a method that takes a T any and an error, and if successful, return the T, otherwise panic.

Based on template.Must:

func Must[T any](t T, err error) T {
  if err != nil {
    panic(err)
  }
  return t
}

I think this method makes sense in the log package, where it can call Fatal or Panic, and be called like so:

var t = log.Must(template.New("name").Parse("text"))

regexp has its own variation, MustCompile, which could be replaced with:

var re = log.Must(regexp.Compile(`^[a-z]+\[[0-9]+\]$`)

Being reusable, folks can plug their own (T, error)-returning methods into log.Must:

var u = log.Must(url.Parse("https://golang.org"))
var ref = log.Must(name.ParseReference("ghcr.io/foo/bar"))

(The impetus for name.MustParseReference)

Or more fancifully, purely for illustration:

func rollDice() (string, error) {
  n, m := rand.Intn(6), rand.Intn(6)
  if n == 0 && m == 0 {
    return "", errors.New("snake eyes!")
  }
  return fmt.Sprintf("%d", n+m), nil
}

func main() {
  log.Println("rolled:", log.Must(rollDice()))
}

Notes

  • This doesn't allow for any additional message to be added by the programmer, since Must(string, T, error) can't be called like Must("uh oh", rollDice()).
  • The same idiom couldn't be supported in *testing.T since T would have to take the type for t.Must to know what to return. Maybe there's a more clever way I'm not seeing though.

Open Questions

  • Should it call log.Fatal or log.Panic? Just panic?

Alternatives

Things that must be true before a program can proceed are handled today with the ubiquitous if err != nil and (often) log.Fatal. If a package-level var should be populated by something that can fail but mustn't, this can be done with an init that log.Fatals. If there are many things that must parse, this can get verbose and error-prone to write repeatedly in an init block.

@gopherbot gopherbot added this to the Proposal milestone Feb 2, 2023
@antichris
Copy link

syntax error: method must have no type parameters

With the current implementation of type parameters ("generics") having the above limitation, it would only be possible to implement this proposal for the package-wide Default() Logger. Many applications pass around and use non-default, customized Logger instances instead.

@mvdan
Copy link
Member

mvdan commented Feb 3, 2023

I think we should have one umbrella proposal for a generic must helper, no matter the package it ends up in. #54297 was already filed last year, so I'd personally suggest to continue using that thread so that we don't split the discussion.

@imjasonh
Copy link
Author

imjasonh commented Feb 3, 2023

I think we should have one umbrella proposal for a generic must helper, no matter the package it ends up in. #54297 was already filed last year, so I'd personally suggest to continue using that thread so that we don't split the discussion.

Oh great, I didn't find that before. I'll close this in favor of that then. 😄

Thanks!

@imjasonh imjasonh closed this as completed Feb 3, 2023
@golang golang locked and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants