eslint-plugin-neverthrow icon indicating copy to clipboard operation
eslint-plugin-neverthrow copied to clipboard

Merging issue #9 and #10

Open Bistard opened this issue 1 year ago • 5 comments

I can create a PR to help solve #9 and #10. I need a confirm that this repository is still worth updating. I hope anyone is still maintaining this.

Bistard avatar Dec 28 '23 00:12 Bistard

Hi @Bistard thanks for you interest, i'm not mantaining this repository actively but if you create the PR solving those issue i'll merge it

mdbetancourt avatar Dec 29 '23 12:12 mdbetancourt

Hi @Bistard thanks for you interest, i'm not mantaining this repository actively but if you create the PR solving those issue i'll merge it

I did a lot of changes to the implementation of eslint-plugin-neverthrow. Not just checking new corner cases but also making some different assumptions.

Here are some small changes I did:

  1. I have my own Result type in my project, which has more methods that can be considered as handled: match, unwrap, unwrapOr, unwrapErr,isOk, isErr, expect,then (only avaliable for AsyncResult). This also helps to solve the issue #9.
  2. Adding AwaitExpression parsing check that can easily solve #10
  3. If the unhandled Result is detected, the linter is not reporting to the original expression, it will try to report the first assigned variable.
{
        const firstRef = getResult(); // `firstRef` will be red-underlined, not `getResult()`
        const secondRef = firstRef;
}
  1. This eslint rule also works with AsyncResult in all cases.

Here is one big assumption i made differently from this repo:

  1. Quoting this, I am assuming passing Result to function is considered as taking ownership. Making this assumption brings me to the following a more robust handling check.
  2. Now linter will check if Result which in function parameter is handled, if not, error reported.
// All these parameters will be reported as unhandled.

function resultInParameter(res2: Result<void, void>): void {}
    
async function asyncResultInParameter(res: AsyncResult<void, void>): Promise<void> {}

const anonymousFunction = function (res: Result<void, void>) {};

const anonymousArrowFunction = (res: Result<void, void>) => {};

class SomeClass {
    public classMethod(res: Result<void, void>) {}
}

@supermacro @mdbetancourt It depends on what kind of changes you guys want me to merge, either all changes, or making some of them as optional, or simply not accepting some changes.

I am ok with all cases.

Bistard avatar Jan 06 '24 06:01 Bistard

@Bistard Would you be able to to open a PR for #10? Since it's a simple change, perhaps we can get it merged quickly.

thenbe avatar Jan 20 '24 05:01 thenbe

@Bistard Would you be able to to open a PR for #10? Since it's a simple change, perhaps we can get it merged quickly.

Just did (here)[https://github.com/mdbetancourt/eslint-plugin-neverthrow/pull/17]

Bistard avatar Jan 20 '24 20:01 Bistard

Once the above PR is merged, I will continue to create a new PR that fixes #9. Additionally, I need confirmation from the maintainer (@mdbetancourt, @supermacro) that should I perform the following big change:

Assuming passing Result to function is considered as taking ownership.

If that is true, I will also complete the linting rule to check the function parameter Result handling.

If not, I will try to fix #9 and #18.

Bistard avatar Jan 20 '24 21:01 Bistard