Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dsch] making isOk and isErr as discriminators #1

Merged
merged 3 commits into from
Sep 19, 2024
Merged

Conversation

DScheglov
Copy link
Owner

@DScheglov DScheglov commented Sep 17, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced documentation for Result<T, E>, Ok<T>, and Err<E> types, improving clarity and usability.
    • Introduced ResultInterface<T, E> for common methods related to result handling.
    • Added getBookWithAuthors2 function for improved book and author fetching with better error handling.
    • Introduced isPromise utility function to check if a value is a Promise.
  • Bug Fixes

    • Simplified error checking by changing method calls to property accesses for isOk and isErr.
  • Chores

    • Updated TypeScript dependency to version ^5.6.2.
    • Removed unused imports and configurations related to asyncDo and do-gen.
  • Tests

    • Added comprehensive tests for new functionalities and refined existing tests for improved type safety and clarity.
    • Introduced tests for the isPromise function to validate its behavior across various scenarios.

Copy link

coderabbitai bot commented Sep 17, 2024

Walkthrough

The changes encompass significant modifications to the documentation, interfaces, and implementation of the Result<T, E> type and its associated functionalities. Key alterations include the introduction of new interfaces, the removal of certain methods, updates to existing functions for clarity and type safety, and a restructuring of the project’s exports. Additionally, several test files have been added or modified to enhance coverage and ensure the correctness of the new implementations.

Changes

File(s) Change Summary
README.md Formatting adjustments, added interfaces ResultInterface<T, E>, Ok<T>, and Err<E>, removed unwrapGen() from Result<T, E>.
bundle-types.sh Removed commands for bundling do-gen.d.ts and async-do-gen.d.ts.
examples/book-and-authors.ts Introduced getBookWithAuthors2 function, modified getBookWithAuthors to use Do, updated type annotations for fetchBook.
examples/equation-pipe.ts Changed result evaluation from result.isOk() to result.isOk.
examples/equations.ts Similar change as above regarding result evaluation.
examples/imports.ts Removed imports of asyncDo from various modules.
examples/methods.ts Changed access of isOk and isErr from method calls to property access.
examples/parser.ts Modified error-checking logic to use property access for isErr.
package.json Upgraded TypeScript version and removed entries for sync/do and async/do from exports.
src/Err.ts Changed ErrImpl to implement Err<E>, modified isOk and isErr to getter properties, removed unwrapGen.
src/Ok.ts Changed OkImpl to implement Ok<T>, modified isOk and isErr to getter properties, removed unwrapGen.
src/async-do-gen.test.ts Deleted file containing tests for asynchronous operations.
src/async-do-gen.ts Deleted file handling asynchronous operations with generator functions.
src/async-methods.ts Simplified result checks and unwrapping by accessing properties directly.
src/async.test.ts Renamed async.asyncDo to async.Do.
src/async.ts Removed export for async-do-gen, added export for do.
src/do-gen.test.ts Deleted file containing tests for functional programming utilities.
src/do-gen.ts Deleted function Do for executing generator jobs.
src/do.test.ts Added comprehensive tests for Do and asyncDo functionalities.
src/do.ts Enhanced Do function to handle both synchronous and asynchronous jobs.
src/fn/is-promise.test.ts Added tests for isPromise function.
src/fn/is-promise.ts Introduced isPromise function to check if a value is a Promise.
src/guards.ts Simplified isOk and isErr functions to use property access.
src/index.test.ts Removed index.asyncDo from exports in tests.
src/index.ts Removed exports for do-gen and async-do-gen, added export for do.
src/lists.ts Changed error handling checks from method calls to property accesses.
src/objects.test.ts Adjusted test assertions to use property access for result checks.
src/objects.ts Similar change as above regarding error handling checks.
src/result.test.ts Enhanced type assertions and added new test cases for equality checks of Ok and Err results.
src/sync-methods.ts Removed unwrapGen, modified type parameters for biMap and biChain.
src/types.test.ts Introduced tests for the Types module.
src/types.ts Transitioned from type aliases to interfaces for Ok and Err, restructured Result types, updated type extraction utilities.
webpack.config.js Removed production configurations for sync/do and async/do.

Poem

In the garden of code, where rabbits play,
New types hop about, brightening the day.
With Result and Ok, they dance in delight,
Errors now clearer, all functions feel right.
So let’s celebrate changes, both big and small,
For in this code burrow, we cherish them all! 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (1)
README.md (1)

162-172: Enhance the clarity of the documentation.

The changes provide a clear explanation of the Result<T, E> type and its relationship with Ok<T> and Err<E> types, enhancing the clarity of the documentation.

Please fix the minor grammatical issue in the following sentence:

-`Result<T, E>` is a generic type that represents either success or failure, and is an union of `Ok<T>` and `Err<E>` types:
+`Result<T, E>` is a generic type that represents either success or failure, and is a union of `Ok<T>` and `Err<E>` types:
Tools
LanguageTool

[misspelling] ~163-~163: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...sents either success or failure, and is an union of Ok<T> and Err<E> types: ...

(EN_A_VS_AN)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d3a05a9 and 1dfa6b3.

Files selected for processing (34)
  • README.md (3 hunks)
  • bundle-types.sh (0 hunks)
  • examples/book-and-authors.ts (1 hunks)
  • examples/equation-pipe.ts (1 hunks)
  • examples/equations.ts (1 hunks)
  • examples/imports.ts (0 hunks)
  • examples/methods.ts (1 hunks)
  • examples/parser.ts (1 hunks)
  • package.json (1 hunks)
  • src/Err.ts (2 hunks)
  • src/Ok.ts (2 hunks)
  • src/async-do-gen.test.ts (0 hunks)
  • src/async-do-gen.ts (0 hunks)
  • src/async-methods.ts (2 hunks)
  • src/async.test.ts (1 hunks)
  • src/async.ts (1 hunks)
  • src/do-gen.test.ts (0 hunks)
  • src/do-gen.ts (0 hunks)
  • src/do.test.ts (1 hunks)
  • src/do.ts (1 hunks)
  • src/fn/is-promise.test.ts (1 hunks)
  • src/fn/is-promise.ts (1 hunks)
  • src/guards.ts (1 hunks)
  • src/index.test.ts (0 hunks)
  • src/index.ts (1 hunks)
  • src/lists.ts (5 hunks)
  • src/objects.test.ts (6 hunks)
  • src/objects.ts (2 hunks)
  • src/result.test.ts (8 hunks)
  • src/sync-methods.ts (1 hunks)
  • src/sync.ts (1 hunks)
  • src/types.test.ts (1 hunks)
  • src/types.ts (2 hunks)
  • webpack.config.js (0 hunks)
Files not reviewed due to no reviewable changes (8)
  • bundle-types.sh
  • examples/imports.ts
  • src/async-do-gen.test.ts
  • src/async-do-gen.ts
  • src/do-gen.test.ts
  • src/do-gen.ts
  • src/index.test.ts
  • webpack.config.js
Files skipped from review due to trivial changes (4)
  • examples/parser.ts
  • src/async.test.ts
  • src/objects.test.ts
  • src/types.test.ts
Additional context used
LanguageTool
README.md

[misspelling] ~163-~163: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...sents either success or failure, and is an union of Ok<T> and Err<E> types: ...

(EN_A_VS_AN)

Additional comments not posted (81)
src/async.ts (1)

3-3: Verify the impact of the export change on the codebase.

The export statement for async-do-gen has been removed and a new export statement for do has been added. This change suggests a shift in the module's public API and may impact how consumers of this module interact with its asynchronous capabilities.

Run the following script to verify the usage of async-do-gen and do modules in the codebase:

Verification successful

Export change aligns with codebase usage patterns

The removal of the 'async-do-gen' export and addition of the 'do' export in src/async.ts is consistent with the usage patterns across the codebase. Key findings:

  • No references to 'async-do-gen' were found, indicating its removal won't impact the codebase.
  • The 'do' module is widely used and exported from multiple files, including src/sync.ts, src/index.ts, and now src/async.ts.
  • The change appears to be part of a refactoring effort to consolidate functionality into the 'do' module.

This modification aligns with the existing structure and doesn't introduce conflicts or breaking changes in the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `async-do-gen` and `do` modules in the codebase.

# Test 1: Search for the usage of `async-do-gen`. Expect: No occurrences.
rg --type typescript -A 5 $'async-do-gen'

# Test 2: Search for the usage of `do`. Expect: Occurrences of the new module usage.
rg --type typescript -A 5 $'do'

Length of output: 141


Script:

#!/bin/bash
# Description: Verify the usage of `async-do-gen` and `do` modules in the codebase.

# Test 1: Search for the usage and imports of `async-do-gen`. Expect: No occurrences.
echo "Searching for 'async-do-gen' usage:"
fd -e ts -e tsx | xargs rg 'async-do-gen'
echo "Searching for 'async-do-gen' imports:"
fd -e ts -e tsx | xargs rg "from ['\"].*async-do-gen['\"]"

# Test 2: Search for the usage and imports of `do`. Expect: Occurrences of the new module usage.
echo "Searching for 'do' usage:"
fd -e ts -e tsx | xargs rg '\bdo\b' -A 5
echo "Searching for 'do' imports:"
fd -e ts -e tsx | xargs rg "from ['\"].*do['\"]"

Length of output: 2567

src/sync.ts (1)

3-3: Verify the impact on dependent modules.

The change looks good. However, please ensure that all dependent modules that relied on the do-gen exports have been updated to use the do exports instead.

Run the following script to verify the usage of do-gen exports:

Verification successful

Verification successful: No usage of 'do-gen' found in the codebase.

The change from exporting 'do-gen' to exporting 'do' appears to have been implemented correctly. Our thorough search of the TypeScript files in the project revealed no remaining references to 'do-gen', either in import statements or elsewhere in the code.

The presence of 'src/do.ts' in the file list aligns with the new export statement, confirming that the transition from 'do-gen' to 'do' has been carried out consistently.

Based on this evidence, it seems that all necessary updates have been made, and there should be no issues with dependent modules related to this change.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `do-gen` exports are not used in the codebase.

# Test: Search for the usage of `do-gen` exports. Expect: No occurrences.
rg --type typescript $'from \'\.\/do-gen\''

Length of output: 140


Script:

#!/bin/bash
# Description: Verify that `do-gen` exports are not used in the codebase.

# Test: Search for the usage of `do-gen` imports (both relative and non-relative). Expect: No occurrences.
rg --type typescript "from ['\"]\.?/do-gen['\"]"

# Additionally, search for any mentions of 'do-gen' in TypeScript files to catch any potential usages
rg --type typescript "do-gen"

Length of output: 154


Script:

#!/bin/bash
# Description: Verify that `do-gen` exports are not used in the codebase.

# Search for 'do-gen' imports in .ts and .tsx files
echo "Searching for 'do-gen' imports:"
rg -g '*.{ts,tsx}' "from ['\"]\.?/do-gen['\"]"

# Search for any mentions of 'do-gen' in .ts and .tsx files
echo "Searching for any mentions of 'do-gen':"
rg -g '*.{ts,tsx}' "do-gen"

# List all .ts and .tsx files for reference
echo "List of all TypeScript files:"
find . -name "*.ts" -o -name "*.tsx"

Length of output: 1491

src/fn/is-promise.ts (1)

1-2: LGTM!

The isPromise function is a useful utility for checking if a value is a Promise. It follows the best practices for type checking and type narrowing in TypeScript.

  • The use of a type predicate value is Promise<unknown> allows for type narrowing when the function returns true.
  • The check for value != null ensures that the function doesn't throw an error when accessing the then property on a null or undefined value.
  • The check for typeof value.then === 'function' is a common way to identify a Promise object, as the then method is a characteristic of Promises.

The function is concise and can help ensure type safety when working with Promises in TypeScript.

src/index.ts (1)

Line range hint 1-6: Verify the impact of the export changes on the codebase.

The changes in the exports look good. The removal of do-gen and async-do-gen exports and the addition of the do export seem to align with the PR objectives.

However, please ensure that:

  1. The removal of do-gen and async-do-gen exports does not break any existing functionality in the codebase.
  2. The addition of the do export is the intended module to be exposed.

Run the following script to verify the impact of the export changes:

Verification successful

Export changes verified: No issues found

The verification process has been completed successfully. Here are the findings:

  1. No imports of the removed modules 'do-gen' and 'async-do-gen' were found in the codebase, confirming that their removal from the exports in src/index.ts does not break any existing functionality.
  2. The newly added 'do' export is only imported in a test file (src/do.test.ts), which is expected and appropriate.

These results indicate that the changes in exports are consistent with the usage in the codebase and do not introduce any breaking changes or unintended consequences.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the impact of the export changes on the codebase.

# Test 1: Search for any imports of the removed modules. Expect: No results.
rg --type typescript $'import.*from.*\'(\.\/)?do-gen\'' 
rg --type typescript $'import.*from.*\'(\.\/)?async-do-gen\''

# Test 2: Search for any imports of the added module. Expect: Only valid imports.
rg --type typescript -A 5 $'import.*from.*\'(\.\/)?do\''

Length of output: 533


Script:

#!/bin/bash
# Description: Verify the impact of the export changes on the codebase (corrected version).

# Test 1: Search for any imports of the removed modules. Expect: No results.
echo "Searching for imports of 'do-gen':"
rg --type typescript "import.*from.*['\"](\\./)?(do-gen)['\"]"

echo "Searching for imports of 'async-do-gen':"
rg --type typescript "import.*from.*['\"](\\./)?(async-do-gen)['\"]"

# Test 2: Search for any imports of the added module. Expect: Only valid imports.
echo "Searching for imports of 'do':"
rg --type typescript -A 5 "import.*from.*['\"](\\./)?(do)['\"]"

Length of output: 574


Script:

#!/bin/bash
# Description: Verify the impact of the export changes on the codebase (using fd and generic search).

# Find all TypeScript files
typescript_files=$(fd -e ts -e tsx)

# Test 1: Search for any imports of the removed modules. Expect: No results.
echo "Searching for imports of 'do-gen':"
echo "$typescript_files" | xargs rg "import.*from.*['\"](\\./)?(do-gen)['\"]"

echo "Searching for imports of 'async-do-gen':"
echo "$typescript_files" | xargs rg "import.*from.*['\"](\\./)?(async-do-gen)['\"]"

# Test 2: Search for any imports of the added module. Expect: Only valid imports.
echo "Searching for imports of 'do':"
echo "$typescript_files" | xargs rg -A 5 "import.*from.*['\"](\\./)?(do)['\"]"

Length of output: 4847

src/guards.ts (2)

13-13: Verify the impact of changing isErr from a method to a property.

Similar to the change in isOk, the modification from value.isErr() to value.isErr suggests that isErr is now a property instead of a method. This could impact the structure of the Result class and the usage of isErr throughout the codebase.

Please ensure that this change is consistent with the intended design and doesn't break existing code that might expect isErr to execute logic.

Run the following script to verify the usage of isErr:

Verification successful

Verification confirms consistent usage of isErr as a property

The change from isErr() method to isErr property has been consistently implemented across the entire codebase. Key findings:

  • No occurrences of isErr() method calls were found.
  • Multiple instances of isErr property access are present in various files.
  • The isErr property is correctly defined in both Ok and Err classes.
  • The isErr function in guards.ts now uses property access (value.isErr).

This change appears to be part of a deliberate refactoring effort and has been applied uniformly throughout the project.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `isErr` in the codebase.

# Test 1: Search for method calls to `isErr()`. Expect: No occurrences.
rg --type typescript $'\.isErr\(\)'

# Test 2: Search for property access to `isErr`. Expect: Only valid occurrences.
rg --type typescript $'\.isErr\b'  

Length of output: 140


Script:

#!/bin/bash
# Description: Verify the usage of `isErr` in the codebase using more generic search patterns.

# Search for TypeScript files
echo "TypeScript files in the repository:"
fd -e ts -e tsx

# Test 1: Search for method calls to `isErr()`
echo -e "\nSearching for method calls to isErr():"
rg '\.isErr\(\)' -g '*.ts' -g '*.tsx'

# Test 2: Search for property access to `isErr`
echo -e "\nSearching for property access to isErr:"
rg '\.isErr\b' -g '*.ts' -g '*.tsx'

# Test 3: Search for any occurrence of `isErr` to catch potential edge cases
echo -e "\nSearching for any occurrence of isErr:"
rg 'isErr' -g '*.ts' -g '*.tsx'

Length of output: 4886


10-10: Verify the impact of changing isOk from a method to a property.

The change from value.isOk() to value.isOk implies that isOk is now a property rather than a method. This could affect how the Result class is structured and how isOk is used throughout the codebase.

Please ensure that this change aligns with the intended design and doesn't break existing code that might expect isOk to execute logic.

Run the following script to verify the usage of isOk:

Verification successful

Change from isOk() method to isOk property is verified and consistent

The modification in src/guards.ts changing isOk from a method call to a property access is verified to be consistent with the rest of the codebase. Key findings:

  • No occurrences of isOk() method calls were found in the codebase.
  • Multiple instances of isOk property access are present across various files.
  • isOk is declared as a readonly property in src/types.ts for both Ok and Err types.

This change aligns with the intended design and doesn't appear to break existing code. The isOk property is being used correctly in conditional statements, assertions, and other contexts throughout the project.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `isOk` in the codebase.

# Test 1: Search for method calls to `isOk()`. Expect: No occurrences.
rg --type typescript $'\.isOk\(\)'

# Test 2: Search for property access to `isOk`. Expect: Only valid occurrences.
rg --type typescript $'\.isOk\b'

Length of output: 138


Script:

#!/bin/bash
# Description: Verify the usage of `isOk` in the codebase using alternative methods.

# Search for TypeScript and JavaScript files
echo "Searching for TypeScript and JavaScript files:"
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \)

echo -e "\nSearching for method calls to isOk():"
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) -print0 | xargs -0 grep -n '\.isOk()'

echo -e "\nSearching for property access to isOk:"
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) -print0 | xargs -0 grep -n '\.isOk\b'

echo -e "\nSearching for declarations of isOk:"
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) -print0 | xargs -0 grep -n 'isOk:'

Length of output: 3271

src/do.ts (4)

1-6: LGTM!

The import statements are correct and necessary for the functionality of this module. There are no unused imports.


8-10: LGTM!

The type definitions for Job and AsyncJob are clear, accurate, and well-suited for the intended functionality. The use of generator functions and the out variance annotation is appropriate.


12-17: LGTM!

The processIteratorResult function correctly handles the different states of the iterator result. The use of ensureResult and err functions ensures type safety and proper error handling. The type annotations provide clear documentation of the expected inputs and outputs.


19-35: LGTM!

The Do function overloads and implementation provide a clear and flexible interface for handling both synchronous and asynchronous jobs. The use of isPromise allows for dynamic handling of iterator results, while the processIteratorResult function ensures proper processing of the results. The return type annotations accurately reflect the possible outcomes of the function.

src/fn/is-promise.test.ts (4)

5-8: LGTM!

The test case correctly checks that isPromise returns true for a new Promise.


10-13: LGTM!

The test case correctly checks that isPromise returns true for a resolved Promise.


15-19: LGTM!

The test case correctly checks that isPromise returns true for a rejected Promise. Good job catching the rejection to avoid unhandled promise rejection warning.


21-30: LGTM!

The test case thoroughly checks that isPromise returns false for various non-promise values. The assertions cover a good range of data types.

examples/equations.ts (1)

41-41: Verify the change from isOk() to isOk.

The code has been updated to use the isOk property instead of the isOk() method for checking the success state of the result object.

Please ensure that this change aligns with the new implementation of the Result<T, E> type and its associated functionalities. If isOk was originally a method that performed additional logic, this change could potentially impact the behavior of the code.

src/objects.ts (2)

18-18: Verify the impact of the change on the function's behavior.

The change from result.isErr() to result.isErr is consistent with the PR objective of making isOk and isErr as discriminators. However, it alters the expected behavior of the error check from a function call to a property evaluation.

Please ensure that this change does not introduce any unintended consequences in the control flow and error handling of the reduceObject function. Consider adding test cases to cover the scenarios affected by this modification.


38-38: Verify the impact of the change on the function's behavior.

The change from result.isOk() to result.isOk is consistent with the PR objective of making isOk and isErr as discriminators. However, it alters the expected behavior of the success check from a function call to a property evaluation.

Please ensure that this change does not introduce any unintended consequences in the control flow and result handling of the reduceObjectErr function. Consider adding test cases to cover the scenarios affected by this modification.

examples/equation-pipe.ts (1)

54-54: Verify the impact of changing isOk() to isOk across the codebase.

The change from result.isOk() to result.isOk aligns with the PR objective of making isOk a discriminator (property) instead of a method. This simplifies the way the success state of the Result is accessed.

However, this is likely a breaking change for consumers of the Result type who were using the isOk() method. Please ensure all usages of isOk() are updated to isOk across the codebase.

Run the following script to verify the usage of isOk:

Verification successful

Verification successful: isOk() method consistently replaced with isOk property

The verification process has confirmed that the change from isOk() method to isOk property has been consistently applied across the codebase. No occurrences of the old isOk() method were found, while multiple instances of the new isOk property usage were detected.

The new isOk property is being used in various files, including:

  • src/sync.test.ts
  • src/objects.ts
  • src/result.test.ts
  • src/objects.test.ts
  • src/lists.ts
  • src/guards.ts
  • src/async-methods.ts
  • examples/methods.ts
  • examples/equations.ts
  • examples/equation-pipe.ts

This change aligns with the PR objective of making isOk a discriminator (property) instead of a method. The consistent implementation across different parts of the codebase suggests that the change has been thoroughly applied.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all usages of `isOk` have been updated from a method to a property.

# Test 1: Search for the old `isOk()` method usage. Expect: No results.
rg --type typescript $'\.isOk\(\)'

# Test 2: Search for the new `isOk` property usage. Expect: Only valid usages of the property, no methods.
rg --type typescript $'\.isOk\b'

Length of output: 138


Script:

#!/bin/bash
# Description: Verify all usages of `isOk` have been updated from a method to a property.

# Find TypeScript files
echo "TypeScript files in the repository:"
fd '\.(ts|tsx)$'

# Test 1: Search for the old `isOk()` method usage. Expect: No results.
echo -e "\nSearching for old isOk() method usage:"
fd '\.(ts|tsx)$' | xargs rg '\.isOk\(\)'

# Test 2: Search for the new `isOk` property usage. Expect: Only valid usages of the property, no methods.
echo -e "\nSearching for new isOk property usage:"
fd '\.(ts|tsx)$' | xargs rg '\.isOk\b'

Length of output: 2468

examples/book-and-authors.ts (4)

1-1: LGTM!

The import statement has been updated correctly to include the necessary functions from @cardellini/ts-result.


15-20: LGTM!

The getBookWithAuthors function has been updated to use the Do notation correctly, and the function logic remains intact.


22-30: Great job on improving type safety and clarity!

The new getBookWithAuthors2 function has an explicit return type that clearly defines the success and error types. This improves the function's contract and makes it easier to understand and use.

The function implementation using the Do notation is consistent with the updated getBookWithAuthors function and looks good.


33-34: LGTM!

The fetchBook function has been updated to explicitly type the ok return values as Book, which enhances type safety and clarity. The changes look good.

src/Ok.ts (5)

7-7: LGTM!

The change in the implemented interface from Result to Ok enhances the clarity and type safety of the code by making the class's purpose more explicit.


10-12: LGTM!

The transformation of isOk from a method to a getter property enhances the readability and usability of the class by allowing the check to be accessed as a property rather than a function call. This change promotes a more idiomatic use of TypeScript's capabilities.


14-16: LGTM!

The transformation of isErr from a method to a getter property enhances the readability and usability of the class by allowing the check to be accessed as a property rather than a function call. This change promotes a more idiomatic use of TypeScript's capabilities.


102-102: LGTM!

The change in the return type of the ok function from Result<T, never> to Ok<T> aligns it with the new implementation of OkImpl. This enhances the clarity and type safety of the code by making the function's purpose more explicit.


1-5: LGTM!

The addition of the AsyncOk and Ok types in the import statements aligns the imports with the usage of these types in the updated code.

src/Err.ts (3)

1-5: LGTM!

The import statement correctly adds the new types AsyncErr and Err to the existing Result import.


107-107: Verify the impact of the return type change on the codebase.

The change in the return type of the err function from Result<never, E> to Err<E> looks good and aligns with the new Err interface implemented by the ErrImpl class.

Run the following script to verify the usage of the err function across the codebase:

Verification successful

The change in the return type of err function is safe and consistent with its usage across the codebase.

The extensive analysis of the err function usage throughout the codebase reveals that the change from Result<never, E> to Err<E> as the return type is compatible with all existing implementations. The err function is used in various contexts, including synchronous and asynchronous operations, and the new return type aligns well with the overall error handling strategy in the project.

No instances were found where the err function is used in a way that would specifically require it to return Result<never, E>. The Err<E> type appears to be a more specific subtype of Result<never, E>, which explains the seamless integration with existing code.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `err` function across the codebase.

# Test: Search for occurrences of `err(`. Expect: Only usage consistent with the `Err<E>` return type.
rg --type typescript $'err\('

Length of output: 65


Script:

#!/bin/bash
# Description: Verify the usage of the `err` function across the codebase.

# Find all TypeScript files and search for 'err(' pattern
fd -e ts -e tsx -x rg -n -C 2 'err\('

Length of output: 30365


7-16: Verify the impact of the interface change on the codebase.

The change in the implemented interface from Result to Err looks good and aligns with the class's intended use as an error state. The isOk and isErr getters also correctly define the state of the instance.

Suggestion: Consider removing the // eslint-disable-line class-methods-use-this comments and updating the linter configuration if this is not needed in getters.

Run the following script to verify the usage of ErrImpl across the codebase:

Verification successful

The interface change from Result to Err is consistent across the codebase.

After thorough verification, I can confirm that the change in the implemented interface from Result to Err for the ErrImpl class does not negatively impact the codebase. The usage of ErrImpl is consistent with its new interface in all occurrences found:

  1. In src/guards.ts, it's correctly imported and used for type checking.
  2. In src/Err.ts, the implementation matches the reviewed code, and it's used appropriately in other methods and exports.

The current implementation appears to be correct and consistent. No further changes are necessary regarding the interface change.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `ErrImpl` across the codebase.

# Test: Search for occurrences of `ErrImpl`. Expect: Only usage consistent with the `Err` interface.
rg --type typescript $'ErrImpl'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify the usage of `ErrImpl` across the codebase.

# Search for ErrImpl occurrences without specifying file type
echo "Searching for ErrImpl occurrences:"
rg 'ErrImpl'

# Find TypeScript files and search for ErrImpl within them
echo -e "\nSearching for ErrImpl in TypeScript files:"
fd -e ts -e tsx | xargs rg 'ErrImpl'

# Search for import statements related to ErrImpl
echo -e "\nSearching for ErrImpl imports:"
rg 'import.*ErrImpl'

Length of output: 1018

src/sync-methods.ts (2)

59-59: LGTM!

The changes to the biMap function signature are consistent with the TypeScript best practices for defining generic type parameters. The reordering and introduction of default types may enhance type inference and usability.


63-63: LGTM!

The changes to the biChain function signature are consistent with the TypeScript best practices for defining generic type parameters. The reordering and introduction of default types may enhance type inference and usability, similar to the changes made to biMap.

src/types.ts (6)

1-5: LGTM!

The change from type alias to interface for Ok<T> provides a more structured definition and enhances type safety. Extending ResultInterface<T, never> ensures that Ok has all the necessary methods. The value property holds the successful result value, and the isOk and isErr discriminator properties make it easier to distinguish between Ok and Err instances.


7-11: LGTM!

The change from type alias to interface for Err<E> provides a more structured definition and enhances type safety. Extending ResultInterface<never, E> ensures that Err has all the necessary methods. The error property holds the error value, and the isOk and isErr discriminator properties make it easier to distinguish between Ok and Err instances.


Line range hint 13-38: LGTM!

The change from type alias to interface for ResultInterface<T, E> allows for a more structured approach to defining the behavior of results. The interface consolidates the methods related to result handling, such as map, chain, unwrap, etc., providing a consistent way to handle results. The methods have appropriate type signatures to ensure type safety. This change enhances the expressiveness and usability of the result types and promotes a more structured and consistent approach to handling results.


41-41: LGTM!

The redefinition of Result<T, E> as a union of Ok<T> and Err<E> reinforces the concept that a result can either be successful (Ok) or erroneous (Err). It leverages the newly defined Ok and Err interfaces, enhancing the clarity and expressiveness of the Result type. This change ensures that a Result can only be either an Ok or an Err, improving type safety.


44-44: LGTM!

The update to ErrTypeOf<T> ensures that it correctly infers the error type from the new Err interface structure. This change maintains the correctness of the ErrTypeOf type extraction utility.


45-45: LGTM!

The update to OkTypeOf<T> ensures that it correctly infers the value type from the new Ok interface structure. This change maintains the correctness of the OkTypeOf type extraction utility.

src/async-methods.ts (3)

20-20: LGTM!

The change simplifies the code by removing the dependency on the Guards module for checking if the result is ok. It maintains the original functionality while improving clarity.


27-27: LGTM!

The change simplifies the code by removing the dependency on the Guards module for checking if the result is an error. It maintains the original functionality while improving clarity.


91-93: LGTM!

The change simplifies the code by removing the dependency on the Guards module for checking if the result is ok or an error. It maintains the original functionality while improving clarity.

package.json (2)

47-47: LGTM!

Updating the TypeScript dependency to ^5.6.2 is a non-breaking change that may introduce new features, improvements, or bug fixes. Ensure that the project still compiles and passes all tests with this updated version.


Line range hint 1-1: Verify the impact of the removed exports.

The removal of the "./sync/do" and "./async/do" entries from the exports section suggests a refactoring or removal of these specific functionalities. This could impact how developers interact with the library's API.

Please ensure that:

  1. The removal is intentional and aligns with the library's goals.
  2. The documentation has been updated to reflect these changes.
  3. Dependent projects have been updated to handle the removed exports.

Run the following script to verify the usage of the removed exports in the codebase:

Verification successful

Removal of exports appears to be safe.

The verification process shows that the removed exports ("./sync/do" and "./async/do") are not referenced anywhere in the codebase. This suggests that their removal from package.json is likely a cleanup of unused or deprecated paths and should not negatively impact the project.

However, please note:

  • Files like src/do.ts, src/async.ts, and src/sync.ts still exist, indicating that similar functionality might be exported through different paths.
  • Ensure that the documentation is updated to reflect any changes in how these functionalities should be imported and used.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the removed exports in the codebase.

# Test: Search for the usage of the removed exports. Expect: No occurrences.
rg --type typescript -e $'sync/do|async/do'

Length of output: 79


Script:

#!/bin/bash
# Description: Verify the usage of the removed exports in the codebase.

# Search for the removed export paths in all files
echo "Searching for removed export paths:"
rg -e '"./sync/do"|"./async/do"' --type-add 'code:*.{js,ts,jsx,tsx}' -t code

# Search for potential usage of 'sync/do' and 'async/do' in all files
echo "Searching for potential usage of removed exports:"
rg -e 'sync/do|async/do' --type-add 'code:*.{js,ts,jsx,tsx}' -t code

# List all JavaScript and TypeScript files in the repository
echo "Listing all JavaScript and TypeScript files:"
fd -e js -e ts -e jsx -e tsx

Length of output: 1525

examples/methods.ts (2)

23-24: Verify if isErr is defined as a property getter and assess the impact of this change.

The change from method call syntax (isErr()) to property access (isErr) alters the semantics and potentially the behavior. Please ensure that isErr is defined as a property getter to avoid unexpected results.

Assess the impact of this change on the public API and existing code that relies on the method call syntax. Consider generating a script to search the codebase for potential breaking changes:

Verification successful

These new scripts should help us gather the necessary information without relying on TypeScript-specific file type recognition. We'll search for the definition of isErr, look for any remaining method calls, and check for property accesses. This will give us a clearer picture of how isErr is defined and used throughout the codebase.


This new script will search for property accesses of isOk and isErr without using the problematic negative lookahead. It will also search for usage of Guards.isOk and Guards.isErr in tests, which we observed in the previous results. This should give us a comprehensive view of how these properties are used throughout the codebase.


Changes to isOk and isErr are correct and consistent throughout the codebase

The transition from method calls to property access for isOk and isErr has been successfully implemented across the entire codebase. Key findings:

  • isErr is properly defined as a getter in both Err and Ok classes (src/Err.ts and src/Ok.ts).
  • No occurrences of isOk() or isErr() method calls were found.
  • Property accesses (e.g., result.isOk, result.isErr) are consistently used in all relevant files.
  • The changes in examples/methods.ts align with the implementation and usage in other parts of the codebase.

These changes maintain the correct behavior while simplifying the API. No breaking changes or inconsistencies were identified.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for potential breaking changes related to `isOk` and `isErr` method calls.

# Test 1: Search for `isOk` method calls. Expect: No occurrences of `isOk()`.
rg --type typescript $'\.isOk\(\)'

# Test 2: Search for `isErr` method calls. Expect: No occurrences of `isErr()`.
rg --type typescript $'\.isErr\(\)'

Length of output: 142


Script:

#!/bin/bash
# Description: Verify isErr definition and search for method calls and property accesses

# Search for the definition of isErr
echo "Searching for isErr definition:"
rg "(?:get )?isErr\s*[\(:]" --type-add 'code:*.{js,ts,jsx,tsx}' -t code

# Search for any remaining isOk() and isErr() method calls
echo -e "\nSearching for isOk() and isErr() method calls:"
rg "\.(isOk|isErr)\(\)" --type-add 'code:*.{js,ts,jsx,tsx}' -t code

# Search for isOk and isErr property accesses
echo -e "\nSearching for isOk and isErr property accesses:"
rg "\.(isOk|isErr)(?!\()" --type-add 'code:*.{js,ts,jsx,tsx}' -t code

Length of output: 1225


Script:

#!/bin/bash
# Description: Search for isOk and isErr property accesses

echo "Searching for isOk and isErr property accesses:"
rg "\.(isOk|isErr)\b" --type-add 'code:*.{js,ts,jsx,tsx}' -t code

echo -e "\nSearching for isOk and isErr usage in tests:"
rg "Guards\.(isOk|isErr)" --type-add 'code:*.{js,ts,jsx,tsx}' -t code

Length of output: 3257


15-16: Verify if isOk is defined as a property getter and assess the impact of this change.

The change from method call syntax (isOk()) to property access (isOk) alters the semantics and potentially the behavior. Please ensure that isOk is defined as a property getter to avoid unexpected results.

Assess the impact of this change on the public API and existing code that relies on the method call syntax. Consider generating a script to search the codebase for potential breaking changes:

Verification successful

Verification complete: Change to property access is consistent and appears correct

The codebase analysis confirms that the change from method calls (isOk(), isErr()) to property access (isOk, isErr) has been consistently applied across the project. No instances of the old method call syntax were found, and the new property access syntax is used correctly in various contexts, including conditionals and type guards.

While we couldn't directly locate the definitions of isOk and isErr, their usage strongly suggests they are indeed properties:

  • Used in conditional statements: if (result.isOk), if (result.isErr)
  • Used in type guards: <T>(value: Result<T, unknown>): value is Ok<T> => value.isOk;

The change appears to be intentional and consistently implemented, reducing the likelihood of breaking changes. However, to ensure complete correctness:

  • Double-check the definitions of isOk and isErr in your Result type or class to confirm they are properly defined as properties or getters.
  • Review the Result type implementation to ensure it aligns with this new property access pattern.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for potential breaking changes related to `isOk` and `isErr` method calls.

# Test 1: Search for `isOk` method calls. Expect: No occurrences of `isOk()`.
rg --type typescript $'\.isOk\(\)'

# Test 2: Search for `isErr` method calls. Expect: No occurrences of `isErr()`.
rg --type typescript $'\.isErr\(\)'

Length of output: 142


Script:

#!/bin/bash
# Description: Search for potential breaking changes related to `isOk` and `isErr`, and their definitions

# Search for old method call syntax
echo "Searching for old method call syntax:"
rg '\.isOk\(\)' --type-add 'ts:*.{ts,tsx}' -t ts
rg '\.isErr\(\)' --type-add 'ts:*.{ts,tsx}' -t ts

# Search for new property access syntax
echo "Searching for new property access syntax:"
rg '\.isOk(?!\()' --type-add 'ts:*.{ts,tsx}' -t ts
rg '\.isErr(?!\()' --type-add 'ts:*.{ts,tsx}' -t ts

# Search for isOk definition as a property or method
echo "Searching for isOk definition:"
ast-grep --lang typescript --pattern 'class $_ {
  $$$
  isOk$_: $_$_ {
    $$$
  }
  $$$
}'

ast-grep --lang typescript --pattern 'class $_ {
  $$$
  isOk$_($_) {
    $$$
  }
  $$$
}'

# Search for isErr definition as a property or method
echo "Searching for isErr definition:"
ast-grep --lang typescript --pattern 'class $_ {
  $$$
  isErr$_: $_$_ {
    $$$
  }
  $$$
}'

ast-grep --lang typescript --pattern 'class $_ {
  $$$
  isErr$_($_) {
    $$$
  }
  $$$
}'

Length of output: 2308


Script:

#!/bin/bash
# Description: Refined search for isOk and isErr usage and definitions

# Search for any remaining old method call syntax
echo "Searching for any remaining old method call syntax:"
rg '\.(isOk|isErr)\(\)' --type-add 'ts:*.{ts,tsx}' -t ts

# Search for new property access syntax
echo "Searching for new property access syntax:"
rg '\.(isOk|isErr)(?!\()' --type-add 'ts:*.{ts,tsx}' -t ts --pcre2

# Search for isOk and isErr definitions
echo "Searching for isOk and isErr definitions:"
ast-grep --lang typescript --pattern 'isOk: $_'
ast-grep --lang typescript --pattern 'isOk($_) { $$ }'
ast-grep --lang typescript --pattern 'isErr: $_'
ast-grep --lang typescript --pattern 'isErr($_) { $$ }'

# Search for class or interface containing isOk and isErr
echo "Searching for class or interface containing isOk and isErr:"
ast-grep --lang typescript --pattern 'class $_ { $$ isOk $$ isErr $$ }'
ast-grep --lang typescript --pattern 'interface $_ { $$ isOk $$ isErr $$ }'

Length of output: 3732

src/lists.ts (5)

29-29: LGTM!

The change from result.isErr() to result.isErr is consistent with the PR objective of making isOk and isErr discriminators. The modification does not introduce any apparent issues.


58-58: LGTM!

The change from result.isOk() to result.isOk is consistent with the PR objective of making isOk and isErr discriminators. The modification does not introduce any apparent issues.


156-156: LGTM!

The change from result.isOk() to result.isOk is consistent with the PR objective of making isOk and isErr discriminators. The modification does not introduce any apparent issues.


183-183: LGTM!

The change from res.isErr() to res.isErr is consistent with the PR objective of making isOk and isErr discriminators. The modification does not introduce any apparent issues.


207-207: LGTM!

The change from res.isErr() to res.isErr is consistent with the PR objective of making isOk and isErr discriminators. The modification does not introduce any apparent issues.

src/do.test.ts (7)

30-41: LGTM!

The qer function correctly implements the quadratic formula and handles the different cases based on the coefficients. The use of the Do construct makes the code readable and easy to follow.


116-129: LGTM!

The okIfPerson function correctly validates an unknown value as a Person object using a series of validation functions. The use of the Do construct makes the validation logic readable and easy to follow. The error paths are correctly mapped to PersonValidationError objects.


199-206: LGTM!

The getBook function correctly retrieves a book by its ID using the Do construct with an async generator function. The use of okIfExists to handle the case when the book is not found makes the code more readable.


217-226: LGTM!

The findAuthorAmong higher-order function correctly creates an index map of authors by their ID for efficient lookup. The returned function uses okIfExists to handle the case when the author is not found, making the code more readable.


228-239: LGTM!

The getBookWithAuthors function correctly retrieves a book with its authors by the book ID using the Do construct with an async generator function. The use of the collect function with findAuthorAmong to retrieve the book's authors makes the code readable and easy to follow.


310-315: LGTM!

The getC higher-order function correctly uses the Do construct with an async generator function to handle different cases based on the input x. The use of yield* to await the result from getB makes the code readable and easy to follow.


43-322: LGTM!

The test cases cover various scenarios for the qer, okIfPerson, getBookWithAuthors, and getC functions. The use of it and it.each functions makes the test cases readable and easy to follow. The expect function is used correctly to assert the expected results. The test case for getC correctly checks the return type of the function using the Expect and Equal types.

README.md (4)

1-4: LGTM!

The reformatting of the badge section improves readability and visual clarity.


115-118: LGTM!

The code segment demonstrates the correct usage of the Result type and its associated functionalities in an async context.


174-244: LGTM!

The changes significantly enhance the documentation by providing detailed information about the Ok<T>, Err<E>, and ResultInterface<T, E> interfaces and their usage.


246-266: LGTM!

The changes provide clear documentation for the ok and err constructor functions, including their function signatures.

src/result.test.ts (20)

27-27: Type assertion is correct.

The type assertion is necessary to set up the test case with a union type.


Line range hint 29-34: Type narrowing is working as expected.

Inside the if statement guarded by isResult, the type of result is correctly narrowed to Result<string, 'foo'>. The type check using Expect and Equal confirms this.


63-63: Type assertion is correct.

The type assertion is necessary to set up the test case with a Result type.


Line range hint 69-76: Type narrowing is working as expected.

Inside the if statement guarded by isOk, the type of result is correctly narrowed to Ok<'foo'>. The type checks using Expect and Equal confirm the narrowed type of result and the type of result.value.


81-81: Type assertion is correct.

The type assertion is necessary to set up the test case with a Result type.


90-96: Type narrowing is working as expected in the else branch.

In the else branch, the type of result is correctly narrowed to Err<string> because it failed the isOk check. The type checks using Expect and Equal confirm the narrowed type of result and the type of result.error.


101-101: Type assertion is correct.

The type assertion is necessary to set up the test case with a Result type.


107-111: Type narrowing is working as expected with the isOk property.

Inside the if statement guarded by the isOk property check, the type of result is correctly narrowed to Ok<'foo'>. The type check using Expect and Equal confirms this.


116-116: Type assertion is correct.

The type assertion is necessary to set up the test case with a Result type.


125-128: Type narrowing is working as expected in the else branch with the isOk property.

In the else branch, the type of result is correctly narrowed to Err<'bar'> because it failed the isOk property check. The type check using Expect and Equal confirms this.


143-143: Type assertion is correct.

The type assertion is necessary to set up the test case with a Result type.


Line range hint 150-153: Type narrowing is working as expected.

Inside the if statement guarded by isErr, the type of result is correctly narrowed to Err<'foo'>. The type check using Expect and Equal confirms this.


159-159: Type assertion is correct.

The type assertion is necessary to set up the test case with a Result type.


169-172: Type narrowing is working as expected in the else branch.

In the else branch, the type of result is correctly narrowed to Ok<string> because it failed the isErr check. The type check using Expect and Equal confirms this.


177-177: Type assertion is correct.

The type assertion is necessary to set up the test case with a Result type.


184-188: Type narrowing is working as expected with the isErr property.

Inside the if statement guarded by the isErr property check, the type of result is correctly narrowed to Err<'foo'>. The type check using Expect and Equal confirms this.


193-193: Type assertion is correct.

The type assertion is necessary to set up the test case with a Result type.


203-206: Type narrowing is working as expected in the else branch with the isErr property.

In the else branch, the type of result is correctly narrowed to Ok<string> because it failed the isErr property check. The type check using Expect and Equal confirms this.


917-934: The test case correctly verifies the composition law for biMap method calls on an Ok result.

The test case compares the result of chained biMap method calls with the result of a single biMap call using composed functions. This is in accordance with the composition law for biMap, which states that biMap(f, h).biMap(g, i) should be equal to biMap(g ∘ f, i ∘ h), where denotes function composition.


959-975: The test case correctly verifies the composition law for biMap method calls on an Err result.

The test case compares the result of chained biMap method calls with the result of a single biMap call using composed functions. This is in accordance with the composition law for biMap, which states that biMap(f, h).biMap(g, i) should be equal to `biMap(g ∘ f, i

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (3)
README.md (3)

163-163: Typographical Error: Use 'a union' instead of 'an union'

In line 163, the phrase "is an union of Ok<T> and Err<E> types:" should use 'a' instead of 'an' because 'union' starts with a consonant sound.

Apply this diff to correct the error:

-is an union of `Ok<T>` and `Err<E>` types:
+is a union of `Ok<T>` and `Err<E>` types:
Tools
LanguageTool

[misspelling] ~163-~163: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...sents either success or failure, and is an union of Ok<T> and Err<E> types: ...

(EN_A_VS_AN)


411-412: Incomplete Sentence: Add a Subject for Clarity

The description of the .value property lacks a subject in the second sentence. For better readability, consider adding a subject to form a complete sentence.

Apply this diff to revise the sentence:

-Returns the value of `Ok<T>`. Could be accessed if and only if the `Result<T, E>` is explicitly narrowed to `Ok<T>`.
+Returns the value of `Ok<T>`. It can be accessed if and only if the `Result<T, E>` is explicitly narrowed to `Ok<T>`.
Tools
LanguageTool

[style] ~411-~411: To form a complete sentence, be sure to include a subject.
Context: ...value: T Returns the value of Ok<T>. Could be accessed if and only if the `Result<...

(MISSING_IT_THERE)


454-455: Incomplete Sentence: Add a Subject for Clarity

In the description of the .error property, the second sentence is missing a subject. Adding a subject improves the grammatical structure and readability.

Apply this diff to revise the sentence:

-Returns the error of `Err<E>`. Could be accessed if and only if the `Result<T, E>` is explicitly narrowed to `Err<E>`.
+Returns the error of `Err<E>`. It can be accessed if and only if the `Result<T, E>` is explicitly narrowed to `Err<E>`.
Tools
LanguageTool

[style] ~454-~454: To form a complete sentence, be sure to include a subject.
Context: ...rror: E Returns the error of Err<E>. Could be accessed if and only if the `Result<...

(MISSING_IT_THERE)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1dfa6b3 and 8e6c078.

Files selected for processing (9)
  • .eslintrc.json (1 hunks)
  • README.md (17 hunks)
  • examples/simple-recover.ts (1 hunks)
  • src/Err.ts (1 hunks)
  • src/Ok.ts (1 hunks)
  • src/lists.ts (5 hunks)
  • src/result.test.ts (8 hunks)
  • src/sync-methods.ts (3 hunks)
  • src/types.ts (2 hunks)
Files skipped from review as they are similar to previous changes (5)
  • src/Err.ts
  • src/Ok.ts
  • src/lists.ts
  • src/result.test.ts
  • src/sync-methods.ts
Additional context used
LanguageTool
README.md

[misspelling] ~163-~163: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...sents either success or failure, and is an union of Ok<T> and Err<E> types: ...

(EN_A_VS_AN)


[style] ~411-~411: To form a complete sentence, be sure to include a subject.
Context: ...value: T Returns the value of Ok<T>. Could be accessed if and only if the `Result<...

(MISSING_IT_THERE)


[style] ~454-~454: To form a complete sentence, be sure to include a subject.
Context: ...rror: E Returns the error of Err<E>. Could be accessed if and only if the `Result<...

(MISSING_IT_THERE)

Additional comments not posted (11)
examples/simple-recover.ts (2)

3-6: LGTM!

The function logic is correct, and the implementation is accurate. It correctly checks if a number is odd and returns the appropriate Result type.


8-11: LGTM!

The function logic is correct, and the implementation is accurate. It correctly uses the okIfOdd function to check if the input is odd and chains an error handling operation to return the next odd number if the input is even. The unwrap method is used appropriately to extract the value from the Result type.

.eslintrc.json (1)

64-66: LGTM!

The addition of "esm/**/*" and "dist/**/*" to the ignorePatterns array is a sensible change. It prevents ESLint from flagging potential issues in the generated ES module outputs and distribution files, which is a common practice.

src/types.ts (5)

1-5: Excellent refactoring to enhance clarity and type safety!

The transition from type alias to interface for Ok<T>, along with the addition of isOk and isErr properties, provides a more structured and expressive approach to defining the behavior of successful results. The use of never type for the error generic parameter reinforces the concept that an Ok result cannot have an error value.

These changes promote better type inference, usability, and safety in handling success cases throughout the codebase.


7-11: Great refactoring to improve clarity and type safety for error results!

Similar to the changes made to Ok<T>, the transition from type alias to interface for Err<E>, along with the addition of isOk and isErr properties, provides a more structured and expressive approach to defining the behavior of error results. The use of never type for the success generic parameter reinforces the concept that an Err result cannot have a success value.

These modifications enhance type inference, usability, and safety in handling error cases throughout the codebase.


Line range hint 13-38: Excellent refactoring to improve structure and functionality of the result type!

Replacing the Result<T, E> type alias with the ResultInterface<T, E> interface is a great decision. It allows for a more structured approach to defining the behavior of results by consolidating the related methods, such as map, chain, and unwrap, in a single interface. This change enhances code organization and readability.

The addition of biMap and biChain methods is a valuable enhancement, as they provide more flexibility in handling both success and error cases simultaneously. These methods enable developers to apply transformations to both the success and error values in a single operation, leading to more concise and expressive code.

Overall, these modifications significantly improve the structure, expressiveness, and functionality of the result type.


41-41: Great update to reinforce the concept of a result being either success or error!

Redefining the Result<T, E> type alias as a union of Ok<T> and Err<E> is a fantastic change. It clearly conveys the idea that a result can only be in one of two states: success or error. By leveraging the newly defined Ok<T> and Err<E> interfaces, this change ensures that a result always adheres to the expected structure and behavior.

This modification enhances the clarity, expressiveness, and type safety of the result type throughout the codebase.


44-45: Excellent update to maintain compatibility and type safety of the extraction utilities!

Updating the ErrTypeOf and OkTypeOf type extraction utilities to reflect the new interface structure is a crucial change. It ensures that these utilities continue to correctly infer the success and error types from the updated Ok and Err interfaces. By maintaining compatibility with the new result type structure, these changes promote type safety and accurate type inference when extracting specific types from a result value.

This modification demonstrates attention to detail and a commitment to maintaining a consistent and reliable type system throughout the codebase.

README.md (3)

179-185: Well-Defined Ok<T> Interface

The definition of the Ok<T> interface is clear and correctly extends ResultInterface<T, never> with appropriate properties. This enhances type safety and clarity when working with successful results.


198-207: Well-Defined Err<E> Interface

The definition of the Err<E> interface is well-structured and correctly extends ResultInterface<never, E> with the appropriate properties. This provides consistency and clarity when handling error cases.


220-247: Introduction of ResultInterface<T, E> Enhances Consistency

The addition of the ResultInterface<T, E> interface standardizes the common methods available for Result instances. This promotes code reuse and maintainability across the codebase.

@DScheglov DScheglov merged commit b371f3e into main Sep 19, 2024
17 checks passed
@DScheglov DScheglov deleted the release/0.4.x branch September 22, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant