Merge pull request #3506 from github/henrymercer/result-better-inference

Improve type inference of `Result<T, E>`

Author: henrymercer

lib/init-action.js

@@ -96,6 +96,8 @@ import {
   GitHubVersion,
   Result,
   getOptionalEnvVar,
+  Success,
+  Failure,
 } from "./util";
 import { checkWorkflow } from "./workflow";
 
@@ -834,25 +836,25 @@ async function loadRepositoryProperties(
       "Skipping loading repository properties because the repository is owned by a user and " +
         "therefore cannot have repository properties.",
     );
-    return Result.success({});
+    return new Success({});
   }
 
   if (!(await features.getValue(Feature.UseRepositoryProperties))) {
     logger.debug(
       "Skipping loading repository properties because the UseRepositoryProperties feature flag is disabled.",
     );
-    return Result.success({});
+    return new Success({});
   }
 
   try {
-    return Result.success(
+    return new Success(
       await loadPropertiesFromApi(gitHubVersion, logger, repositoryNwo),
     );
   } catch (error) {
     logger.warning(
       `Failed to load repository properties: ${getErrorMessage(error)}`,
     );
-    return Result.failure(error);
+    return new Failure(error);
   }
 }
 

src/init-action.ts

@@ -564,27 +564,27 @@ test("joinAtMost - truncates list if array is > than limit", (t) => {
   t.false(result.includes("test6"));
 });
 
-test("Result.success creates a success result", (t) => {
-  const result = util.Result.success("test value");
+test("Success creates a success result", (t) => {
+  const result = new util.Success("test value");
   t.true(result.isSuccess());
   t.false(result.isFailure());
   t.is(result.value, "test value");
 });
 
-test("Result.failure creates a failure result", (t) => {
+test("Failure creates a failure result", (t) => {
   const error = new Error("test error");
-  const result = util.Result.failure(error);
+  const result = new util.Failure(error);
   t.false(result.isSuccess());
   t.true(result.isFailure());
   t.is(result.value, error);
 });
 
-test("Result.orElse returns the value for a success result", (t) => {
-  const result = util.Result.success("success value");
+test("Success.orElse returns the value for a success result", (t) => {
+  const result = new util.Success("success value");
   t.is(result.orElse("default value"), "success value");
 });
 
-test("Result.orElse returns the default value for a failure result", (t) => {
-  const result = util.Result.failure(new Error("test error"));
+test("Failure.orElse returns the default value for a failure result", (t) => {
+  const result = new util.Failure(new Error("test error"));
   t.is(result.orElse("default value"), "default value");
 });

src/util.test.ts

@@ -1291,42 +1291,51 @@ export function joinAtMost(
   return array.join(separator);
 }
 
-/** A success result. */
-type Success<T> = Result<T, never>;
-/** A failure result. */
-type Failure<E> = Result<never, E>;
+/** An interface representing something that is either a success or a failure. */
+interface ResultLike<T, E> {
+  /** The value of the result, which can be either a success value or a failure value. */
+  value: T | E;
+  /** Whether this result represents a success. */
+  isSuccess(): this is Success<T>;
+  /** Whether this result represents a failure. */
+  isFailure(): this is Failure<E>;
+  /** Get the value if this is a success, or return the default value if this is a failure. */
+  orElse<U>(defaultValue: U): T | U;
+}
 
-/**
- * A simple result type representing either a success or a failure.
- */
-export class Result<T, E> {
-  private constructor(
-    private readonly _ok: boolean,
-    public readonly value: T | E,
-  ) {}
-
-  /** Creates a success result. */
-  static success<T>(value: T): Success<T> {
-    return new Result(true, value) as Success<T>;
+/** A simple result type representing either a success or a failure. */
+export type Result<T, E> = Success<T> | Failure<E>;
+
+/** A result representing a success. */
+export class Success<T> implements ResultLike<T, never> {
+  constructor(public readonly value: T) {}
+
+  isSuccess(): this is Success<T> {
+    return true;
   }
 
-  /** Creates a failure result. */
-  static failure<E>(value: E): Failure<E> {
-    return new Result(false, value) as Failure<E>;
+  isFailure(): this is Failure<never> {
+    return false;
   }
 
-  /** Whether this result represents a success. */
-  isSuccess(): this is Success<T> {
-    return this._ok;
+  orElse<U>(_defaultValue: U): T {
+    return this.value;
+  }
+}
+
+/** A result representing a failure. */
+export class Failure<E> implements ResultLike<never, E> {
+  constructor(public readonly value: E) {}
+
+  isSuccess(): this is Success<never> {
+    return false;
   }
 
-  /** Whether this result represents a failure. */
   isFailure(): this is Failure<E> {
-    return !this._ok;
+    return true;
   }
 
-  /** Get the value if this is a success, or return the default value if this is a failure. */
-  orElse<U>(defaultValue: U): T | U {
-    return this.isSuccess() ? this.value : defaultValue;
+  orElse<U>(defaultValue: U): U {
+    return defaultValue;
   }
 }