Skip to content

Fix lack of error on default export of nonexistant name #40094

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

Merged
merged 4 commits into from
Aug 17, 2020

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Aug 17, 2020

Fixes #39694

This was actually more interesting to fix than you may think, because it didn't repro in the test harness. Turn out that's because the test harness calls emit, and then getPreEmitDiagnostics, which is different from the LS or command line compiler - namely, it captures an error added by the EmitResolver (which shouldn't be adding new errors in the first place)! So once I realized this, I added some new test infrastructure to figure out when this happens and report it (by getting the diagnostics twice - once before emit and once after). That then exposed a crash that wasn't captured by our test harness but would have happened on the command line (accessing a parent pointer in a place where it may not have been bound), which I fixed, and exposed two existing tests where similar EmitResolver-added-error behavior is occurring. In one case, the order we checked controlled which declarations we actually recorded duplicate identifier errors on, because we were only scanning for them on one "side" of the conflicting declarations. In the other, we were explicitly only looking for flag conflicts on the "first" declaration bound to a symbol, but that meant that if checking started in a file other than the one that declaration was in, we'd fail to capture any errors for the conflict.

@weswigham weswigham requested review from sandersn and rbuckton August 17, 2020 19:46
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 17, 2020
@weswigham weswigham requested a review from elibarzilay August 17, 2020 20:39
@weswigham
Copy link
Member Author

@rbuckton do you have any commentary on the test harness change before I merge? You can check the older commits on this PR to see what the output looks like.

@weswigham weswigham merged commit f9cca25 into microsoft:master Aug 17, 2020
@weswigham weswigham deleted the export-default-missing-name branch August 17, 2020 22:30
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.

No compilation error for export default whatever
4 participants