luau icon indicating copy to clipboard operation
luau copied to clipboard

ConfigResolver / `Luau::parseConfig` should store location of resolved config alongside alias

Open JohnnyMorganz opened this issue 1 year ago • 1 comments

I would like to start implementing alias resolution via .luaurc for Luau LSP.

We already use Luau::ConfigResolver from Luau.Analysis, so it seems like it would be best to continue using that. Luau LSP essentially replicates the same CliConfigResolver used for luau-analyze: https://github.com/luau-lang/luau/blob/e8a7acb8023d3fa30d7d6c0fbbe6fb98faae49a1/CLI/Analyze.cpp#L193

The problem right now, however, is that once we want to start resolving aliases, we do not have the correct location to resolve it relative to. The aliases RFC states (https://github.com/luau-lang/rfcs/blob/master/docs/require-by-string-aliases.md):

if an alias is bound to a relative path, the path will be evaluated relative to the .luaurc file in which the alias was defined.

However this location is not preserved via Luau::parseConfig, making the Config datastructure unsuitable for alias resolution in analyse.

The Luau REPL does not seem to have this issue because it decides to implement its own recursive config resolution, and ends up storing and later referencing lastSearchedDir:

  • https://github.com/luau-lang/luau/blob/e8a7acb8023d3fa30d7d6c0fbbe6fb98faae49a1/CLI/Require.cpp#L256
  • https://github.com/luau-lang/luau/blob/e8a7acb8023d3fa30d7d6c0fbbe6fb98faae49a1/CLI/Require.cpp#L253

As an aside, do you folks intend to support the new require-by-string + aliases in luau-analyze soon? I imagine you will hit this same issue once you do so.

JohnnyMorganz avatar Sep 15 '24 10:09 JohnnyMorganz

Hey, I'll take a look at this. I may have to wait until this RFC is all finalized and merged in, though: https://github.com/luau-lang/rfcs/pull/56.

vrn-sn avatar Sep 23 '24 19:09 vrn-sn

Saw the introduction of Alias options in the latest sync, nice! But it wasn't included in release notes yet. Is it ready for use?

JohnnyMorganz avatar Nov 09 '24 11:11 JohnnyMorganz

Yes! AliasOptions can be passed to parseConfig, and the Config itself will store the deduped config locations in the new AliasInfo struct.

If you want to see how it might be used in action, Require.cpp was refactored a bit to use this.

  • parseConfigInDirectory now creates and passes an AliasOptions struct when parsing a new config file.
  • getAlias now resolves the alias relative to the stored configLocation, rather than lastSearchedDir.

As an aside, do you folks intend to support the new require-by-string + aliases in luau-analyze soon? I imagine you will hit this same issue once you do so.

Also, regarding this, require-by-string semantics for luau-analyze are implemented—will land in the next release.

vrn-sn avatar Nov 12 '24 20:11 vrn-sn