Skip to content

Do not classify Infinity and NaN#44778

Merged
orta merged 4 commits intomicrosoft:mainfrom
jeanp413:fix-42022
Aug 4, 2021
Merged

Do not classify Infinity and NaN#44778
orta merged 4 commits intomicrosoft:mainfrom
jeanp413:fix-42022

Conversation

@jeanp413
Copy link
Copy Markdown
Contributor

Fixes #42022

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Jun 28, 2021
Comment on lines +228 to +231
function isInfinityOrNaNString(name: __String): boolean {
return name === "Infinity" || name === "-Infinity" || name === "NaN";
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if there's a better way to check for Infinity and NaN, I copied this from checker.ts line 26202.
Let me know if there's a better way to fix this.

Comment thread src/services/classifier2020.ts Outdated
@mjbvz mjbvz removed their assignment Jun 29, 2021
@sandersn sandersn requested a review from orta July 15, 2021 22:48
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

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

Sorry, I thought we'd have to expose something from the checker, but the logic is exactly the same as that of the type-checker. I think I'm going to need to back-pedal a bit and suggest to just check against Infinity and NaN, since exposing another API now seems overkill.

On that note, could you add these cases to the existing test?

// Regular properties

const obj1 = {
    Infinity: 100,
    NaN: 200,
    "-Infinity": 300
};

obj1.Infinity;
obj1.NaN;
obj1["-Infinity"];

// Shorthand properties

const obj2 = {
    Infinity,
    NaN,
}

obj2.Infinity;
obj2.NaN;

Comment thread tests/cases/fourslash/semanticModernClassificationInfinityAndNaN.ts Outdated
@orta
Copy link
Copy Markdown
Contributor

orta commented Jul 19, 2021

Cool, improved the tests and made it only check for "Infinity" and not -Infinity (which didn't make a difference on the tests, so wasn't doing work anyway)

@orta orta merged commit 1cbb0bd into microsoft:main Aug 4, 2021
@jeanp413 jeanp413 deleted the fix-42022 branch August 4, 2021 18:18
@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.

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

JS/TS semantic highlighting for keywords (readonly global vars)

6 participants