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

Write more useful types in .types test outputs #48578

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

rbuckton
Copy link
Member

@rbuckton rbuckton commented Apr 5, 2022

#42149 and #42284 improved what we display to users when presenting complex type aliases, but this had the downside of making hundreds of .types test outputs mostly useless, as numerous test outputs would now report T : T for the type of a type alias, when we were actually using those results to validate the expected type.

This changes the type writer we use to generate .types tests to compare the stringified type to the name of the type alias and, if they match, to re-stringify the type without using its name (by passing TypeFormatFlags.InTypeAlias to typeToString).

This change makes over a thousand individual tests across over 427 .tests files useful again.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 5, 2022
@jakebailey
Copy link
Member

jakebailey commented Apr 6, 2022

With this change in place, how are PRs like the above ones tested/verified to still be working in the future? The two linked PRs are all code changes + .types files, where people noted the improvement, but there doesn't seem to be a file that is sorta-kinda "what the user sees". Presumably, if this change had been in place before those two PRs, they wouldn't have really had much in the way of diffs.

@rbuckton
Copy link
Member Author

rbuckton commented Apr 6, 2022

The type we use for what the user sees can be verified when it's used in other places. The user never sees SomeType when hovering over type SomeType = ... in the editor, because we also ignore the type alias when we generate the quick info.

We also could leverage fourslash tests for things like verifying quickinfo as well. Traditionally, the .types tests are for us to verify the correct types are being produced, not for reporting what the user sees in the editor.

@rbuckton rbuckton merged commit b975dfa into main Apr 7, 2022
@rbuckton rbuckton deleted the fixTypeWriterOutputsForTypeAliases branch April 7, 2022 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants