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: noInferableTypes removes incorrectly type from Map variable #1512

Closed
3 tasks done
rubiesonthesky opened this issue Apr 1, 2024 · 1 comment · Fixed by #1520
Closed
3 tasks done

🐛 Bug: noInferableTypes removes incorrectly type from Map variable #1512

rubiesonthesky opened this issue Apr 1, 2024 · 1 comment · Fixed by #1520
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

rubiesonthesky commented Apr 1, 2024

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Expected

This code stays as it is

type MapStringToNumber = Map<string, number>;
const incompleteTypes: MapStringToNumber = new Map();

Actual

It changes to

type MapStringToNumber = Map<string, number>;
const incompleteTypes = new Map();

Additional Info

This was uncovered with changes in #1511

typestat.json

{
	"fixes": {
		"noInferableTypes": true
	},
	"types": {
		"strictNullChecks": true
	}
}

tsconfig

{
	"files": ["actual.ts"]
}
@rubiesonthesky rubiesonthesky added the type: bug Something isn't working :( 🐛 label Apr 1, 2024
@rubiesonthesky rubiesonthesky added status: accepting prs Please, send a pull request to resolve this! 🙏 area: fixers Around how TypeStat fixes code. labels Apr 1, 2024
rubiesonthesky added a commit that referenced this issue Apr 1, 2024
<!-- 👋 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 #1454 
- [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.
-->

- Refactor file reading in testSetup.ts
- Fix reading tsConfig settings

Some files changed to be what they should be, and some are now exposing
bugs (`test/cases/fixes/incompleteTypes/returnTypes/expected.ts`) .

I'm not sure why the type is being removed in
`test/cases/fixes/noInferableTypes/variableDeclarations/expected.ts` but
I think this is also exposing some type. Or it may be that Map would
need some different tsconfig settings. - Filed new issue about it
#1512
@rubiesonthesky
Copy link
Collaborator Author

In declaredTypeIsEquivalent when comparing Map to Map, they are assignable to each other, so we need to add extra check declaredType.getSymbol()?.getEscapedName() !== "Map" to the first if.

However, then we go to typeSymbolsAndArgumentsAreEquivalent where we are now comparing string and any and again they are assignable to each other.

So we need to have some better check somewhere to say that Map and Map<string, number> are not same thing and we should not remove Map<string, number> from assigned types.

JoshuaKGoldberg added a commit that referenced this issue Apr 23, 2024
<!-- 👋 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 #1512
- [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.
-->

I'm not sure that this is correct place to place this code. It's also
maybe too heavy handed. But on the other hand, I think it's better to
prevent cases where we introduce implicit anys when there was some type
earlier. Keeping types that could have been removed, is less bad in that
case.

I have tried to find what would be right place to do this check but I
did not have any luck.

---------

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
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.

1 participant