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

Error on references to literal enum members in conditions #58264

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

weswigham
Copy link
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 added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 20, 2024
@weswigham
Copy link
Member Author

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 20, 2024

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

Hey @weswigham, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

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

Everything looks good!

@fatcerberus
Copy link

fatcerberus commented Apr 21, 2024

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
Copy link
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
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 22, 2024

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

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

Hey @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
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 22, 2024

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

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 22, 2024

Hey @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 weswigham merged commit e6ba82b into microsoft:main Apr 24, 2024
25 checks passed
@weswigham weswigham deleted the always-enum-error branch April 24, 2024 17:33
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.

None yet

6 participants