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

Return type duplicate infinite amount: | boolean #1447

Closed
rubiesonthesky opened this issue Mar 24, 2024 · 3 comments · Fixed by #1481
Closed

Return type duplicate infinite amount: | boolean #1447

rubiesonthesky opened this issue Mar 24, 2024 · 3 comments · Fixed by #1481
Labels
area: fixers Around how TypeStat fixes code. status: accepting prs Please, send a pull request to resolve this! 🙏 type: bug Something isn't working :( 🐛

Comments

@rubiesonthesky
Copy link
Collaborator

🐛 Bug Report

  • TypeStat version: 0.7.3
  • TypeScript version: 4.9.5
  • Node version: 20.11

Actual Behavior

declare function navigateByUrl(url: string): Promise<boolean>;

export class exampleDirective {
  async navigateTo(id: number): Promise<boolean> | boolean | boolean | boolean | boolean | boolean {
    return await navigateByUrl(`/page/${id}`);
  }
}

Expected Behavior

There is no need to change this the return type. This is async function and Promise<boolean> should cover that. But also, it should not add boolean infinite amount to return types union.

declare function navigateByUrl(url: string): Promise<boolean>;

export class exampleDirective {
  async navigateTo(id: number): Promise<boolean> {
    return await navigateByUrl(`/page/${id}`);
  }
}

Reproduction

Original code

declare function navigateByUrl(url: string): Promise<boolean>;

export class exampleDirective {
  async navigateTo(id: number): Promise<boolean> {
    return await navigateByUrl(`/page/${id}`);
  }
}

I cloned this repo and added this test case to test/cases/fixes/incompleteTypes/returnTypes/original.ts file, and I can see same behavior there.

    function navigateByUrl(url: string): Promise<boolean>;

    async function navigateTo(id: number): Promise<boolean> {
        return await navigateByUrl(`/page/${id}`);
    }

After using command yarn run test:mutation --accept, I see this in excepted.ts and actual.ts:

    function navigateByUrl(url: string): Promise<boolean>;

    async function navigateTo(id: number): Promise<boolean> | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean | boolean {
        return await navigateByUrl(`/page/${id}`);
    }
@JoshuaKGoldberg JoshuaKGoldberg changed the title Return type duplicate infinite amount Return type duplicate infinite amount: | boolean Mar 24, 2024
@JoshuaKGoldberg JoshuaKGoldberg added type: bug Something isn't working :( 🐛 status: accepting prs Please, send a pull request to resolve this! 🙏 area: fixers Around how TypeStat fixes code. labels Mar 24, 2024
@JoshuaKGoldberg
Copy link
Owner

@all-contributors please add @rubiesonthesky for bug.

🤖 Beep boop! This comment was added automatically by all-contributors-auto-action.
Not all contributions can be detected from Git & GitHub alone. Please comment any missing contribution types this bot missed.
...and of course, thank you for contributing! 💙

Copy link
Contributor

@JoshuaKGoldberg

I've put up a pull request to add @rubiesonthesky! 🎉

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

JoshuaKGoldberg pushed a commit that referenced this issue Mar 25, 2024
Adds @rubiesonthesky as a contributor for bug.

This was requested by JoshuaKGoldberg [in this
comment](#1447 (comment))

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
@rubiesonthesky
Copy link
Collaborator Author

rubiesonthesky commented Mar 29, 2024

This problem comes from react type fixes!

I did two changes that fix this problem.

In fillOutRawOptions, I set hints.react.propTypes to ReactPropTypesHint.Ignore if rawOptions.hints?.react?.propTypes is not set.

Then, in fixIncompleteReturnTypes, I set the function to return empty array, if request.options.hints.react.propTypes is ReactPropTypesHint.Ignore. This will skip all react related checkings.

I think it would be better default option to not assume that project uses React, if that is not otherwise gathered. For example, project should have React installed or jsx: react set in tsconfig.

edit: Now I lost it... it was working for a second. 🤯


edit: Yeah, no. That did not help really. But here are some test cases

	function navigateByUrl(url: string): Promise<boolean>;
        // this mutates to infinite "| boolean"
	async function navigateTo(): Promise<boolean> {
		return await navigateByUrl("");
	}
        // this mutates to infinite "| boolean"
	async function navigateTo2(): Promise<boolean> {
		const navigated = await navigateByUrl("");
		return navigated;
	}
        // this stays same
	async function returnSame(): Promise<boolean> {
		return navigateByUrl("");
	}
        // this stays same
	async function returnPromise(): Promise<string> {
		return Promise.resolve("");
	}

JoshuaKGoldberg pushed a commit that referenced this issue Mar 30, 2024
…1481)

<!-- 👋 Hi, thanks for sending a PR to TypeStat! 💖.
Please fill out all fields below and make sure each item is true and [x]
checked.
Otherwise we may not be able to review your PR. -->

## PR Checklist

- [x] Addresses an existing open issue: fixes #1447 - I think it also
fixes #1284
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/TypeStat/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/TypeStat/blob/main/.github/CONTRIBUTING.md)
were taken 😹

## Overview

<!-- Description of what is changed and how the code change does that.
-->

It was this small change in the end??

Note, `async function navigateTo(): Promise<boolean> | boolean {` and
`async function navigateTo2(): Promise<boolean> | boolean {` are wrong.
Both should return only `Promise<boolean>`. I can remove those from the
test but I wanted to keep them to see, the difference.

Co-authored-by: rubiesonthesky <rubiesonthesky>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: fixers Around how TypeStat fixes code. status: accepting prs Please, send a pull request to resolve this! 🙏 type: bug Something isn't working :( 🐛
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants