fix: streamline type collection by removing 'flags' #2025
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2025 +/- ##
==========================================
- Coverage 80.25% 80.04% -0.22%
==========================================
Files 172 171 -1
Lines 10825 10736 -89
Branches 1040 1030 -10
==========================================
- Hits 8688 8594 -94
- Misses 2132 2137 +5
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
🎉 This is included in version v0.8.2 🎉 The release is available on: Cheers! 📦🚀 |
| // The 'void' type shouldn't be assigned to anything, so we ignore it | ||
| if (assignedType.flags === ts.TypeFlags.Void) { | ||
| remainingMissingTypes.delete(assignedType); | ||
| } |
There was a problem hiding this comment.
Should the next if be converted to an else-if, or does isAssignedTypeMissingFromDeclared need to run for side-effects?
There was a problem hiding this comment.
That might be a good first contribution for you 😄 do you see any buggy behavior from it?
There was a problem hiding this comment.
My primary concern was efficiency (it always starts with a single if-statement in a loop, but then one more if-statement is added, and then one more, and suddenly the short and fast loop is not so short and fast any longer).
But there's also a readability aspect, with an else-if it's clear that either isAssignedTypeMissingFromDeclared doesn't have any important side-effects, or that things are really intricate and any minor change might break things.
## PR Checklist - [ ] Addresses an existing open issue: fixes #000 - [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 Make the case for the void type mutually exclusive with the other check in the loop. Suggested in #2025 (comment).
PR Checklist
status: accepting prsOverview
Previously, types collection included both flags (
ts.TypeFlagsnumbers) and the actual types (ts.Typeobjects). I don't remember why I separated the two. Everything is capturable by types, andchecker.isTypeAssignableToonly applies on types.This removes the flags and goes with just types. Doing so fixes the bug around enums not being captured (because of bypassing assignability checking).