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

Bug: return type duplicate infinite amount: | T #2015

Open
pjonsson opened this issue Nov 29, 2024 · 3 comments
Open

Bug: return type duplicate infinite amount: | T #2015

pjonsson opened this issue Nov 29, 2024 · 3 comments
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

@pjonsson
Copy link
Contributor

🐛 Bug Report

  • TypeStat version: 0.8.1
  • TypeScript version: 5.2
  • Node version: 20

Actual Behavior

Grabbing one from the pile here, this isn't the only one, but the rest look similar:

diff --git a/lib/Models/SelectableDimensions/SelectableDimensions.ts b/lib/Models/SelectableDimensions/SelectableDimensions.ts
index b36326626..1747314a8 100644
--- a/lib/Models/SelectableDimensions/SelectableDimensions.ts
+++ b/lib/Models/SelectableDimensions/SelectableDimensions.ts
@@ -19,7 +19,7 @@ export interface EnumDimensionOption<T = string> {
 }

 export interface EnumDimension<T = string> extends Dimension {
-  readonly options?: readonly EnumDimensionOption<T>[];
+  readonly options?: readonly EnumDimensionOption<T>[] | T | T;
   readonly selectedId?: T;
   readonly allowUndefined?: boolean;
   /** If true, then the user can set the value to arbitrary text */

I'm guessing this is related to #1447, but for a different type.

Expected Behavior

I'm not certain that the type on the interface originally is correct because TypeStat has found some similar type issues in other places (relating to promises), so it's quite possible that the interface should have the type EnumDimensionOption<T>[] | T. I expect TypeStat to stop adding | T after the first one is added though.

Is there a way to find the code point that caused TypeStat to add the type? I'm familiar with how Hindley-Milner type inference works, so I realize it might be hard to give a good code point, but a bad code point that at least was involved in constraining the type would still be better than the current "it's somewhere in the source code" black box.

Reproduction

Same as #2014.

@JoshuaKGoldberg
Copy link
Owner

I'm familiar with how Hindley-Milner type inference works

I'm not 😄. I've tried to keep TypeStat to just use TypeScript compiler APIs and just basic arrays, Maps, and Sets - so that folks don't need to ramp up on deeper type theory to contribute.

If you run with --logfile some-log-file.txt then it'll [log] output the specific fixers that resulted in each mutation. Looking at this file's:

[log] fixReactPropsFromUses found mutations in /Users/josh/repos/terriajs/lib/Models/SelectableDimensions/SelectableDimensions.ts: [
    {
        "mutations": [
            {
                "mutations": [
                    {
                        "insertion": " | T",
                        "range": {
                            "begin": 853
                        },
                        "type": "text-insert"

Digging into the code a bit, fixReactPropsFromUses is calling to the shared createTypeExpansionMutation helper. My hunch is that createTypeExpansionMutation isn't factoring in T generics / deduplicating added type strings as it should. Fun!

@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 Nov 30, 2024
@rubiesonthesky
Copy link
Collaborator

Is there some minimal reproduction of this? Preferably something that would fit into one file easily. I'd like to add this as a mutation test to see if we already fixed this or if not, then it would be easier to investigate where we are going wrong :)

Just by adding this bit does not seem to trigger the problem

	interface Dimension {
		/** Machine readable ID */
		readonly id?: string;
		/** Human readable name */
		readonly name?: string;
	}

	interface EnumDimensionOption<T = string> {
		readonly id?: T;
		readonly name?: string;
	}

	interface EnumDimension<T = string> extends Dimension {
		readonly options?: readonly EnumDimensionOption<T>[];
		readonly selectedId?: T;
		readonly allowUndefined?: boolean;
		/** If true, then the user can set the value to arbitrary text */
		readonly allowCustomInput?: boolean;
		readonly undefinedLabel?: string;
	}

@JoshuaKGoldberg
Copy link
Owner

Is there some minimal reproduction of this?

I tried for a bit and couldn't figure it out quickly. So I gave up. But yes +1 that a PR fixing this should ideally have a test case added!

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

No branches or pull requests

3 participants