From 8faf8930b30df3603410889c1aed64c025989553 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Mon, 27 Sep 2021 20:55:12 -0700 Subject: [PATCH] Add `use` unit tests and tweak documentation This explicitly tests a common error case not currently verified by the existing unit tests. Thus improving test coverage by one line. :-) --- pkg/eval/builtin_special.go | 28 +++++++++++++++----- pkg/eval/builtin_special_test.go | 7 ++--- website/ref/language.md | 45 ++++++++++++++++++++++---------- 3 files changed, 56 insertions(+), 24 deletions(-) diff --git a/pkg/eval/builtin_special.go b/pkg/eval/builtin_special.go index f1f43461..94b6ff9e 100644 --- a/pkg/eval/builtin_special.go +++ b/pkg/eval/builtin_special.go @@ -36,9 +36,11 @@ var builtinSpecials map[string]compileBuiltin // intended for external consumption, e.g. the syntax highlighter. var IsBuiltinSpecial = map[string]bool{} -type noSuchModule struct{ spec string } +// NoSuchModule encodes an error where a module spec cannot be resolved. +type NoSuchModule struct{ spec string } -func (err noSuchModule) Error() string { return "no such module: " + err.spec } +// Error implements the error interface. +func (err NoSuchModule) Error() string { return "no such module: " + err.spec } func init() { // Needed to avoid initialization loop @@ -316,7 +318,12 @@ func (op useOp) exec(fm *Frame) Exception { return nil } +// TODO: Add support for module specs relative to a package/workspace. +// See https://github.com/elves/elvish/issues/1421. func use(fm *Frame, spec string, r diag.Ranger) (*Ns, error) { + // Handle relative imports. Note that this deliberately does not support Windows backslash as a + // path separator because module specs are meant to be platform independent. If necessary, we + // translate a module spec to an appropriate path for the platform. if strings.HasPrefix(spec, "./") || strings.HasPrefix(spec, "../") { var dir string if fm.srcMeta.IsFile { @@ -331,6 +338,8 @@ func use(fm *Frame, spec string, r diag.Ranger) (*Ns, error) { path := filepath.Clean(dir + "/" + spec) return useFromFile(fm, spec, path, r) } + + // Handle imports of pre-defined modules like `builtin` and `str`. if ns, ok := fm.Evaler.modules[spec]; ok { return ns, nil } @@ -338,16 +347,21 @@ func use(fm *Frame, spec string, r diag.Ranger) (*Ns, error) { return evalModule(fm, spec, parse.Source{Name: "[bundled " + spec + "]", Code: code}, r) } + + // Handle imports relative to the Elvish module search directories. + // // TODO: For non-relative imports, use the spec (instead of the full path) // as the module key instead to avoid searching every time. for _, dir := range fm.Evaler.LibDirs { ns, err := useFromFile(fm, spec, filepath.Join(dir, spec), r) - if _, isNoSuchModule := err.(noSuchModule); isNoSuchModule { + if _, isNoSuchModule := err.(NoSuchModule); isNoSuchModule { continue } return ns, err } - return nil, noSuchModule{spec} + + // Sadly, we couldn't resolve the module spec. + return nil, NoSuchModule{spec} } // TODO: Make access to fm.Evaler.modules concurrency-safe. @@ -360,7 +374,7 @@ func useFromFile(fm *Frame, spec, path string, r diag.Ranger) (*Ns, error) { code, err := readFileUTF8(path + ".elv") if err != nil { if os.IsNotExist(err) { - return nil, noSuchModule{spec} + return nil, NoSuchModule{spec} } return nil, err } @@ -370,7 +384,7 @@ func useFromFile(fm *Frame, spec, path string, r diag.Ranger) (*Ns, error) { plug, err := pluginOpen(path + ".so") if err != nil { - return nil, noSuchModule{spec} + return nil, NoSuchModule{spec} } sym, err := plug.Lookup("Ns") if err != nil { @@ -378,7 +392,7 @@ func useFromFile(fm *Frame, spec, path string, r diag.Ranger) (*Ns, error) { } ns, ok := sym.(**Ns) if !ok { - return nil, noSuchModule{spec} + return nil, NoSuchModule{spec} } fm.Evaler.modules[path] = *ns return *ns, nil diff --git a/pkg/eval/builtin_special_test.go b/pkg/eval/builtin_special_test.go index fe1b47a5..8981167f 100644 --- a/pkg/eval/builtin_special_test.go +++ b/pkg/eval/builtin_special_test.go @@ -349,13 +349,11 @@ func TestUse_SupportsCircularDependency(t *testing.T) { func TestUse(t *testing.T) { libdir1 := InTempDir(t) - ApplyDir(Dir{ "shadow.elv": "put lib1", }) libdir2 := InTempDir(t) - ApplyDir(Dir{ "has-init.elv": "put has-init", "put-x.elv": "put $x", @@ -410,7 +408,10 @@ func TestUse(t *testing.T) { // modules That("x = foo; use put-x").Throws(AnyError), - // TODO: Test module namespace + // Using an unknown module spec fails. + That("use unknown").Throws(ErrorWithType(NoSuchModule{})), + That("use ./unknown").Throws(ErrorWithType(NoSuchModule{})), + That("use ../unknown").Throws(ErrorWithType(NoSuchModule{})), // Nonexistent module That("use non-existent").Throws(ErrorWithMessage("no such module: non-existent")), diff --git a/website/ref/language.md b/website/ref/language.md index 5bbcbe44..8bff519c 100644 --- a/website/ref/language.md +++ b/website/ref/language.md @@ -169,10 +169,11 @@ writes out `\*`. A string is a (possibly empty) sequence of bytes. [Single-quoted string literals](#single-quoted-string), -[double-quoted string literals](#double-quoted-string)and [barewords](#bareword) -all evaluate to string values. Unless otherwise noted, different syntaxes of -string literals are equivalent in the code. For instance, `xyz`, `'xyz'` and -`"xyz"` are different syntaxes for the same string with content `xyz`. +[double-quoted string literals](#double-quoted-string) and +[barewords](#bareword) all evaluate to string values. Unless otherwise noted, +different syntaxes of string literals are equivalent in the code. For instance, +`xyz`, `'xyz'` and `"xyz"` are different syntaxes for the same string with +content `xyz`. Strings that contain UTF-8 encoded text can be [indexed](#indexing) with a **byte index** where a codepoint starts, which results in the codepoint that @@ -2392,18 +2393,34 @@ itself or defined by the user. ### Importing modules with `use` -Modules are imported using the `use` special command, which accepts a **module -spec** and an optional alias: +Modules are imported using the `use` special command. It requires a **module +spec** and allows a namespace alias: ```elvish use $spec $alias? ``` -Both the module spec and the alias must appear as a single string literal. +The module spec and the alias must both be a simple [string literal](#string). +[Compound strings](#compounding) such as `'a'/b` are not allowed. -The module spec specifies which module to import, and the alias, if given, -specifies the namespace to import the module under. By default, the namespace is -derived from the module spec, by taking the part after the last slash. +The module spec specifies which module to import. The alias, if given, specifies +the namespace to import the module under. By default, the namespace is derived +from the module spec by taking the part after the last slash. + +Module specs fall into three categories that are resolved in the following +order: + +1. **Relative**: These are [relative](#relative-imports) to the file containing + the `use` command. + +1. **User defined**: These match a [user defined module](#user-defined-modules) + in a [module search directory](#module-search-directories). + +1. **Pre-defined**: These match the name of a + [pre-defined module](#pre-defined-modules), such as `math` or `str`. + +If a module spec doesn't match any of the above a "no such module" +[exception](#exception) is raised. Examples: @@ -2525,10 +2542,10 @@ functions. ### Relative imports -The module spec may being with `./` or `../`, which introduce **relative -imports**. When `use` is invoked from a file, this will import the file relative -to the location of the file. When `use` is invoked at the interactive prompt, -this will import the file relative to the current working directory. +The module spec may begin with `./` or `../` to introduce a **relative import**. +When `use` is invoked from a file this will import the file relative to the +location of the file. When `use` is invoked from an interactive prompt, this +will import the file relative to the current working directory. ### Scoping of imports