Skip to content

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

Merged
sandersn merged 1 commit intomicrosoft: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
sandersn merged 1 commit intomicrosoft:masterfrom
a-tarasyuk:feat/40197

Conversation

@a-tarasyuk
Copy link
Contributor

@a-tarasyuk a-tarasyuk commented Aug 26, 2020

Fixes #40197

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Aug 26, 2020
@a-tarasyuk a-tarasyuk marked this pull request as draft August 26, 2020 10:20
@a-tarasyuk a-tarasyuk marked this pull request as ready for review August 26, 2020 10:44
@a-tarasyuk a-tarasyuk force-pushed the feat/40197 branch 2 times, most recently from 1899aa1 to ce4dcf3 Compare August 26, 2020 11:01
RyanCavanaugh
RyanCavanaugh reviewed Aug 26, 2020
View reviewed changes
RyanCavanaugh
RyanCavanaugh reviewed Aug 26, 2020
View reviewed changes
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Aug 26, 2020

@typescript-bot test this
@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 26, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 26, 2020 •
edited
Loading

Heya @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 typescript-bot mentioned this pull request Aug 26, 2020
Open
@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 26, 2020

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

RyanCavanaugh
RyanCavanaugh requested changes Aug 26, 2020
View reviewed changes
Copy link
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

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

RWC analysis

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

This is OK

var myUid = 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)

@a-tarasyuk a-tarasyuk marked this pull request as draft August 26, 2020 16:48
@a-tarasyuk a-tarasyuk marked this pull request as ready for review August 26, 2020 20:04
DanielRosenwasser
DanielRosenwasser reviewed Aug 27, 2020
View reviewed changes
@a-tarasyuk a-tarasyuk force-pushed the feat/40197 branch 2 times, most recently from 70e03d8 to b08d886 Compare August 28, 2020 08:38
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Aug 28, 2020

@typescript-bot pack this
@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 28, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 28, 2020 •
edited
Loading

Heya @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
Copy link
Collaborator

typescript-bot commented Aug 28, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 28, 2020 •
edited
Loading

Heya @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
Copy link
Collaborator

typescript-bot commented Aug 28, 2020 •
edited
Loading

Heya @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
Collaborator

typescript-bot commented Aug 28, 2020

@DanielRosenwasser
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..40260

Metric master 40260 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 344,546k (± 0.02%) 344,288k (± 0.03%) -258k (- 0.07%) 344,097k 344,547k
Parse Time 1.99s (± 0.49%) 2.01s (± 0.57%) +0.02s (+ 0.90%) 1.99s 2.03s
Bind Time 0.82s (± 0.68%) 0.82s (± 0.63%) -0.00s (- 0.36%) 0.81s 0.83s
Check Time 4.78s (± 0.66%) 4.78s (± 0.46%) +0.00s (+ 0.04%) 4.73s 4.84s
Emit Time 5.17s (± 0.53%) 5.17s (± 0.88%) +0.00s (+ 0.00%) 5.08s 5.27s
Total Time 12.77s (± 0.29%) 12.79s (± 0.42%) +0.02s (+ 0.16%) 12.66s 12.90s
Monaco - node (v10.16.3, x64)
Memory used 339,408k (± 0.03%) 339,336k (± 0.03%) -72k (- 0.02%) 339,094k 339,629k
Parse Time 1.56s (± 0.49%) 1.55s (± 0.48%) -0.00s (- 0.13%) 1.53s 1.57s
Bind Time 0.72s (± 0.51%) 0.71s (± 0.66%) -0.01s (- 0.84%) 0.70s 0.72s
Check Time 4.97s (± 0.68%) 4.95s (± 0.58%) -0.02s (- 0.44%) 4.91s 5.05s
Emit Time 2.75s (± 0.66%) 2.73s (± 0.63%) -0.02s (- 0.87%) 2.69s 2.78s
Total Time 10.00s (± 0.50%) 9.95s (± 0.36%) -0.06s (- 0.57%) 9.87s 10.05s
TFS - node (v10.16.3, x64)
Memory used 302,337k (± 0.04%) 302,299k (± 0.02%) -38k (- 0.01%) 302,197k 302,434k
Parse Time 1.21s (± 0.72%) 1.21s (± 0.64%) +0.00s (+ 0.25%) 1.20s 1.23s
Bind Time 0.67s (± 1.26%) 0.67s (± 0.88%) +0.00s (+ 0.45%) 0.66s 0.69s
Check Time 4.47s (± 0.91%) 4.46s (± 0.71%) -0.00s (- 0.09%) 4.42s 4.57s
Emit Time 2.92s (± 0.79%) 2.89s (± 0.91%) -0.03s (- 1.20%) 2.82s 2.93s
Total Time 9.26s (± 0.71%) 9.23s (± 0.46%) -0.03s (- 0.37%) 9.15s 9.33s
material-ui - node (v10.16.3, x64)
Memory used 461,337k (± 0.01%) 461,230k (± 0.01%) -107k (- 0.02%) 461,089k 461,337k
Parse Time 1.96s (± 0.51%) 1.95s (± 0.57%) -0.01s (- 0.36%) 1.93s 1.98s
Bind Time 0.66s (± 1.14%) 0.66s (± 1.16%) -0.01s (- 1.36%) 0.63s 0.67s
Check Time 13.52s (± 1.00%) 13.49s (± 0.62%) -0.04s (- 0.28%) 13.28s 13.72s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.15s (± 0.87%) 16.10s (± 0.55%) -0.05s (- 0.31%) 15.90s 16.34s
Angular - node (v12.1.0, x64)
Memory used 321,687k (± 0.03%) 321,555k (± 0.02%) -132k (- 0.04%) 321,463k 321,704k
Parse Time 1.99s (± 0.78%) 1.99s (± 0.65%) -0.01s (- 0.30%) 1.96s 2.01s
Bind Time 0.81s (± 0.74%) 0.81s (± 0.59%) +0.00s (+ 0.25%) 0.80s 0.82s
Check Time 4.67s (± 0.46%) 4.69s (± 0.41%) +0.01s (+ 0.26%) 4.65s 4.72s
Emit Time 5.36s (± 1.08%) 5.39s (± 1.17%) +0.02s (+ 0.43%) 5.30s 5.60s
Total Time 12.83s (± 0.37%) 12.87s (± 0.63%) +0.03s (+ 0.24%) 12.72s 13.12s
Monaco - node (v12.1.0, x64)
Memory used 321,619k (± 0.01%) 321,641k (± 0.02%) +22k (+ 0.01%) 321,377k 321,757k
Parse Time 1.53s (± 0.80%) 1.54s (± 0.75%) +0.01s (+ 0.72%) 1.52s 1.56s
Bind Time 0.69s (± 0.96%) 0.69s (± 0.58%) -0.00s (- 0.29%) 0.68s 0.70s
Check Time 4.77s (± 0.31%) 4.78s (± 0.53%) +0.01s (+ 0.27%) 4.72s 4.84s
Emit Time 2.82s (± 0.90%) 2.81s (± 0.93%) -0.01s (- 0.46%) 2.76s 2.89s
Total Time 9.81s (± 0.31%) 9.81s (± 0.51%) +0.01s (+ 0.06%) 9.75s 9.97s
TFS - node (v12.1.0, x64)
Memory used 286,688k (± 0.03%) 286,585k (± 0.01%) -103k (- 0.04%) 286,503k 286,694k
Parse Time 1.23s (± 0.76%) 1.23s (± 0.63%) -0.00s (- 0.16%) 1.21s 1.25s
Bind Time 0.64s (± 1.09%) 0.64s (± 1.57%) +0.00s (+ 0.62%) 0.63s 0.67s
Check Time 4.36s (± 0.59%) 4.36s (± 0.60%) +0.00s (+ 0.02%) 4.32s 4.45s
Emit Time 2.92s (± 0.71%) 2.92s (± 0.59%) -0.00s (- 0.17%) 2.86s 2.94s
Total Time 9.15s (± 0.48%) 9.15s (± 0.37%) -0.00s (- 0.01%) 9.10s 9.26s
material-ui - node (v12.1.0, x64)
Memory used 439,707k (± 0.01%) 439,555k (± 0.06%) -152k (- 0.03%) 438,489k 439,782k
Parse Time 1.98s (± 0.53%) 1.97s (± 0.56%) -0.01s (- 0.25%) 1.94s 1.99s
Bind Time 0.63s (± 0.91%) 0.62s (± 1.09%) -0.01s (- 0.95%) 0.61s 0.64s
Check Time 12.17s (± 1.26%) 12.09s (± 0.96%) -0.07s (- 0.59%) 11.84s 12.40s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 14.77s (± 1.06%) 14.69s (± 0.81%) -0.08s (- 0.57%) 14.43s 14.98s
Angular - node (v8.9.0, x64)
Memory used 341,102k (± 0.02%) 340,929k (± 0.02%) -173k (- 0.05%) 340,804k 341,099k
Parse Time 2.54s (± 0.58%) 2.53s (± 0.44%) -0.01s (- 0.39%) 2.51s 2.56s
Bind Time 0.85s (± 0.58%) 0.85s (± 0.78%) +0.00s (+ 0.47%) 0.83s 0.86s
Check Time 5.40s (± 0.48%) 5.42s (± 0.47%) +0.02s (+ 0.31%) 5.36s 5.48s
Emit Time 5.92s (± 0.49%) 5.88s (± 1.16%) -0.05s (- 0.83%) 5.63s 5.98s
Total Time 14.72s (± 0.30%) 14.68s (± 0.59%) -0.04s (- 0.28%) 14.39s 14.81s
Monaco - node (v8.9.0, x64)
Memory used 340,627k (± 0.01%) 340,590k (± 0.02%) -37k (- 0.01%) 340,416k 340,664k
Parse Time 1.87s (± 0.41%) 1.87s (± 0.36%) +0.00s (+ 0.11%) 1.86s 1.89s
Bind Time 0.89s (± 0.56%) 0.89s (± 0.58%) +0.00s (+ 0.34%) 0.88s 0.90s
Check Time 5.48s (± 0.53%) 5.50s (± 0.38%) +0.02s (+ 0.36%) 5.46s 5.54s
Emit Time 3.21s (± 0.58%) 3.24s (± 1.10%) +0.03s (+ 0.78%) 3.19s 3.35s
Total Time 11.45s (± 0.32%) 11.51s (± 0.49%) +0.05s (+ 0.47%) 11.43s 11.68s
TFS - node (v8.9.0, x64)
Memory used 303,971k (± 0.03%) 303,938k (± 0.01%) -33k (- 0.01%) 303,834k 304,063k
Parse Time 1.55s (± 0.57%) 1.55s (± 0.52%) -0.00s (- 0.13%) 1.53s 1.57s
Bind Time 0.68s (± 0.33%) 0.67s (± 0.60%) -0.01s (- 1.18%) 0.66s 0.68s
Check Time 5.22s (± 0.68%) 5.19s (± 0.55%) -0.03s (- 0.58%) 5.14s 5.27s
Emit Time 2.92s (± 0.94%) 2.93s (± 0.66%) +0.01s (+ 0.38%) 2.89s 2.98s
Total Time 10.37s (± 0.42%) 10.34s (± 0.38%) -0.03s (- 0.29%) 10.26s 10.44s
material-ui - node (v8.9.0, x64)
Memory used 465,698k (± 0.01%) 465,689k (± 0.01%) -10k (- 0.00%) 465,582k 465,804k
Parse Time 2.38s (± 0.47%) 2.38s (± 0.71%) +0.00s (+ 0.13%) 2.35s 2.42s
Bind Time 0.78s (± 1.65%) 0.78s (± 1.21%) +0.00s (+ 0.13%) 0.76s 0.81s
Check Time 17.87s (± 1.28%) 17.94s (± 1.02%) +0.07s (+ 0.41%) 17.48s 18.30s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 21.03s (± 1.08%) 21.10s (± 0.84%) +0.07s (+ 0.35%) 20.63s 21.43s
Angular - node (v8.9.0, x86)
Memory used 195,669k (± 0.02%) 195,619k (± 0.02%) -51k (- 0.03%) 195,532k 195,683k
Parse Time 2.46s (± 0.72%) 2.46s (± 0.63%) +0.01s (+ 0.29%) 2.43s 2.48s
Bind Time 0.99s (± 0.60%) 0.98s (± 0.90%) -0.01s (- 0.61%) 0.95s 0.99s
Check Time 4.90s (± 0.44%) 4.87s (± 0.58%) -0.03s (- 0.57%) 4.82s 4.95s
Emit Time 5.91s (± 0.73%) 5.88s (± 1.24%) -0.03s (- 0.42%) 5.73s 6.04s
Total Time 14.24s (± 0.28%) 14.19s (± 0.63%) -0.06s (- 0.40%) 14.00s 14.44s
Monaco - node (v8.9.0, x86)
Memory used 193,655k (± 0.01%) 193,665k (± 0.02%) +10k (+ 0.01%) 193,565k 193,777k
Parse Time 1.90s (± 0.53%) 1.92s (± 1.36%) +0.02s (+ 0.94%) 1.87s 1.97s
Bind Time 0.70s (± 0.82%) 0.71s (± 2.00%) +0.01s (+ 1.00%) 0.69s 0.76s
Check Time 5.60s (± 1.21%) 5.58s (± 1.06%) -0.02s (- 0.32%) 5.37s 5.67s
Emit Time 2.71s (± 2.76%) 2.70s (± 3.13%) -0.01s (- 0.41%) 2.64s 3.04s
Total Time 10.92s (± 0.47%) 10.91s (± 0.57%) -0.01s (- 0.05%) 10.81s 11.05s
TFS - node (v8.9.0, x86)
Memory used 173,889k (± 0.04%) 173,871k (± 0.02%) -19k (- 0.01%) 173,791k 173,919k
Parse Time 1.59s (± 1.11%) 1.59s (± 1.31%) -0.00s (- 0.13%) 1.56s 1.65s
Bind Time 0.65s (± 1.72%) 0.65s (± 1.78%) +0.00s (+ 0.15%) 0.63s 0.68s
Check Time 4.73s (± 0.66%) 4.75s (± 0.45%) +0.02s (+ 0.36%) 4.72s 4.80s
Emit Time 2.80s (± 1.40%) 2.77s (± 0.86%) -0.03s (- 0.89%) 2.70s 2.81s
Total Time 9.76s (± 0.56%) 9.75s (± 0.43%) -0.01s (- 0.11%) 9.66s 9.86s
material-ui - node (v8.9.0, x86)
Memory used 263,677k (± 0.01%) 263,658k (± 0.01%) -19k (- 0.01%) 263,608k 263,744k
Parse Time 2.44s (± 0.74%) 2.42s (± 0.55%) -0.02s (- 0.70%) 2.40s 2.45s
Bind Time 0.68s (± 1.92%) 0.67s (± 1.66%) -0.01s (- 1.77%) 0.65s 0.70s
Check Time 16.59s (± 0.56%) 16.49s (± 0.62%) -0.11s (- 0.63%) 16.23s 16.71s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.71s (± 0.50%) 19.58s (± 0.52%) -0.13s (- 0.67%) 19.31s 19.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)
Benchmark Name Iterations
Current 40260 10
Baseline master 10

@sandersn sandersn added the Breaking Change Would introduce errors in existing code label Sep 8, 2020
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Sep 8, 2020

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 8, 2020 •
edited
Loading

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 8, 2020 •
edited
Loading

Heya @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
Copy link
Collaborator

typescript-bot commented Sep 8, 2020 •
edited
Loading

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

sandersn
sandersn requested changes Sep 8, 2020
View reviewed changes
Copy link
Member

@sandersn sandersn left a comment

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?

@sandersn sandersn self-assigned this Sep 8, 2020
@a-tarasyuk a-tarasyuk marked this pull request as draft September 8, 2020 18:41
@a-tarasyuk a-tarasyuk marked this pull request as ready for review September 28, 2020 12:45
sandersn
sandersn requested changes Sep 30, 2020
View reviewed changes
Copy link
Member

@sandersn sandersn left a comment

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?

@sandersn sandersn merged commit eaf4f46 into microsoft:master Oct 7, 2020
@sandersn
Copy link
Member

sandersn commented Oct 7, 2020

@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 added a commit that referenced this pull request Oct 7, 2020
@sandersn
Revert "feat(40197): handle uncalled function checks in binary expres…
cf3e28e 
…sions (#40260)"

This reverts commit eaf4f46.
@sandersn
Copy link
Member

sandersn commented Oct 7, 2020

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
Contributor Author

a-tarasyuk commented Oct 8, 2020

@sandersn Do I need to re-open PR?

@orta
Copy link
Contributor

orta commented Oct 8, 2020 •
edited
Loading

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

@sandersn
Copy link
Member

sandersn commented Oct 8, 2020

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

@a-tarasyuk
Copy link
Contributor Author

a-tarasyuk commented Oct 9, 2020

@orta @sandersn Ok, thanks

sandersn added a commit that referenced this pull request Nov 9, 2020
@sandersn
Revert "Revert "feat(40197): handle uncalled function checks in binar…
d40de9c 
…y expressions (#40260)""

This reverts commit cf3e28e.
@sandersn sandersn mentioned this pull request Nov 9, 2020
Merged
sandersn added a commit that referenced this pull request Nov 9, 2020
@sandersn
Revert "Revert "feat(40197): handle uncalled function checks in binar…
64be2a8 
…y expressions (#40260)"" (#41462)

This reverts commit cf3e28e.
@a-tarasyuk a-tarasyuk mentioned this pull request Nov 19, 2020
Closed
@aweebit aweebit mentioned this pull request Jun 1, 2025
Closed
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn sandersn sandersn approved these changes

@DanielRosenwasser DanielRosenwasser DanielRosenwasser left review comments

@RyanCavanaugh RyanCavanaugh RyanCavanaugh requested changes

Assignees

@sandersn sandersn

Labels

Breaking Change Would introduce errors in existing code For Backlog Bug PRs that fix a backlog bug

Projects

PR Backlog
Archived in project

Milestone

No milestone

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