carvel-dev / carvel-dev/ytt

Report error when assigning key to Starlark Builtin

Open
#610 0 comments 1 reaction 0 assignees View on GitHub
enhancement error msg improvement priority/unprioritized-backlog
Dominant language
Go
Stars
1.9k
Forks
167
PR merge metrics
No merged PRs in 30d

Description

**Describe the problem/challenge you have**
When assigning any [starlark builtin](https://github.com/google/starlark-go/blob/7a1108eaa0124ea025e567d9feb4e41aab4bb024/starlark/library.go#L77-L143) without invoking it to a yaml key, a panic is reported. While this is invalid code since its a reserved name in starlark, an error could be reported instead of panicing.

```starlark
key: #@ "".lower <- no parenthesis, just the function name
```

results in:

```go
- (p) unknown type *starlark.Builtin for conversion to go value (backtrace: goroutine 1 [running]:
in
defaults.yml:1 | key: #@ "".lower
...
```
**Describe the solution you'd like**
I would like to see an error that hints at a possible solution:

```go
- Unable to convert value: type *starlark.Builtin (did you mean .lower()?)
in
defaults.yml:1 | key: #@ "".lower
```

**Anything else you would like to add:**
The panic happens in [`starlark_value.go:108`](https://github.com/vmware-tanzu/carvel-ytt/blob/develop/pkg/template/core/starlark_value.go#L108). We should be able to catch this type to report an error.

---
Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

Contributor guide

Open the contributing guide

Research direction

Start at pkg/template/core/starlark_value.go:108 and reproduce the issue with the provided defaults.yml snippet, where a Starlark builtin is assigned without parentheses. Trace the conversion path and add coverage for this input; done means it reports a conversion error with a possible .lower() hint instead of panicking.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.