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

[Feature idea] Add support for common operators in conditional expressions (and maybe elsewhere?) #2411

Open
JohnstonJ opened this issue Oct 4, 2024 · 8 comments

Comments

@JohnstonJ
Copy link

I wanted to write some code like this:

export VCPKG_DEFAULT_TRIPLET := if os_family() == "windows" && arch() == "x86_64" {
    "x64-windows-static"
} else if <snip> {
    <snip>
}

But apparently most common operators aren't implemented beyond what looks like string concatenation (judging from the error message). For example, logical operators would be quite useful here.

error: Expected '{', '+', or '/', but found '&&'
 ——▶ justfile:1:61
  │
1 │ export VCPKG_DEFAULT_TRIPLET := if os_family() == "windows" && arch() == "x86_64" {
  │                                                             ^^

This time, I can work around this with a condition like if os_family() + "-" + arch() == "windows-x86_64" {, but I would imagine there are other scenarios out there where it doesn't work out as cleanly.

It seems that GNU make has similar limitations: https://stackoverflow.com/questions/6451477/makefile-ifeq-logical-and

But I don't know if those limitations from GNU make are worth keeping in just? None of the proposed solutions in the above StackOverflow link look "better" to me than what the original poster was trying to do.

Some operators I could imagine being useful:

  • Logical operators:
    • &&
    • ||
    • ! (unary NOT)
    • maybe an XOR operator
  • Math operators: if the variable holds a number, then do some math on it. (+, -, *, /, etc). But unless just gains a type system, IDK if that's something you'd want to do (else, + is ambiguous with strong concatenation)...

To me, the logical operators would be the most nice ones to have, but maybe keep options open for math operators in the future?

@casey
Copy link
Owner

casey commented Oct 30, 2024

Thanks for opening this issue!

Yah, conditionals are a massive hack. They exist as a special case in the grammar of conditionals, not as expressions. My motivation for implementing them in this terrible, awful, no good way, was because I've wanted to avoid committing to the representation of true and false in just. If logical operations are just allowed in conditionals, you can't get the result of evaluating one, so we don't need to decide what they evaluate too.

This is tricky, because I can imagine that one might want && and || to behave in ways which aren't exactly the same as a logical operator that only takes true and false.

For example, you might want:

"a" || "" -> "a"
"" || "b" -> "b"
"a" || "b" -> "a"
"" || "" -> ""

"a" && "" -> ""
"" && "b" -> ""
"a" && "b" -> "b"
"" || "" -> ""

So || and && behave as lazily-evaluated string-coalescing operators in expression contexts. I think this is actually the most natural, given that just is a string-oriented langauge, and this is what python and and or do.

For example, with the inbound PR #2440, you could do:

make := which("gmake") || which("make")

I.e., get make, preferring gmake if it exists, but otherwise falling back to make.

Whereas if || was a logical, this would evaluate to true/false or whatever the string representation of bools was.

@laniakea64
Copy link
Contributor

So || and && behave as lazily-evaluated string-coalescing operators in expression contexts. I think this is actually the most natural,

Having the empty string be the "falsy" value wouldn't mix well with some of the existing syntaxes:

foo_path := "/something/that/evaluates/to/a/nonexistant/path"

# because path_exists() returns the string "false" for nonexistant paths,
# this would evaluate to foo_path
# but the most natural-seeming evaluation would be "/fallback"
foo := path_exists(foo_path) && foo_path || '/fallback'

Also, different justfiles may need different definitions of "falsy" (for example, the string "0" is truthy as an exit code stored in a backtick but falsy in a boolean 1/0 env var).

Maybe could let the user decide what strings are falsy on a per-justfile basis? -

set false-regex := '^\s*(?:[Ff]alse|0)\s*$'

To avoid surprises when using functions like path_exists(), this setting could default to '^(false)$' to match only empty strings and the string "false" by default.

@casey
Copy link
Owner

casey commented Oct 31, 2024

Yeah, I've ben thinking about our current functions that return true / false. They are is_dependency, path_exists, semver_matches.

My current thinking is that having them return true / false is actually just a mistake. Playing with || and &&, I think the empty string is just a much better false value.

I'm hesitant to introduce something like false-regex, because if users miss that setting, and mix is_path with && the behavior they get is quite annoying and hard to figure out.

One option would be to deprecate those three functions, and introduce new versions which return true and the empty string, like, is_recipe_dependency, does_path_exist or (exists), and is_semver_match. These are not amazing names, but in this scenario, if you used the old versions, which didn't play nice with && and ||, you would get a warning, which seems less error prone.

I think this is something that's worth thinking hard about.

Just only has a single type, the string, and using strings to represent other types is not ideal, precisely because of issues like these. (An additional issue is the case where the empty string is a valid value, so doing something like foo || fallback can't be used in that case.)

I think using "true" / "false" is a mistake, just in the sense that the empty string is a better choice for false.

Another thought is that I've often thought that perhaps the base type should actually not be strings, but lists of strings. (See the rc shell, which does this.) In which case it wouldn't be the empty string which is the best representation of false, but the empty list.)

So, to stop rambling and sum up:

  1. We could deprecate the three functions that return true and false and make new versions that return the empty string for `false.
  2. We could add a setting like false-regex (control what just considers to be false) , or a setting like empty-false (make those three functions return the empty string for false)
  3. We could write a static type system for just, so we can have more than one type.
  4. I opened PR Add && and || operators #2444 to add && and ||, and in lieu of figuring this out, I could land it as unstable, with a big warning in the readme documenting the unfortunate situation with the three functions which use false as the return value.

A static type system would be great, but it's a huge undertaking, and although I did take a compilers class in college where we learned about type systems and wrote a compiler for a statically-typed python-like language, that was a long time ago and I've basically forgotten everything 😅

Any other options I'm missing?

@laniakea64
Copy link
Contributor

laniakea64 commented Nov 1, 2024

  1. We could deprecate the three functions that return true and false and make new versions that return the empty string for `false.

That would be weird and seems unnecessary. With such duplication, the use of the new functions represents an explicit opt-in to the new return value. In which case there would be no need to have both the old and new functions simultaneously available for backwards compatibility, since justfiles using the new functions are updated anyway, and the functions would be identical in behavior all the way until the exact text of the true/false return values.

And as you pointed out, the new functions would need to have "not amazing" names, since the existing functions took the clear, concise names. That the "better" functions have the worse names adds even more confusion.

I could land it as unstable, with a big warning in the readme documenting the unfortunate situation with the three functions which use false as the return value.

Landing as unstable sounds like a good idea for at first 👍 regardless of the ultimate path forward.

Any other options I'm missing?

  1. Add a single boolean setting that:
  • is required to enable use of && and || operators,
  • changes the "false" return values of the just functions to a falsy string,
  • allows conditionals without conditional operator, since strings can now be truthy/falsy:
foo := if path_exists("/some/path") { "It does exist" } else { "It doesn't exist" }

Not sure what would be a good name for this setting?

@casey
Copy link
Owner

casey commented Nov 1, 2024

That would be weird and seems unnecessary. With such duplication, the use of the new functions represents an explicit opt-in to the new return value. In which case there would be no need to have both the old and new functions simultaneously available for backwards compatibility, since justfiles using the new functions are updated anyway, and the functions would be identical in behavior all the way until the exact text of the true/false return values.

Wouldn't you need both the new and the old functions? Maybe I'm misunderstanding, but removing the old functions would break old justfiles which use them.

And as you pointed out, the new functions would need to have "not amazing" names, since the existing functions took the clear, concise names. That the "better" functions have the worse names adds even more confusion.

This is true, but a deprecation warning, along with removing the old functions from the docs would hopefully minimize that.

Landing as unstable sounds like a good idea for at first 👍 regardless of the ultimate path forward.

I'll do that, just so I can get the PR in and then we can worry about the larger problems later.

  1. Add a single boolean setting that:
  • is required to enable use of && and || operators,
  • changes the "false" return values of the just functions to a falsy string,
  • allows conditionals without conditional operator, since strings can now be truthy/falsy:
foo := if path_exists("/some/path") { "It does exist" } else { "It doesn't exist" }

Not sure what would be a good name for this setting?

I agree that the behavior of a setting like that is the end state that we want to get to, but I hesitate to add a setting like that because I think that you really just want it to be the default.

It would also mean that you would have to remember to add it to your justfile every time, and restricting the use of && and ||, changing the values of boolean functions, and allowing conditionals without an operator, would mean that in those places in the codebase, you would have to check what the setting was.

You could definitely do that, but it would be annoying. For example, the natural place to restrict && and || is in the parser, but at that stage you don't know what the settings are, because you haven't parsed them yet, so you would have to defer checking the setting until the justfile was parsed.

Having worse names for three functions is lame, but seems less annoying and invasive to me.

@casey
Copy link
Owner

casey commented Nov 1, 2024

Landed && and || as unstable in #2444.

@laniakea64
Copy link
Contributor

Wouldn't you need both the new and the old functions? Maybe I'm misunderstanding, but removing the old functions would break old justfiles which use them.

Settings are the usual answer for opting into backwards-incompatible behavior. Having a setting to alter the functions' falsy return value would avoid duplication confusion and preserve the easy-to-understand existing function names.

With the only difference being in the string representation of one specific return value, it would seem the rationale that would favor deprecating instead of a setting would be if migrating to the new functions is onerous, i.e. justfiles may need the option of a transition period of gradually migrating the function calls instance-by-instance instead of doing it all at once. But not seeing how that would be for only changing "false" to "" 😕

I hesitate to add a setting like that because I think that you really just want it to be the default.

It would also mean that you would have to remember to add it to your justfile every time,

For what it's worth I had #1201 in mind when thinking of that setting idea

the natural place to restrict && and || is in the parser, but at that stage you don't know what the settings are, because you haven't parsed them yet, so you would have to defer checking the setting until the justfile was parsed.

Having worse names for three functions is lame, but seems less annoying and invasive to me.

Oops, didn't realise the implementation impact 😳

If the functions are going to be deprecated in favor of new ones, then instead of basically renaming, what about switching to non-boolean return values to prevent this problem recurring in face of a better (or different canonical) representation/idea of "truthy"?

This could reduce the negative impact of not being able to use the existing function names.

As examples of this idea:

  • path_exists(path) -> path_if_exists(path), returning the path argument unchanged if the path exists, and the empty string if the path does not exist.
  • is_dependency() -> depending_recipe(), returning the name of the recipe that invoked this one as a dependency, or an empty string if the recipe is not a dependency.

For semver_matches(version, req) no idea how to adapt it along this line 😨

@casey
Copy link
Owner

casey commented Nov 1, 2024

I think I overstated the implementation impact. It's actually not so bad to note use of unstable features in the parser, and convert use of unstable features into errors in the analyzer later.

Let me rephrase I think the main reason I prefer deprecating the old functions. If the functions are deprecated and new versions with better behavior are introduced, the impact is:

  1. Users who use the three old functions have to switch to the new functions.
  2. The names of three functions will be slightly worse going forward.
  3. Two version of the functions exist in the codebase.

1 is annoying, but it's a one-time cost, 2 is ongoing but I think pretty marginal, and 3 is ongoing but also pretty marginal.

Whereas with a setting:

  1. If you want to use the new logical operators, you have to add a setting to your justfile.
  2. The setting and different behaviors of the three functions need to be documented.

I think these are both slightly more annoying drawbacks than with deprecation, and they're ongoing, users will forever have to add the setting if they want to use the logical operators.

Not to overstate my case though! I think a setting isn't a terrible idea, and deprecation is annoying. You all of a sudden start getting warnings for things that used to work.

Rolling it into a hypothetical future edition does make it better, since multiple such changes can be bundled into a single setting, but it would still be better if we could avoid gating things behind editions, i.e., they're kind of a last-ditch solution for things that absolutely cannot be done in a backwards compatible way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants