From ead60c003ec6ad9bf6a77581e2e2567cc23c3818 Mon Sep 17 00:00:00 2001 From: Erica Marigold Date: Fri, 13 Dec 2024 14:33:05 +0000 Subject: [PATCH] refactor(lib): use extension pattern for result<->option Formerly, we used metatables to get custom `Option` and `Result` objects which were difficult to type properly, leading to a lot of `unknown` and `any` casts. This refactor fixes it by making extensions opt-in, where we import the extension methods separately from the original implementations, thereby allowing us to not have to typecast things everywhere. --- .lune/update_tools.luau | 10 +++--- pesde.lock | 18 ++++++++++ pesde.toml | 1 + toolchainlib/src/compression.luau | 10 +++--- toolchainlib/src/github.luau | 10 +++--- toolchainlib/src/init.luau | 14 +++++--- toolchainlib/src/platform/arch.luau | 5 ++- toolchainlib/src/platform/descriptor.luau | 13 +++---- .../src/platform/detection/pattern.luau | 5 ++- toolchainlib/src/platform/result.luau | 4 +-- toolchainlib/src/platform/toolchain.luau | 5 ++- toolchainlib/src/utils/exec.luau | 23 +++++-------- toolchainlib/src/utils/ext/option.luau | 16 +++++++++ toolchainlib/src/utils/ext/result.luau | 14 ++++++++ .../src/utils/result_option_conv.luau | 34 ------------------- 15 files changed, 96 insertions(+), 86 deletions(-) create mode 100644 toolchainlib/src/utils/ext/option.luau create mode 100644 toolchainlib/src/utils/ext/result.luau delete mode 100644 toolchainlib/src/utils/result_option_conv.luau diff --git a/.lune/update_tools.luau b/.lune/update_tools.luau index 7b9a55a..06af079 100644 --- a/.lune/update_tools.luau +++ b/.lune/update_tools.luau @@ -7,11 +7,11 @@ local pathfs = require("../lune_packages/pathfs") local base64 = require("../lune_packages/base64") local manifestTypes = require("../toolchainlib/src/manifest") -local types = require("../toolchainlib/src/utils/result_option_conv") -local Option = types.Option -type Option = types.Option -local Result = types.Result -type Result = types.Result +local Result = require("../lune_packages/result") +local Option = require("../lune_packages/option") +type Result = Result.Result +type Option = Option.Option + local Github = require("../toolchainlib/src/github") type GithubContents = { diff --git a/pesde.lock b/pesde.lock index 878bcbb..bcf4a62 100644 --- a/pesde.lock +++ b/pesde.lock @@ -65,6 +65,24 @@ index_url = "https://github.com/daimond113/pesde-index" environment = "lune" lib = "lib/init.luau" +[graph."lukadev_0/result"."1.2.0 lune"] +direct = ["result", { name = "lukadev_0/result", version = "^1.2.0" }, "dev"] +resolved_ty = "dev" + +[graph."lukadev_0/result"."1.2.0 lune".target] +environment = "lune" +lib = "lib/init.luau" + +[graph."lukadev_0/result"."1.2.0 lune".pkg_ref] +ref_ty = "pesde" +name = "lukadev_0/result" +version = "1.2.0" +index_url = "https://github.com/daimond113/pesde-index" + +[graph."lukadev_0/result"."1.2.0 lune".pkg_ref.target] +environment = "lune" +lib = "lib/init.luau" + [graph."synpixel/base64"."3.0.1 lune"] direct = ["base64", { name = "synpixel/base64", version = "^3.0.1" }, "dev"] resolved_ty = "dev" diff --git a/pesde.toml b/pesde.toml index 7fcc034..9e01d40 100644 --- a/pesde.toml +++ b/pesde.toml @@ -9,6 +9,7 @@ environment = "lune" [dev_dependencies] option = { name = "lukadev_0/option", version = "^1.2.0" } +result = { name = "lukadev_0/result", version = "^1.2.0" } pathfs = { name = "jiwonz/pathfs", version = "^0.1.0" } base64 = { name = "synpixel/base64", version = "^3.0.1" } diff --git a/toolchainlib/src/compression.luau b/toolchainlib/src/compression.luau index f0d9327..6e01ee0 100644 --- a/toolchainlib/src/compression.luau +++ b/toolchainlib/src/compression.luau @@ -4,13 +4,13 @@ local process = require("@lune/process") local dirs = require("../lune_packages/dirs") local pathfs = require("../lune_packages/pathfs") +local Result = require("../lune_packages/result") +local Option = require("../lune_packages/option") +type Result = Result.Result +type Option = Option.Option + local revTable = require("./utils/rev_table") local CommandBuilder = require("./utils/exec") -local types = require("./utils/result_option_conv") -local Option = types.Option -type Option = types.Option -local Result = types.Result -type Result = types.Result export type CompressionFormat = "TarGz" | "TarXz" | "Zip" diff --git a/toolchainlib/src/github.luau b/toolchainlib/src/github.luau index d636d45..a956f2a 100644 --- a/toolchainlib/src/github.luau +++ b/toolchainlib/src/github.luau @@ -1,11 +1,11 @@ local net = require("@lune/net") +local Result = require("../lune_packages/result") +local Option = require("../lune_packages/option") +type Result = Result.Result +type Option = Option.Option + local copy = require("./utils/copy") -local types = require("./utils/result_option_conv") -local Option = types.Option -local Result = types.Result -type Option = types.Option -type Result = types.Result local Github = {} export type Github = typeof(setmetatable(Github :: GithubFields, { __index = Github })) diff --git a/toolchainlib/src/init.luau b/toolchainlib/src/init.luau index 58ec2c0..94c6ec0 100644 --- a/toolchainlib/src/init.luau +++ b/toolchainlib/src/init.luau @@ -15,9 +15,11 @@ local serde = require("@lune/serde") local pathfs = require("../lune_packages/pathfs") local dirs = require("../lune_packages/dirs") -local types = require("./utils/result_option_conv") -local Option = types.Option -type Option = types.Option +local Result = require("../lune_packages/result") +local ResultExt = require("./utils/ext/result") +local Option = require("../lune_packages/option") +type Result = Result.Result +type Option = Option.Option local Github = require("./github") local PlatformDescriptor = require("./platform/descriptor") @@ -54,7 +56,7 @@ local function downloadAndDecompress(asset: { return error(`Failed to download asset {asset.name}: HTTP Code {contentsResp.statusCode}`) end - return compression.decompress[format](buffer.fromstring(contentsResp.body)):ok() :: Option + return ResultExt.ok(compression.decompress[format](buffer.fromstring(contentsResp.body))) end) :: Option end @@ -89,7 +91,9 @@ function runTool(tool: ToolId | pathfs.Path): number -- FIXME: `process.spawn` has a bug where interactive features don't -- forward properly local toolId = tool :: ToolId - local path = if toolId.alias ~= nil then LINK_INSTALL_DIR:join(toolAliasOrDefault(toolId)) else tool :: pathfs.Path + local path = if (toolId :: any).alias ~= nil + then LINK_INSTALL_DIR:join(toolAliasOrDefault(toolId)) + else tool :: pathfs.Path return process.spawn(path:toString(), process.args, { cwd = process.cwd, diff --git a/toolchainlib/src/platform/arch.luau b/toolchainlib/src/platform/arch.luau index 6bb77c9..b84207f 100644 --- a/toolchainlib/src/platform/arch.luau +++ b/toolchainlib/src/platform/arch.luau @@ -4,9 +4,8 @@ local process = require("@lune/process") local detection = require("./detection") -local types = require("../utils/result_option_conv") -local Option = types.Option -type Option = types.Option +local Option = require("../../lune_packages/option") +type Option = Option.Option export type Arch = process.Arch | "arm" | "x86" diff --git a/toolchainlib/src/platform/descriptor.luau b/toolchainlib/src/platform/descriptor.luau index 516ea93..b35f9cb 100644 --- a/toolchainlib/src/platform/descriptor.luau +++ b/toolchainlib/src/platform/descriptor.luau @@ -7,11 +7,11 @@ local toolchain = require("./toolchain") local result = require("./result") local detectFromExecutable = require("./detection/executable") -local types = require("../utils/result_option_conv") -local Option = types.Option -local Result = types.Result -type Option = types.Option -type Result = types.Result +local Result = require("../../lune_packages/result") +local Option = require("../../lune_packages/option") +local OptionExt = require("../utils/ext/option") +type Result = Result.Result +type Option = Option.Option local PlatformDescriptor = {} export type PlatformDescriptor = { @@ -63,7 +63,8 @@ function PlatformDescriptor.fromExecutable(path: string): result.PlatformResult< } end) :: Option - return platformDesc:okOr( + return OptionExt.okOr( + platformDesc, "NoExecutableDetected" :: result.PlatformError ) :: result.PlatformResult end diff --git a/toolchainlib/src/platform/detection/pattern.luau b/toolchainlib/src/platform/detection/pattern.luau index 44f12ac..df1c4df 100644 --- a/toolchainlib/src/platform/detection/pattern.luau +++ b/toolchainlib/src/platform/detection/pattern.luau @@ -1,8 +1,7 @@ local String = require("../../utils/string") -local types = require("../../utils/result_option_conv") -local Option = types.Option -type Option = types.Option +local Option = require("../../../lune_packages/option") +type Option = Option.Option local function charWordSep(char: string) return char == " " or char == "-" or char == "_" diff --git a/toolchainlib/src/platform/result.luau b/toolchainlib/src/platform/result.luau index da14582..5d76bfe 100644 --- a/toolchainlib/src/platform/result.luau +++ b/toolchainlib/src/platform/result.luau @@ -1,5 +1,5 @@ -local types = require("../utils/result_option_conv") -type Result = types.Result +local Result = require("../../lune_packages/result") +type Result = Result.Result export type PlatformError = "NoPatternDetected" | "NoExecutableDetected" | "UnknownExecutableField" export type PlatformResult = Result diff --git a/toolchainlib/src/platform/toolchain.luau b/toolchainlib/src/platform/toolchain.luau index 89171a4..78f8c6b 100644 --- a/toolchainlib/src/platform/toolchain.luau +++ b/toolchainlib/src/platform/toolchain.luau @@ -1,6 +1,5 @@ -local types = require("../utils/result_option_conv") -local Option = types.Option -type Option = types.Option +local Option = require("../../lune_packages/option") +type Option = Option.Option local TOOLCHAINS: { Toolchain } = { "msvc", "gnu", "musl" } export type Toolchain = "msvc" | "gnu" | "musl" diff --git a/toolchainlib/src/utils/exec.luau b/toolchainlib/src/utils/exec.luau index 4926f5a..308ae2d 100644 --- a/toolchainlib/src/utils/exec.luau +++ b/toolchainlib/src/utils/exec.luau @@ -3,9 +3,8 @@ local process = require("@lune/process") local task = require("@lune/task") -local types = require("../utils/result_option_conv") -local Option = types.Option -type Option = types.Option +local Option = require("../../lune_packages/option") +type Option = Option.Option local CommandBuilder = {} type CommandBuilderFields = { @@ -34,10 +33,9 @@ export type ChildStatus = { ok: boolean, code: number, io: { stderr: string, } } --- FIXME: remove unknown usage local DEFAULT_STDIO_STRATEGY: IoStrategyMapping = { - stdout = Option.Some("pipe" :: StdioStrategy) :: Option, - stderr = Option.Some("pipe" :: StdioStrategy) :: Option, + stdout = Option.Some("pipe" :: StdioStrategy), + stderr = Option.Some("pipe" :: StdioStrategy), } local DEFAULT_RETRIES = 0 local DEFAULT_IGNORE_ERRORS = false @@ -84,11 +82,10 @@ function CommandBuilder.withStdioStrategy( self: CommandBuilder, strategy: StdioStrategy | IoStrategyMapping ): CommandBuilder - -- FIXME: remove unknown usage self.stdioStrategy = Option.Some(if typeof(strategy) == "string" then { - stdout = Option.Some(strategy) :: Option, - stderr = Option.Some(strategy) :: Option, + stdout = Option.Some(strategy), + stderr = Option.Some(strategy), } else strategy) :: Option return self @@ -124,12 +121,8 @@ function CommandBuilder.intoChildProcess(self: CommandBuilder): ChildProcess else `{self.program} {argsList} & echo $!`, {}, { - stdio = self - .stdioStrategy - -- FIXME: remove unknown usage - :orOpt( - Option.Some(DEFAULT_STDIO_STRATEGY) :: Option - ) + stdio = self.stdioStrategy + :orOpt(Option.Some(DEFAULT_STDIO_STRATEGY)) :map(function(mappings: IoStrategyMapping) local translatedMappings: process.SpawnOptionsStdio = {} for field, value in mappings do diff --git a/toolchainlib/src/utils/ext/option.luau b/toolchainlib/src/utils/ext/option.luau new file mode 100644 index 0000000..7b10dd0 --- /dev/null +++ b/toolchainlib/src/utils/ext/option.luau @@ -0,0 +1,16 @@ +--> Non-exhaustive set of extensions for the `Option` type + +local Option = require("../../../lune_packages/option") +local Result = require("../../../lune_packages/result") + +local OptionExt = {} + +function OptionExt.okOr(self: Option.Option, err: E): Result.Result + return self:mapOrElse(function() + return Result.Err(err) + end, function(val) + return Result.Ok(val) + end) :: Result.Result +end + +return OptionExt diff --git a/toolchainlib/src/utils/ext/result.luau b/toolchainlib/src/utils/ext/result.luau new file mode 100644 index 0000000..001fc9f --- /dev/null +++ b/toolchainlib/src/utils/ext/result.luau @@ -0,0 +1,14 @@ +--> Non-exhaustive set of extensions for the `Result` type + +local Option = require("../../../lune_packages/option") +local Result = require("../../../lune_packages/result") + +local ResultExt = {} + +function ResultExt.ok(self: Result.Result): Option.Option + return self:mapOr(Option.None, function(val: T) + return Option.Some(val) + end) :: Option.Option +end + +return ResultExt diff --git a/toolchainlib/src/utils/result_option_conv.luau b/toolchainlib/src/utils/result_option_conv.luau deleted file mode 100644 index 915d8d4..0000000 --- a/toolchainlib/src/utils/result_option_conv.luau +++ /dev/null @@ -1,34 +0,0 @@ ---> Non-exhaustive set of extensions for Option<->Result conversion - -local OptionImpl = require("../../lune_packages/option") -local ResultImpl = require("../../lune_packages/result") - -local Option = {} -local Result = {} - -export type Option = OptionImpl.Option & typeof(Option) -export type Result = ResultImpl.Result & typeof(Result) - -function Option.okOr(self: Option, err: E): Result - return self:mapOrElse(function() - return ResultImpl.Err(err) - end, function(val) - return ResultImpl.Ok(val) - end) :: Result -end - -function Result.ok(self: Result): Option - return self:mapOr(OptionImpl.None, function(val: T) - return OptionImpl.Some(val) - end) :: Option -end - -return { - Option = setmetatable(OptionImpl, { - __index = Option, - }), - - Result = setmetatable(ResultImpl, { - __index = Result, - }), -}