Skip to content

Error on references to literal enum members in conditions - #58264

Merged
Wesley Wigham (weswigham) merged 1 commit into
microsoft:mainfrom
weswigham:always-enum-error
Apr 24, 2024
Merged

Error on references to literal enum members in conditions#58264
Wesley Wigham (weswigham) merged 1 commit into
microsoft:mainfrom
weswigham:always-enum-error

Conversation

@weswigham

@weswigham Wesley Wigham (weswigham) commented Apr 20, 2024

Copy link
Copy Markdown
Member

So when you write

enum NodeKind {
    FunctionDeclaration = 0,
    VariableDeclaration = 1,
    ExpressionStatement = 2,
}

function kindToString(kind: NodeKind) {
    Debug.assert(!isExpressionStatementKind(kind));
    return NodeKind.FunctionDeclaration ? "func" : "var";
}

we inform you in advance that you've failed to actually write kind ===.

@typescript-bot TypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 20, 2024
@weswigham

Wesley Wigham (weswigham) commented Apr 20, 2024

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) test it

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 20, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started ✅ Results
user test this ✅ Started ✅ Results
run dt ✅ Started ✅ Results
perf test this faster ✅ Started 👀 Results

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 20, 2024

Copy link
Copy Markdown
Contributor

Hey Wesley Wigham (@weswigham), the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 20, 2024

Copy link
Copy Markdown
Contributor

Wesley Wigham (@weswigham) Here are the results of running the user tests comparing main and refs/pull/58264/merge:

Everything looks good!

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 20, 2024

Copy link
Copy Markdown
Contributor

Wesley Wigham (@weswigham)
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Angular - node (v18.15.0, x64)
Memory used 297,484k (± 0.01%) 297,458k (± 0.01%) ~ 297,443k 297,484k p=0.066 n=6
Parse Time 3.29s (± 0.23%) 3.29s (± 0.52%) ~ 3.27s 3.31s p=0.869 n=6
Bind Time 0.97s (± 0.53%) 0.97s (± 0.42%) ~ 0.97s 0.98s p=0.595 n=6
Check Time 9.81s (± 0.33%) 9.84s (± 0.33%) ~ 9.80s 9.87s p=0.162 n=6
Emit Time 8.45s (± 0.32%) 8.47s (± 0.17%) ~ 8.45s 8.49s p=0.191 n=6
Total Time 22.52s (± 0.17%) 22.58s (± 0.20%) ~ 22.53s 22.64s p=0.089 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used 193,959k (± 1.02%) 192,729k (± 0.76%) ~ 192,029k 195,730k p=0.230 n=6
Parse Time 2.03s (± 0.97%) 2.02s (± 1.58%) ~ 1.99s 2.07s p=0.520 n=6
Bind Time 1.07s (± 1.09%) 1.07s (± 0.84%) ~ 1.06s 1.08s p=0.672 n=6
Check Time 14.07s (± 0.51%) 14.06s (± 0.77%) ~ 13.91s 14.16s p=0.809 n=6
Emit Time 3.87s (± 0.77%) 3.89s (± 0.53%) ~ 3.85s 3.91s p=0.418 n=6
Total Time 21.05s (± 0.35%) 21.04s (± 0.38%) ~ 20.94s 21.13s p=0.936 n=6
mui-docs - node (v18.15.0, x64)
Memory used 1,753,238k (± 0.00%) 1,753,261k (± 0.00%) ~ 1,753,222k 1,753,298k p=0.230 n=6
Parse Time 6.94s (± 0.30%) 6.93s (± 0.22%) ~ 6.92s 6.96s p=0.511 n=6
Bind Time 2.31s (± 0.18%) 2.31s (± 0.36%) ~ 2.30s 2.32s p=0.097 n=6
Check Time 56.76s (± 0.56%) 56.75s (± 0.30%) ~ 56.56s 56.96s p=0.873 n=6
Emit Time 0.14s (± 2.88%) 0.14s (± 2.88%) ~ 0.14s 0.15s p=1.000 n=6
Total Time 66.15s (± 0.50%) 66.13s (± 0.23%) ~ 65.97s 66.34s p=1.000 n=6
self-build-src - node (v18.15.0, x64)
Memory used 2,322,214k (± 0.04%) 2,322,572k (± 0.04%) ~ 2,321,760k 2,324,125k p=0.810 n=6
Parse Time 7.56s (± 0.81%) 7.58s (± 0.40%) ~ 7.55s 7.63s p=0.630 n=6
Bind Time 2.73s (± 0.72%) 2.73s (± 0.82%) ~ 2.71s 2.77s p=0.808 n=6
Check Time 49.46s (± 0.76%) 49.26s (± 0.49%) ~ 48.98s 49.53s p=0.575 n=6
Emit Time 4.08s (± 1.79%) 4.02s (± 4.91%) ~ 3.80s 4.38s p=0.298 n=6
Total Time 63.84s (± 0.59%) 63.58s (± 0.45%) ~ 63.14s 63.85s p=0.378 n=6
self-build-src-public-api - node (v18.15.0, x64)
Memory used 2,396,448k (± 0.04%) 2,396,351k (± 0.03%) ~ 2,395,423k 2,397,135k p=0.810 n=6
Parse Time 7.82s (± 0.69%) 7.78s (± 0.84%) ~ 7.67s 7.87s p=0.423 n=6
Bind Time 2.46s (± 1.01%) 2.46s (± 0.97%) ~ 2.44s 2.49s p=0.870 n=6
Check Time 50.15s (± 0.72%) 50.08s (± 0.39%) ~ 49.87s 50.42s p=0.936 n=6
Emit Time 4.04s (± 2.79%) 4.07s (± 5.41%) ~ 3.84s 4.35s p=1.000 n=6
Total Time 64.47s (± 0.45%) 64.39s (± 0.51%) ~ 63.98s 64.81s p=0.810 n=6
self-compiler - node (v18.15.0, x64)
Memory used 423,932k (± 0.01%) 423,924k (± 0.01%) ~ 423,881k 423,973k p=0.873 n=6
Parse Time 2.90s (± 1.05%) 2.91s (± 0.71%) ~ 2.87s 2.93s p=0.935 n=6
Bind Time 1.09s (± 1.16%) 1.09s (± 0.37%) ~ 1.08s 1.09s p=0.753 n=6
Check Time 15.42s (± 0.38%) 15.41s (± 0.29%) ~ 15.33s 15.45s p=1.000 n=6
Emit Time 1.18s (± 1.13%) 1.17s (± 0.64%) ~ 1.16s 1.18s p=0.155 n=6
Total Time 20.59s (± 0.28%) 20.57s (± 0.28%) ~ 20.49s 20.65s p=0.687 n=6
ts-pre-modules - node (v18.15.0, x64)
Memory used 369,286k (± 0.01%) 369,273k (± 0.01%) ~ 369,250k 369,318k p=0.936 n=6
Parse Time 3.65s (± 0.75%) 3.67s (± 0.75%) ~ 3.63s 3.71s p=0.366 n=6
Bind Time 1.92s (± 2.19%) 1.93s (± 1.12%) ~ 1.90s 1.96s p=1.000 n=6
Check Time 19.46s (± 0.30%) 19.44s (± 0.42%) ~ 19.30s 19.54s p=0.809 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 25.03s (± 0.40%) 25.03s (± 0.47%) ~ 24.86s 25.19s p=0.872 n=6
vscode - node (v18.15.0, x64)
Memory used 2,920,809k (± 0.00%) 2,920,823k (± 0.00%) ~ 2,920,785k 2,920,849k p=0.423 n=6
Parse Time 16.64s (± 0.33%) 16.60s (± 0.23%) ~ 16.56s 16.66s p=0.195 n=6
Bind Time 4.96s (± 0.55%) 4.95s (± 1.00%) ~ 4.90s 5.02s p=0.872 n=6
Check Time 88.06s (± 0.11%) 87.89s (± 0.42%) ~ 87.49s 88.55s p=0.173 n=6
Emit Time 24.55s (± 7.26%) 23.71s (± 0.50%) ~ 23.60s 23.93s p=0.065 n=6
Total Time 134.20s (± 1.32%) 133.16s (± 0.27%) ~ 132.68s 133.65s p=0.093 n=6
webpack - node (v18.15.0, x64)
Memory used 409,945k (± 0.01%) 409,910k (± 0.02%) ~ 409,817k 410,030k p=0.298 n=6
Parse Time 4.89s (± 0.50%) 4.89s (± 0.93%) ~ 4.84s 4.97s p=0.618 n=6
Bind Time 2.02s (± 1.20%) 2.02s (± 0.85%) ~ 2.00s 2.04s p=0.871 n=6
Check Time 21.06s (± 0.37%) 21.13s (± 0.36%) ~ 21.05s 21.26s p=0.296 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 27.97s (± 0.30%) 28.04s (± 0.31%) ~ 27.95s 28.20s p=0.419 n=6
xstate-main - node (v18.15.0, x64)
Memory used 459,428k (± 0.02%) 459,427k (± 0.01%) ~ 459,383k 459,486k p=0.471 n=6
Parse Time 4.02s (± 0.30%) 4.00s (± 0.40%) ~ 3.98s 4.02s p=0.121 n=6
Bind Time 1.47s (± 0.70%) 1.47s (± 1.33%) ~ 1.45s 1.50s p=0.935 n=6
Check Time 22.42s (± 0.48%) 22.32s (± 0.64%) ~ 22.15s 22.51s p=0.298 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 27.90s (± 0.38%) 27.80s (± 0.56%) ~ 27.59s 27.98s p=0.261 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 20, 2024

Copy link
Copy Markdown
Contributor

Wesley Wigham (@weswigham) Here are the results of running the top 400 repos comparing main and refs/pull/58264/merge:

Everything looks good!

@fatcerberus

Bruce Pascoe (fatcerberus) commented Apr 21, 2024 •
edited
Loading

Copy link
Copy Markdown

Is this a common mistake? I don't think I've ever done this. I think I'd rather this be a more general check of "you're testing the value of a (pseudo-)constant, did you mean to do that?" which feels way more useful than trying to detect that I wrote if (y) instead of if (x === y)

@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Apr 22, 2024

Copy link
Copy Markdown
Member

I think I'd rather this be a more general check of "you're testing the value of a (pseudo-)constant, did you mean to do that?

I think we basically already have that in some cases but not all, e.g.

function foo() { }
if (foo) {
}

where your program has no plausible way of varying on foo

@gabritto

Gabriela Araujo Britto (gabritto) commented Apr 22, 2024

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) pack this

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 22, 2024

Copy link
Copy Markdown
Contributor

Hey Gabriela Araujo Britto (@gabritto), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/161409/artifacts?artifactName=tgz&fileId=6DEC9CAB3714C4E9D09BADA60D30625831D6CED0CF6BE9ACC45D0C7B100BE6B602&fileName=/typescript-5.5.0-insiders.20240422.tgz"
    }
}

and then running npm install.

@jakebailey

Jake Bailey (jakebailey) commented Apr 22, 2024

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) pack this

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot

TypeScript Bot (typescript-bot) commented Apr 22, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@jakebailey), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/161410/artifacts?artifactName=tgz&fileId=6DEC9CAB3714C4E9D09BADA60D30625831D6CED0CF6BE9ACC45D0C7B100BE6B602&fileName=/typescript-5.5.0-insiders.20240422.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.5.0-pr-58264-13".;

@weswigham
Wesley Wigham (weswigham) merged commit e6ba82b into microsoft:main Apr 24, 2024
@weswigham
Wesley Wigham (weswigham) deleted the always-enum-error branch April 24, 2024 17:33
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@gabritto Gabriela Araujo Britto (gabritto) gabritto approved these changes
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Awaiting requested review from DanielRosenwasser

Assignees

@weswigham Wesley Wigham (weswigham)

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@weswigham @typescript-bot @fatcerberus @RyanCavanaugh @gabritto @jakebailey