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.

Sign up for GitHub

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

Disallow numeric literals with negative numbers #55268

Merged
merged 2 commits into from
Aug 7, 2023
Merged

Disallow numeric literals with negative numbers #55268

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
167a0ee
Disallow numeric literals with negative numbers
Andarist Aug 4, 2023
7fc1803
tweaks
Andarist Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8112,7 +8112,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return factory.createStringLiteral(name, !!singleQuote);
}
if (isNumericLiteralName(name) && startsWith(name, "-")) {
return factory.createComputedPropertyName(factory.createNumericLiteral(+name));
return factory.createComputedPropertyName(factory.createPrefixUnaryExpression(SyntaxKind.MinusToken, factory.createNumericLiteral(Math.abs(+name))));
}
return createPropertyNameNodeForIdentifierOrLiteral(name, getEmitScriptTarget(compilerOptions));
}
Expand Down Expand Up @@ -38003,9 +38003,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return hasSkipDirectInferenceFlag(node) ?
wildcardType :
getFreshTypeOfLiteralType(getStringLiteralType((node as StringLiteralLike).text));
case SyntaxKind.NumericLiteral:
case SyntaxKind.NumericLiteral: {
checkGrammarNumericLiteral(node as NumericLiteral);
return getFreshTypeOfLiteralType(getNumberLiteralType(+(node as NumericLiteral).text));
const value = +(node as NumericLiteral).text;
if (!isFinite(value)) {
return numberType;
}
return getFreshTypeOfLiteralType(getNumberLiteralType(value));
}
case SyntaxKind.BigIntLiteral:
checkGrammarBigIntLiteral(node as BigIntLiteral);
return getFreshTypeOfLiteralType(getBigIntLiteralType({
Expand Down Expand Up @@ -46777,8 +46782,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (enumResult) return enumResult;
const literalValue = (type as LiteralType).value;
return typeof literalValue === "object" ? factory.createBigIntLiteral(literalValue) :
typeof literalValue === "number" ? factory.createNumericLiteral(literalValue) :
factory.createStringLiteral(literalValue);
typeof literalValue === "string" ? factory.createStringLiteral(literalValue) :
literalValue < 0 ? factory.createPrefixUnaryExpression(SyntaxKind.MinusToken, factory.createNumericLiteral(Math.abs(literalValue))) :
factory.createNumericLiteral(literalValue);
}

function createLiteralConstValue(node: VariableDeclaration | PropertyDeclaration | PropertySignature | ParameterDeclaration, tracker: SymbolTracker) {
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/factory/nodeFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
CaseOrDefaultClause,
cast,
CatchClause,
CharacterCodes,
ClassDeclaration,
ClassElement,
ClassExpression,
Expand Down Expand Up @@ -1100,8 +1101,10 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode

// @api
function createNumericLiteral(value: string | number, numericLiteralFlags: TokenFlags = TokenFlags.None): NumericLiteral {
const text = typeof value === "number" ? value + "" : value;
Debug.assert(text.charCodeAt(0) !== CharacterCodes.minus, "Negative numbers should be created in combination with createPrefixUnaryExpression");
const node = createBaseDeclaration<NumericLiteral>(SyntaxKind.NumericLiteral);
node.text = typeof value === "number" ? value + "" : value;
node.text = text;
node.numericLiteralFlags = numericLiteralFlags;
if (numericLiteralFlags & TokenFlags.BinaryOrOctalSpecifier) node.transformFlags |= TransformFlags.ContainsES2015;
return node;
Expand Down
9 changes: 8 additions & 1 deletion src/compiler/transformers/declarations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1765,7 +1765,14 @@ export function transformDeclarations(context: TransformationContext) {
if (shouldStripInternal(m)) return;
// Rewrite enum values to their constants, if available
const constValue = resolver.getConstantValue(m);
return preserveJsDoc(factory.updateEnumMember(m, m.name, constValue !== undefined ? typeof constValue === "string" ? factory.createStringLiteral(constValue) : factory.createNumericLiteral(constValue) : undefined), m);
const newInitializer = typeof constValue === "undefined"
? undefined
Andarist marked this conversation as resolved.
Show resolved Hide resolved
: typeof constValue === "string"
? factory.createStringLiteral(constValue)
: constValue < 0
? factory.createPrefixUnaryExpression(SyntaxKind.MinusToken, factory.createNumericLiteral(Math.abs(constValue)))
: factory.createNumericLiteral(constValue);
return preserveJsDoc(factory.updateEnumMember(m, m.name, newInitializer), m);
}))));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/generators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2528,7 +2528,7 @@ export function transformGenerators(context: TransformationContext): (x: SourceF
labelExpressions = [];
}

const expression = factory.createNumericLiteral(-1);
const expression = factory.createNumericLiteral(Infinity);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This shows how createNumericLiteral is still a somewhat weird thing. Infinity is allowed because parsed numbers are "normalized" to text here:

TypeScript/src/compiler/scanner.ts

Lines 1765 to 1773 in 83f3abd

else { // not a bigint, so can convert to number in simplified form
// Number() may not support 0b or 0o, so use parseInt() instead
const numericValue = tokenFlags & TokenFlags.BinarySpecifier
? parseInt(tokenValue.slice(2), 2) // skip "0b"
: tokenFlags & TokenFlags.OctalSpecifier
? parseInt(tokenValue.slice(2), 8) // skip "0o"
: +tokenValue;
tokenValue = "" + numericValue;
return SyntaxKind.NumericLiteral;

So a sufficiently long number in the input can be stringified to Infinity 🤷‍♂️ I find it weird an there is an issue about it that is labeled as a bug. While it doesn't mention Infinity case the issue there is basically related to the same "normalization".

This issue is certainly fixable but I'm not 100% sure if you'd really want to fix it.

Copy link
Member

Choose a reason for hiding this comment

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

Don't change this to Infinity since we really should consider that invalid as well. I'd just use a large value that we're unlikely to actually hit in a generator, something like 2 ** 32 - 1 (max uint32). The only reason it's set to -1 is to make it obvious something went wrong in tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't change this to Infinity

Ye, I committed it this way specifically to add this PR comment 😅

since we really should consider that invalid as well.

Strong agree, I had versions of this work that forbid this as well but decided to roll them back and only start the conversation about this for now. I'm open to following up with further changes but it seems that the mentioned "normalization" of parsed numeric literals is relied upon in a couple of places. They can be adjusted but I'm not sure if this is something you'd like to do. In a way, it would be easiest to just drop this whole normalization altogether and just cast parsed text to string in those other places (to produce correct resolved property names etc).

I'd just use a large value that we're unlikely to actually hit in a generator, something like 2 ** 32 - 1 (max uint32).

pushed out a change with Number.MAX_SAFE_INTEGER

if (labelExpressions[label] === undefined) {
labelExpressions[label] = [expression];
}
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/transformers/ts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1888,7 +1888,9 @@ export function transformTypeScript(context: TransformationContext) {
function transformEnumMemberDeclarationValue(member: EnumMember): Expression {
const value = resolver.getConstantValue(member);
if (value !== undefined) {
return typeof value === "string" ? factory.createStringLiteral(value) : factory.createNumericLiteral(value);
return typeof value === "string" ? factory.createStringLiteral(value) :
value < 0 ? factory.createPrefixUnaryExpression(SyntaxKind.MinusToken, factory.createNumericLiteral(Math.abs(value))) :
factory.createNumericLiteral(value);
}
else {
enableSubstitutionForNonQualifiedEnumMembers();
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/binaryIntegerLiteral.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var bin3 = 0B1111111111111111111111111111111111111111111111110100101010000001011

var bin4 = 0B111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111101001010100000010111110001111111111;
>bin4 : number
>0B111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111101001010100000010111110001111111111 : Infinity
>0B111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111101001010100000010111110001111111111 : number
Copy link
Member

Choose a reason for hiding this comment

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

I really hope no one is relying on this.


var obj1 = {
>obj1 : { 26: string; a: number; bin1: number; b: number; Infinity: boolean; }
Expand Down
Loading