Convert import to statement instead of pseudo-fun

[YorikSar]

Currently import is treated as a very special function that only accepts literal string as its first argument. It has following downsides:

• Special handling of import is hidden. User can assume that its just a function while it's a special keyword. User might expect to be able to pass variables to it while it is only handled before typechecking and evaluation.
• We can't extend import functionality without introducing new keywords which is not backward-compatible.

This change makes import into another statement like let or fun, which means it cannot be confused with a function anymore. It also means that expressions like import "foo.ncl" bar and import "foo.ncl" & {..} are invalid, and import statement need to be put in parethesis: (import "foo.ncl") bar.

For more context, see discussion in #329 (comment)

[vkleen]

Looks reasonable. We should add the requirement for parentheses around import statements (when they get combined with something else) into the breaking changes list in the release notes.

[YorikSar]

Will do. There's also one spot I've missed in tests.

[aspiwack]

I don't see why the added parentheses are needed, can you elaborate on this part?

[YorikSar]

I don't see why the added parentheses are needed, can you elaborate on this part?

Currently import is syntactically a function, which means every term after it is treated as another argument for function application. So for example import "foo.ncl" from "somewhere" that I’ve tried to implement in the linked issue is treated as passing 2 arguments to the result of the import. We need to somehow distinguish import functionality from just arguments and variables. We can do this by either adding more keywords to the grammar, but that’s not backward compatible. Another option - pull import up to the statement level, in that case it will need to be separated from the rest of expression, just like let or fun.

[aspiwack]

Ah, I see, I misunderstood the context, but reading your original comment again, I see that this is entirely a parsing issue. Specifically, you want to create more parsing rules around import, like import "foo" from "bar" and you're hitting a bit of a snag because it interferes with import foo binding exactly as tight as a function. So import "foo" from "bar" is parsed as passing three argument to import including from.

I don't know which is most valuable: to be able to use import "foo" bar as a function, or to have a somewhat elegant import "foo" from "package" (compared to something like import-from "foo" "package"). This is worth pondering. And now is kind of the right time, as we are reaching 1.0.

In a sense your proposal has the advantage of forward compatibility: if we go for your more restrictive parsing rule, then we can always change our minds later, and make import "foo" bar a thing again. But if we keep the current parsing rule, then implementing your change will be a breaking change.

[YorikSar]

I don't know which is most valuable: to be able to use import "foo" bar as a function, or to have a somewhat elegant import "foo" from "package" (compared to something like import-from "foo" "package"). This is worth pondering. And now is kind of the right time, as we are reaching 1.0.

Another benefit of this approach is that we don't pretend that import is a function even though in Nickel it's a language construct.

In a sense your proposal has the advantage of forward compatibility: if we go for your more restrictive parsing rule, then we can always change our minds later, and make import "foo" bar a thing again. But if we keep the current parsing rule, then implementing your change will be a breaking change.

Exactly. We can work around this in nickel-nix right now, for example, but without this change there would be no hope to add more functionality to import without adding keywords and breaking backward compatibility.

[YorikSar]

@vkleen I've added a line to the release notes, also fixed issue in benchmarks.
On that note, should we have those benchmark typechecks in flake's checks?

[vkleen]

I'm not sure what the original reason was for not having the benchmark typechecking be configured in the flake checks. If there even is a reason 😆

[YorikSar]

I'm not sure what the original reason was for not having the benchmark typechecking be configured in the flake checks. If there even is a reason 😆

I guess, it was just easier. I see that there is benchmark check that runs cargo bench --no-run, but it doesn't include type checking them. Maybe we could hook type check into the benchmark building process...

[aspiwack]

Another benefit of this approach is that we don't pretend that import is a function even though in Nickel it's a language construct.

I really wouldn't use this terminology. “Parsing at the same level as functions” and “pretending that it's a function” are completely different things. And it's actually what caused me to be confused at first.

[yannham]

It's a detail, but we should refrain from using statement I think. Everything is an expression in Nickel, including imports, which do have a value (as opposed to an if statement in e.g. C). It's not too bad in RELEASES.md, but just as a general note.

[YorikSar]

I agree. We should probably rephrase it in RELEASES as well, although I'm not sure how to properly put it to words.