LSP complains about missing environment variables

[akshaymankar]

Most of the time when I am writing dhall, I don't have the secrets/configs that I want to source from the environment, and it would be nice if I could keep it that way, otherwise setting up Emacs/Vim would become more complex that I'd like.
So, I was wondering if it would be better if LSP just ignored env import errors and continued validating rest of the expression.

[Gabriella439]

@akshaymankar: You can use the alternative import operator to provide a fallback expression if import resolution fails, like this:

env:SOME_SECRET as Text ? "fallback value"

[akshaymankar]

It would work for things which can have defaults. For things which cannot, I'd really like them to fail when environment variables are not provided while resolving expressions with dhall or dhall-to-json.

[sjakobi]

I guess we could make an option where env:X as Text is resolved as "stub-X" or something like that.

"Skipping" env-import resolution in general seems tricky.

[Gabriella439]

@sjakobi: Note that that is already possible by setting the environment variable in the environment the language server runs in:

X=stub-X dhall-lsp-server

[sjakobi]

@Gabriel439 I imagine that this would become somewhat tedious when there are many env-imports.

I wanted to suggest an option where every import of the form env:... as Text is resolved to some stub value.

In principle, something similar could be done for as Location imports but there's probably less need.

[Gabriella439]

@sjakobi: We don't actually have to resolve the import to type-check the file in that case. We only need the type of the import (which we can infer from the as clause) to type-check the file.

[sjakobi]

I thought we need the/a value when the import appears in a let-binding? Or would you modify the type-checker for this?

[Gabriella439]

@sjakobi: Conceptually we could modify the type-checker to support an expression with unresolved imports, so long as all imports were of the form as X. We could even standardize that

[sjakobi]

That might actually be a nice idea – but it also sounds quite a bit more complex than the stubbing…

[Gabriella439]

@sjakobi: Stubbing in a fake value seems too unprincipled to me and could lead to unintended consequences. Faking success with an in-band value (e.g. 0, [], nil, etc.) instead of failing loudly is a common source of programming errors.

[sjakobi]

I don't see how it's much worse than asking the user to provide fake values – which admittedly we could also simplify by accepting env-vars from a .env-file or something like that.

[sjakobi]

Maybe I should ask: How do you work on Dhall configurations that env-import secrets @Gabriel439?

[Gabriella439]

@sjakobi: I provide a fallback import using ?. In fact, usually the fallback import is actually the main import and the env import is just an optional higher-priority override

[sjakobi]

@Gabriel439 Then you probably don't import any secrets via env:, right?

Maybe we should find out how other users who configure secrets via Dhall access them and how they replace them during development.

[Gabriella439]

@sjakobi: Yeah, that's correct. We currently don't use dhall for secret management

[sjakobi]

@akshaymankar Do you already have a workaround for this issue? I think you could otherwise easily set stub values with a script based on

dhall resolve --transitive-dependencies | grep '^env:'

---

@Gabriel439 Could you elaborate on what kind of bad things you believe could happen if we add an option that fakes env:X as Text imports? I wonder whether we can limit the option in a way that would prevent these problems.

Regarding the idea of delaying import resolution – I'm not yet convinced that it's worth giving up the current neat phase separation of import resolution -> type-checking -> normalization.

[Gabriella439]

@sjakobi: It wouldn't be violating phase separation. It can be done entirely within the type-checking phase without resolving any imports. It's actually a tiny change to the standard to implement, which is just add a type-checking judgment for an as X import

[sjakobi]

Ah, I see! So we'd have something like a --no-resolve-typed-env-imports option?! Assertions using these imports could simply fail – they'd probably be fairly useless anyway.

[Gabriella439]

@sjakobi: The main change is that the type of the type-checking function would change to accept either X or Import as the type parameter. Normalization would still require an X, so this feature would only be useful for the dhall type subcommand (since it does not normalize the expression).

It also wouldn't require a flag: it could just be the default behavior for dhall type because it doesn't need to resolve those imports to infer those types

[akshaymankar]

@sjakobi I faced this problem while writing concourse pipelines in Dhall. I was resolving my secret's after translating dhall to yaml using concourse's CLI by putting "((some-secret))" in the dhall code. As I write this comment I realize I could've just added "((some-secret))" as a fallback of my env import and made concourse CLI fail on unresolved variables.

Generating stubs just before starting the lsp-server is definitely a better workaround in terms of use cases. However, it would very likely require restarting the lsp-server while writing code, which would be a little inconvenient but most LSP frontends have an easy restart shortcut. I might end up wrapping lsp-server with some bash which does this. I am sure I will need this workaround sooner or later as I write more things in dhall 😄 (Kubernetes is next, I think).

[EggBaconAndSpam]

Generating stubs just before starting the lsp-server is definitely a better workaround in terms of use cases. However, it would very likely require restarting the lsp-server while writing code, which would be a little inconvenient but most LSP frontends have an easy restart shortcut. I might end up wrapping lsp-server with some bash which does this. I am sure I will need this workaround sooner or later as I write more things in dhall smile (Kubernetes is next, I think).

If we really wanted to we could easily add a list of env's to be stubbed as a config option, so you wouldn't have to reload anything. With per-workspace configuration that could be quite usable.

Come to think of it, being able to specify environment variables to Dhall in the LSP config would be useful in any case! Waiting on #1533.