Skip to content

feat(40197): No "did you mean to call" error when invocation is part of a logical expression - #40260

Merged
Nathan Shively-Sanders (sandersn) merged 1 commit into
microsoft:masterfrom
a-tarasyuk:feat/40197
Oct 7, 2020
Merged

feat(40197): No "did you mean to call" error when invocation is part of a logical expression#40260
Nathan Shively-Sanders (sandersn) merged 1 commit into
microsoft:masterfrom
a-tarasyuk:feat/40197

Conversation

@a-tarasyuk

Copy link
Copy Markdown
Contributor

Fixes#40197

@typescript-botTypeScript Bot (typescript-bot) added the For Backlog Bug PRs that fix a backlog bug label Aug 26, 2020
@a-tarasyuk
Oleksandr Tarasiuk (a-tarasyuk) marked this pull request as draft August 26, 2020 10:20
@a-tarasyuk
Oleksandr Tarasiuk (a-tarasyuk) marked this pull request as ready for review August 26, 2020 10:44
Comment threadsrc/compiler/binder.ts
Comment threadsrc/compiler/binder.ts
@RyanCavanaugh

Copy link
Copy Markdown
Member

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 26, 2020

Copy link
Copy Markdown
Contributor

Heya Ryan Cavanaugh (@RyanCavanaugh), I've started to run the extended test suite on this PR at ce4dcf3. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 26, 2020

Copy link
Copy Markdown
Contributor

Heya Ryan Cavanaugh (@RyanCavanaugh), I've started to run the parallelized community code test suite on this PR at ce4dcf3. You can monitor the build here.

@typescript-bot

Copy link
Copy Markdown
Contributor

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RWC analysis

typeofwindow!=='undefined'&&window.console&&(window.console.firebug||window.console.exception&&window.console.table)||// Is firefox >= v31?

This is OK

varmyUid=process.getuid&&process.getuid()

This should be fixed; if the right operand refers to the function anywhere, it shouldn't be an error (same as for if)

Comment threadsrc/compiler/checker.ts
@DanielRosenwasser

Copy link
Copy Markdown
Member

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the extended test suite on this PR at b08d886. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized community code test suite on this PR at b08d886. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the tarball bundle task on this PR at b08d886. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the perf test suite on this PR at b08d886. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized Definitely Typed test suite on this PR at b08d886. You can monitor the build here.

@typescript-bot

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser)
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..40260

Metricmaster40260DeltaBestWorst
Angular - node (v10.16.3, x64)
Memory used344,546k (± 0.02%)344,288k (± 0.03%)-258k (- 0.07%)344,097k344,547k
Parse Time1.99s (± 0.49%)2.01s (± 0.57%)+0.02s (+ 0.90%)1.99s2.03s
Bind Time0.82s (± 0.68%)0.82s (± 0.63%)-0.00s (- 0.36%)0.81s0.83s
Check Time4.78s (± 0.66%)4.78s (± 0.46%)+0.00s (+ 0.04%)4.73s4.84s
Emit Time5.17s (± 0.53%)5.17s (± 0.88%)+0.00s (+ 0.00%)5.08s5.27s
Total Time12.77s (± 0.29%)12.79s (± 0.42%)+0.02s (+ 0.16%)12.66s12.90s
Monaco - node (v10.16.3, x64)
Memory used339,408k (± 0.03%)339,336k (± 0.03%)-72k (- 0.02%)339,094k339,629k
Parse Time1.56s (± 0.49%)1.55s (± 0.48%)-0.00s (- 0.13%)1.53s1.57s
Bind Time0.72s (± 0.51%)0.71s (± 0.66%)-0.01s (- 0.84%)0.70s0.72s
Check Time4.97s (± 0.68%)4.95s (± 0.58%)-0.02s (- 0.44%)4.91s5.05s
Emit Time2.75s (± 0.66%)2.73s (± 0.63%)-0.02s (- 0.87%)2.69s2.78s
Total Time10.00s (± 0.50%)9.95s (± 0.36%)-0.06s (- 0.57%)9.87s10.05s
TFS - node (v10.16.3, x64)
Memory used302,337k (± 0.04%)302,299k (± 0.02%)-38k (- 0.01%)302,197k302,434k
Parse Time1.21s (± 0.72%)1.21s (± 0.64%)+0.00s (+ 0.25%)1.20s1.23s
Bind Time0.67s (± 1.26%)0.67s (± 0.88%)+0.00s (+ 0.45%)0.66s0.69s
Check Time4.47s (± 0.91%)4.46s (± 0.71%)-0.00s (- 0.09%)4.42s4.57s
Emit Time2.92s (± 0.79%)2.89s (± 0.91%)-0.03s (- 1.20%)2.82s2.93s
Total Time9.26s (± 0.71%)9.23s (± 0.46%)-0.03s (- 0.37%)9.15s9.33s
material-ui - node (v10.16.3, x64)
Memory used461,337k (± 0.01%)461,230k (± 0.01%)-107k (- 0.02%)461,089k461,337k
Parse Time1.96s (± 0.51%)1.95s (± 0.57%)-0.01s (- 0.36%)1.93s1.98s
Bind Time0.66s (± 1.14%)0.66s (± 1.16%)-0.01s (- 1.36%)0.63s0.67s
Check Time13.52s (± 1.00%)13.49s (± 0.62%)-0.04s (- 0.28%)13.28s13.72s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time16.15s (± 0.87%)16.10s (± 0.55%)-0.05s (- 0.31%)15.90s16.34s
Angular - node (v12.1.0, x64)
Memory used321,687k (± 0.03%)321,555k (± 0.02%)-132k (- 0.04%)321,463k321,704k
Parse Time1.99s (± 0.78%)1.99s (± 0.65%)-0.01s (- 0.30%)1.96s2.01s
Bind Time0.81s (± 0.74%)0.81s (± 0.59%)+0.00s (+ 0.25%)0.80s0.82s
Check Time4.67s (± 0.46%)4.69s (± 0.41%)+0.01s (+ 0.26%)4.65s4.72s
Emit Time5.36s (± 1.08%)5.39s (± 1.17%)+0.02s (+ 0.43%)5.30s5.60s
Total Time12.83s (± 0.37%)12.87s (± 0.63%)+0.03s (+ 0.24%)12.72s13.12s
Monaco - node (v12.1.0, x64)
Memory used321,619k (± 0.01%)321,641k (± 0.02%)+22k (+ 0.01%)321,377k321,757k
Parse Time1.53s (± 0.80%)1.54s (± 0.75%)+0.01s (+ 0.72%)1.52s1.56s
Bind Time0.69s (± 0.96%)0.69s (± 0.58%)-0.00s (- 0.29%)0.68s0.70s
Check Time4.77s (± 0.31%)4.78s (± 0.53%)+0.01s (+ 0.27%)4.72s4.84s
Emit Time2.82s (± 0.90%)2.81s (± 0.93%)-0.01s (- 0.46%)2.76s2.89s
Total Time9.81s (± 0.31%)9.81s (± 0.51%)+0.01s (+ 0.06%)9.75s9.97s
TFS - node (v12.1.0, x64)
Memory used286,688k (± 0.03%)286,585k (± 0.01%)-103k (- 0.04%)286,503k286,694k
Parse Time1.23s (± 0.76%)1.23s (± 0.63%)-0.00s (- 0.16%)1.21s1.25s
Bind Time0.64s (± 1.09%)0.64s (± 1.57%)+0.00s (+ 0.62%)0.63s0.67s
Check Time4.36s (± 0.59%)4.36s (± 0.60%)+0.00s (+ 0.02%)4.32s4.45s
Emit Time2.92s (± 0.71%)2.92s (± 0.59%)-0.00s (- 0.17%)2.86s2.94s
Total Time9.15s (± 0.48%)9.15s (± 0.37%)-0.00s (- 0.01%)9.10s9.26s
material-ui - node (v12.1.0, x64)
Memory used439,707k (± 0.01%)439,555k (± 0.06%)-152k (- 0.03%)438,489k439,782k
Parse Time1.98s (± 0.53%)1.97s (± 0.56%)-0.01s (- 0.25%)1.94s1.99s
Bind Time0.63s (± 0.91%)0.62s (± 1.09%)-0.01s (- 0.95%)0.61s0.64s
Check Time12.17s (± 1.26%)12.09s (± 0.96%)-0.07s (- 0.59%)11.84s12.40s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time14.77s (± 1.06%)14.69s (± 0.81%)-0.08s (- 0.57%)14.43s14.98s
Angular - node (v8.9.0, x64)
Memory used341,102k (± 0.02%)340,929k (± 0.02%)-173k (- 0.05%)340,804k341,099k
Parse Time2.54s (± 0.58%)2.53s (± 0.44%)-0.01s (- 0.39%)2.51s2.56s
Bind Time0.85s (± 0.58%)0.85s (± 0.78%)+0.00s (+ 0.47%)0.83s0.86s
Check Time5.40s (± 0.48%)5.42s (± 0.47%)+0.02s (+ 0.31%)5.36s5.48s
Emit Time5.92s (± 0.49%)5.88s (± 1.16%)-0.05s (- 0.83%)5.63s5.98s
Total Time14.72s (± 0.30%)14.68s (± 0.59%)-0.04s (- 0.28%)14.39s14.81s
Monaco - node (v8.9.0, x64)
Memory used340,627k (± 0.01%)340,590k (± 0.02%)-37k (- 0.01%)340,416k340,664k
Parse Time1.87s (± 0.41%)1.87s (± 0.36%)+0.00s (+ 0.11%)1.86s1.89s
Bind Time0.89s (± 0.56%)0.89s (± 0.58%)+0.00s (+ 0.34%)0.88s0.90s
Check Time5.48s (± 0.53%)5.50s (± 0.38%)+0.02s (+ 0.36%)5.46s5.54s
Emit Time3.21s (± 0.58%)3.24s (± 1.10%)+0.03s (+ 0.78%)3.19s3.35s
Total Time11.45s (± 0.32%)11.51s (± 0.49%)+0.05s (+ 0.47%)11.43s11.68s
TFS - node (v8.9.0, x64)
Memory used303,971k (± 0.03%)303,938k (± 0.01%)-33k (- 0.01%)303,834k304,063k
Parse Time1.55s (± 0.57%)1.55s (± 0.52%)-0.00s (- 0.13%)1.53s1.57s
Bind Time0.68s (± 0.33%)0.67s (± 0.60%)-0.01s (- 1.18%)0.66s0.68s
Check Time5.22s (± 0.68%)5.19s (± 0.55%)-0.03s (- 0.58%)5.14s5.27s
Emit Time2.92s (± 0.94%)2.93s (± 0.66%)+0.01s (+ 0.38%)2.89s2.98s
Total Time10.37s (± 0.42%)10.34s (± 0.38%)-0.03s (- 0.29%)10.26s10.44s
material-ui - node (v8.9.0, x64)
Memory used465,698k (± 0.01%)465,689k (± 0.01%)-10k (- 0.00%)465,582k465,804k
Parse Time2.38s (± 0.47%)2.38s (± 0.71%)+0.00s (+ 0.13%)2.35s2.42s
Bind Time0.78s (± 1.65%)0.78s (± 1.21%)+0.00s (+ 0.13%)0.76s0.81s
Check Time17.87s (± 1.28%)17.94s (± 1.02%)+0.07s (+ 0.41%)17.48s18.30s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time21.03s (± 1.08%)21.10s (± 0.84%)+0.07s (+ 0.35%)20.63s21.43s
Angular - node (v8.9.0, x86)
Memory used195,669k (± 0.02%)195,619k (± 0.02%)-51k (- 0.03%)195,532k195,683k
Parse Time2.46s (± 0.72%)2.46s (± 0.63%)+0.01s (+ 0.29%)2.43s2.48s
Bind Time0.99s (± 0.60%)0.98s (± 0.90%)-0.01s (- 0.61%)0.95s0.99s
Check Time4.90s (± 0.44%)4.87s (± 0.58%)-0.03s (- 0.57%)4.82s4.95s
Emit Time5.91s (± 0.73%)5.88s (± 1.24%)-0.03s (- 0.42%)5.73s6.04s
Total Time14.24s (± 0.28%)14.19s (± 0.63%)-0.06s (- 0.40%)14.00s14.44s
Monaco - node (v8.9.0, x86)
Memory used193,655k (± 0.01%)193,665k (± 0.02%)+10k (+ 0.01%)193,565k193,777k
Parse Time1.90s (± 0.53%)1.92s (± 1.36%)+0.02s (+ 0.94%)1.87s1.97s
Bind Time0.70s (± 0.82%)0.71s (± 2.00%)+0.01s (+ 1.00%)0.69s0.76s
Check Time5.60s (± 1.21%)5.58s (± 1.06%)-0.02s (- 0.32%)5.37s5.67s
Emit Time2.71s (± 2.76%)2.70s (± 3.13%)-0.01s (- 0.41%)2.64s3.04s
Total Time10.92s (± 0.47%)10.91s (± 0.57%)-0.01s (- 0.05%)10.81s11.05s
TFS - node (v8.9.0, x86)
Memory used173,889k (± 0.04%)173,871k (± 0.02%)-19k (- 0.01%)173,791k173,919k
Parse Time1.59s (± 1.11%)1.59s (± 1.31%)-0.00s (- 0.13%)1.56s1.65s
Bind Time0.65s (± 1.72%)0.65s (± 1.78%)+0.00s (+ 0.15%)0.63s0.68s
Check Time4.73s (± 0.66%)4.75s (± 0.45%)+0.02s (+ 0.36%)4.72s4.80s
Emit Time2.80s (± 1.40%)2.77s (± 0.86%)-0.03s (- 0.89%)2.70s2.81s
Total Time9.76s (± 0.56%)9.75s (± 0.43%)-0.01s (- 0.11%)9.66s9.86s
material-ui - node (v8.9.0, x86)
Memory used263,677k (± 0.01%)263,658k (± 0.01%)-19k (- 0.01%)263,608k263,744k
Parse Time2.44s (± 0.74%)2.42s (± 0.55%)-0.02s (- 0.70%)2.40s2.45s
Bind Time0.68s (± 1.92%)0.67s (± 1.66%)-0.01s (- 1.77%)0.65s0.70s
Check Time16.59s (± 0.56%)16.49s (± 0.62%)-0.11s (- 0.63%)16.23s16.71s
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)0.00s ( NaN%)0.00s0.00s
Total Time19.71s (± 0.50%)19.58s (± 0.52%)-0.13s (- 0.67%)19.31s19.80s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
BenchmarkNameIterations
Current4026010
Baselinemaster10

@sandersnNathan Shively-Sanders (sandersn) added the Breaking Change Would introduce errors in existing code label Sep 8, 2020
@DanielRosenwasser

Copy link
Copy Markdown
Member

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 8, 2020

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the extended test suite on this PR at b08d886. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 8, 2020

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized community code test suite on this PR at b08d886. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Sep 8, 2020

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized Definitely Typed test suite on this PR at b08d886. You can monitor the build here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a test with a nested || to show what happens there?

typeof window !== 'undefined' && window.console && (window.console.firebug || window.console.exception && window.console.table) || // Is firefox >= v31?

Comment threadsrc/compiler/checker.ts Outdated
Comment threadsrc/compiler/emitter.ts
Comment threadsrc/compiler/checker.ts
Comment threadsrc/compiler/checker.ts Outdated
Comment threadsrc/compiler/checker.ts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try calling the uncalled functions in the emitter, since that was probably the original intent?

Comment threadsrc/compiler/emitter.ts
@sandersn
Nathan Shively-Sanders (sandersn) merged commit eaf4f46 into microsoft:masterOct 7, 2020
@sandersn

Copy link
Copy Markdown
Member

Daniel Rosenwasser (@DanielRosenwasser) points out that this is a breaking change, so needs to wait until 4.2. I'll revert it for now and then merge once 4.1 has got its own branch.

@sandersn

Copy link
Copy Markdown
Member

OK, it's reverted. I'll revert the revert once release-4.1 is created and we are merging features into master again.

@a-tarasyuk

Copy link
Copy Markdown
ContributorAuthor

Nathan Shively-Sanders (@sandersn) Do I need to re-open PR?

@orta

Orta Therox (orta) commented Oct 8, 2020

Copy link
Copy Markdown
Contributor

Yes No, I think Nathan is saying he'll handle it actually

@sandersn

Copy link
Copy Markdown
Member

Right, I just made a TODO to revert the revert manually.

@a-tarasyuk

Copy link
Copy Markdown
ContributorAuthor

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Breaking ChangeWould introduce errors in existing codeFor Backlog BugPRs that fix a backlog bug

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

No "did you mean to call" error when invocation is part of a logical expression

6 participants

@a-tarasyuk@RyanCavanaugh@typescript-bot@DanielRosenwasser@sandersn@orta