feat(linter): implement unicorn/no-useless-undefined

[cblh]

Resolves #3870

Hey there, thought I'd give this a try as it's tagged "good first issue" :) Let me know if there's anything that needs to change

Another part of the code also needs modification:
https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/rules/no-useless-undefined.js#L95

I tried to modify it. If there are any issues with these modifications, please let me know.

fn is_has_function_return_type(node: &AstNode, ctx: &LintContext<'_>) -> bool {
    let Some(parent_node) = ctx.nodes().parent_node(node.id()) else {
        return false;
    };
    match parent_node.kind() {
        AstKind::ArrowFunctionExpression(arrow_func_express) => {
            arrow_func_express.return_type.is_some()
        }
        AstKind::Function(func) => func.return_type.is_some(),
        _ => is_has_function_return_type(parent_node, ctx),
    }
}

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[codspeed-hq]

CodSpeed Performance Report

Merging #4079 will not alter performance

_{Comparing cblh:feature/20240704no-useless-undefined (cd39baf) with main (ddfa343)}

Summary

✅ 31 untouched benchmarks

[DonIsaac]

I'll run this fixer using oxc-ecosystem-ci, and if it looks good we should be able to merge.

[DonIsaac]

LGTM: I ran the fixer and saw no parse errors on a second pass. I'd like another review from a second pair of eyes if possible, @mysteryven think you could give me a hand?

[mysteryven]

test('extension is not outdated when there is no local and gallery', () => {
    const extension = instantiationService.createInstance(Extension, () => ExtensionState.Installed, () => undefined, undefined, undefined, undefined, undefined);
    assert.strictEqual(extension.outdated, false);
});

https://github.com/microsoft/vscode/blob/c37f4a49e28831cec1aec1af0f626edaada1f7ce/src/vs/workbench/contrib/extensions/test/electron-sandbox/extension.test.ts#L29-L32

Our fixer need to run twice to fix this, tested on unicorn/no-useless-undefined, it needs once.

Other parts are also look good to me.

[cblh]

test('extension is not outdated when there is no local and gallery', () => {
    const extension = instantiationService.createInstance(Extension, () => ExtensionState.Installed, () => undefined, undefined, undefined, undefined, undefined);
    assert.strictEqual(extension.outdated, false);
});

https://github.com/microsoft/vscode/blob/c37f4a49e28831cec1aec1af0f626edaada1f7ce/src/vs/workbench/contrib/extensions/test/electron-sandbox/extension.test.ts#L29-L32

Our fixer need to run twice to fix this, tested on unicorn/no-useless-undefined, it needs once.

Other parts are also look good to me.

ok, I will fix it

[cblh]

@mysteryven @DonIsaac

Why our fixer need to run twice

            if i64::from(start) <= last_pos {
                return;
            }

https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/fixer.rs#L278

I found that span is an interval that is closed on the left and open on the right. It should not be <=, which will cause the continuous interval to be skipped. It should be <.

The test case needs to be modified to allow continuous interval modification, but I found that it will cause no_useless_escape test failure

https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/fixer.rs#L520

    const REMOVE_START: Fix = Fix::delete(Span::new(0, 4));
    const REPLACE_ID: Fix = Fix { span: Span::new(4, 10), content: Cow::Borrowed("foo") };
    fn apply_two_fix_when_the_start_the_same_as_the_previous_end() {
        let result = get_fix_result(vec![
            create_message(remove_start(), Some(REMOVE_START)),
            create_message(replace_id(), Some(REPLACE_ID)),
        ]);
        assert_eq!(result.fixed_code, TEST_CODE.replace("var answer", "foo"));
        assert!(result.fixed);
    }

[mysteryven]

I found that span is an interval that is closed on the left and open on the right. It should not be <=, which will cause the continuous interval to be skipped. It should be <.

Since it not the issue about current rule, let's merge this PR first.