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

Add assert comments in CodeFixes and Refactors #33016

Merged
merged 3 commits into from
Aug 21, 2019

Conversation

RyanCavanaugh
Copy link
Member

Our telemetry-sourced bugs have a ton of call stacks like this, where it isn't obvious which refactor triggered it (tons of them have a doChange function) and it isn't obvious which assert failed without checking out very specific versions of tsserver.js and digging into the line numbers:

Error: Debug Failure. False expression.
    at doChange (tsserver.js:116689:22)
    at unknown (tsserver.js:116680:96)
    at Function.ChangeTracker.with (tsserver.js:111861:17)
    at Object.getCodeActions (tsserver.js:116680:64)
    at unknown (tsserver.js:112847:121)
    at Object.flatMap (tsserver.js:573:25)
    at Object.getFixes (tsserver.js:112847:23)
    at unknown (tsserver.js:122423:35)
    at Object.flatMap (tsserver.js:573:25)
    at Object.getCodeFixesAtPosition (tsserver.js:122421:23)
    at suppressed_frame()
    at suppressed_frame()
    at IOSession.Session.getCodeFixes (tsserver.js:131477:64)

This PR adds messages to all the assert* family calls in these two folders. As a follow-up we could also do the same for cast as that has a few ambiguous stacks as well.

In most cases I tried to make the assert messages globally unique rather than making them consistent, since this will make looking them up later easier.

@RyanCavanaugh RyanCavanaugh merged commit 016884d into microsoft:master Aug 21, 2019
@RyanCavanaugh RyanCavanaugh deleted the addAssertComments branch August 21, 2019 21:22
timsuchanek pushed a commit to timsuchanek/TypeScript that referenced this pull request Sep 11, 2019
* Add comments to assert calls

* Add comments to assert calls in codefixes

* So linty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants