cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.11k stars 291 forks source link

time.Format does something that is not intuitive #1508

Open bttk opened 2 years ago

bttk commented 2 years ago

I see no effective method for manipulating dates in CUE.

time.Parse helps ingest strings with time values, but it also returns a string, which needs parsing to be useful.

time.Parse(layout, value string) (string, error)

time.Format is even less useful: it can be used to create a value representing a set of all strings matching the specified layout.

func Format(value, layout string) (bool, error)

Notice that Parse and Format take arguments in different orders.

What version of CUE are you using (cue version)?

$ cue version
cue version v0.4.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you see?

-- in.cue --
import "time"

format: time.Format("2022-01-31", time.RFC3339Date)
parse: time.Parse(time.RFC3339Date, "2022-01-31")
-- log --
$ cue export in.cue
{
    "format": true,
    "parse": "2022-01-31T00:00:00Z"
}

What did you expect to see?

-- in.cue --
import "time"

format: time.Format(parse, "02/01/2006")
parse: time.Parse(time.RFC3339Date, "2022-01-31")
split: time.SplitParts(parse)
-- log --
$ cue export in.cue
{
    "format": "31/01/2022",
    "parse": "2022-01-31T00:00:00Z"
    "split": {
        "year": 2022,
        "month": 1,
        "day": 31,
        "hour": 0,
        "minute": 0,
        // ...
    }
}
antong commented 2 years ago

I think time.Format is meant to be used to specify constraints on the format of string representations of dates and timestamps, and for that use it is quite useful and intuitive. For instance:

import "time"

#Measurement: t: time.Format(time.RFC3339)
#Measurement: value: float

data: [ ...#Measurement ]

data: [
    {t: "2022-02-09T12:34:56Z", value: 23.7 },
    {t: "2022-02-09T12:34:58Z", value: 27.8 },
    {t: "2022-02-09T12:35:00Z", value: 27.7 },
    // {t: "2022-02-32T12:36:00Z", value: 27.7 }, // -> ... time.Format: invalid time "2022-02-32T12:35:00Z"
]

There is also #319 that could help manipulating dates and timestamps. But are you looking for something like a time.FormatString() that would return a string? I don't know what the arguments would be. For it to work in a general case it would have to know about locations / time zones. JSON and CUE don't have time types so have to rely on numbers and strings. It could be an interesting thing to think about how the hermetic CUE could handle something like externally defined time zones that may change suddenly with political decisions.

bttk commented 2 years ago

Maybe time.Format just needs a different name? In its current form it is ambiguous whether "Format" is used as a noun or a verb.

The name time.FormatString(layout, value) is clear in that it is not used as a constraint.

Some ideas for the name of the constraint:

It may be a good convention to use boolean-y IsFoo() names for constraints, but how to distinguish constraints from actual boolean values?

mpvl commented 2 years ago

@antong @bttk I agree the naming is unfortunate. Intuitively, Format indeed seems to imply an action.

It is a bit hard to change the names, as it would break configurations, but we are pre-v1, so we can do it with a bit of transition plan.

This is one potential path:

Note that we already have time.Time and time.Duration as validators. Note also that CUE now (still unofficially) supports variable-length args. It is not possible to use this for validators, but we plan to introduce name args (see #943), so we could support something like time.Time(format: "....") as an alternative to time.Format. The advantage of this approach is that there will be only one builtin for defining a field needs to be a time.

This is a bit ugly for boolean checks, but ultimately it is probably nicer to have separate builtins for validators and simple boolean checks, as it just reads better.

So the path would be:

Phase 1: Introduce time.IsFormat and/or the multi-arg time.Time. cue fix to rewrite usages of time.Format to the appropriate one.

Introduce time.FormatString to be what time.Format should be.

Phase 1b: The use of time.Format would generate an error overridable with an env var.

Phase 2: Officially Deprecate time.Format

Phase 3 (a few 0.x.0 versions later): Reintroduce time.Format, now with the new meaning. Deprecate time.FormatString. cue fix to rewrite usages of time.FormatString to the new time.Format.

Phase 4: Remove time.FormatString.

mpvl commented 2 years ago

So the easiest to provide the functionality for now is to introduce time.FormatString to mean you foreseen semantics of time.Format, with the intent to transition later to time.Format.

mpvl commented 2 years ago

As for SplitParts, I would just simply call that Split. I think time.Split reads well.

antong commented 2 years ago

I also think it would be good if there was some clearer way to directly see that, and how a function can be used as a validator. I think it is so that if the function returns bool (+ perhaps an error), or *Bottom, then you can use it as a validator by omitting the first argument in the CUE invocation. But this is not really that intuitive. When used correctly these validators look really clear and nice, but when looking at the package reference doc, at least for me it isn't until you see an example.

mpvl commented 2 years ago

Yes, indeed, this is sorely lacking documentation. This is certainly on the todo list.

mpvl commented 2 years ago

My plan was to document this as part of a more broader function refinement that is alluded to in #943

mpvl commented 2 years ago

The 0.4.3 part of this bug is fixed, moving to 0.5.x

myitcv commented 1 year ago

The v0.5.x milestone is now closed because we have moved onto focus on v0.6.0. Hence moving this to v0.6.x because this didn't get done in the stretch goal that is v0.5.x.