add support for BinaryExpression node #2411
Conversation
| return node.kind === ts.SyntaxKind.BinaryExpression; | ||
| } | ||
| public createType(node: ts.BinaryExpression, context: Context): BaseType { | ||
| // For the purposes of types, assume that binary expressions always |
There was a problem hiding this comment.
do they always evaluate to number? is 0n a binary exp too?
There was a problem hiding this comment.
"a" + "b" is also a binary expression. I don't think this works.
There was a problem hiding this comment.
some pseudocode I got with help from chatgpt:
impl(node: ts.BinaryExpression): 'string' | 'number' {
// You can assert this before calling, or guard here:
if (node.operatorToken.kind !== ts.SyntaxKind.PlusToken) {
throw new Error('createType only supports + binary expressions');
}
const leftType = this.checker.getTypeAtLocation(node.left);
const rightType = this.checker.getTypeAtLocation(node.right);
// 1) If either side is string-like → 'string'
if (this.isStringLike(leftType) || this.isStringLike(rightType)) {
return 'string';
}
// 2) If both sides are definitely number-like → 'number'
if (this.isDefinitelyNumberLike(leftType) && this.isDefinitelyNumberLike(rightType)) {
return 'number';
}
// 3) Anything else (objects, any, unknown, weird unions, etc.) → 'string'
// because at runtime + will usually go through ToPrimitive and end up
// in the "string concatenation" branch when non-numeric stuff is involved.
return 'string';
}
private isStringLike(type: ts.Type): boolean {
// Use apparent type to collapse things like literal unions, etc.
type = this.checker.getApparentType(type);
// Union? Any member being string-like is enough.
if (type.isUnion()) {
return type.types.some(t => this.isStringLike(t));
}
const f = type.flags;
// String primitives + string literals + template literals
if (f & ts.TypeFlags.StringLike) {
return true;
}
// Optionally treat String object type as string-like:
const symbol = type.getSymbol();
if (symbol && symbol.getName() === 'String') {
return true;
}
return false;
}
private isDefinitelyNumberLike(type: ts.Type): boolean {
// Again, use apparent type for unions/intersections
type = this.checker.getApparentType(type);
if (type.isUnion()) {
// Must be number-like for *all* members to be "definitely number-like"
return type.types.every(t => this.isDefinitelyNumberLike(t));
}
const f = type.flags;
// Number, number literal, enums, bigint etc.
// If you don't want bigint, drop BigIntLike.
const numericFlags =
ts.TypeFlags.NumberLike |
ts.TypeFlags.BigIntLike |
ts.TypeFlags.EnumLike;
if (f & numericFlags) {
return true;
}
return false;
}i hope it helps
There was a problem hiding this comment.
if (isAnyLike(left) || isAnyLike(right)) {
return any;
}
if (isStringLike(left) || isStringLike(right)) {
// includes unions containing string, template literal types, etc.
return string;
}
if (isNumberLike(left) && isNumberLike(right)) {
return number;
}
if (isBigIntLike(left) && isBigIntLike(right)) {
return bigint;
}
// mixtures: (string | number) + (string | number) → string | number, etc.
// if nothing fits cleanly, often falls back to any / error / unionThere was a problem hiding this comment.
Took another stab at this and added some more test cases.
| return node.kind === ts.SyntaxKind.BinaryExpression; | ||
| } | ||
| public createType(node: ts.BinaryExpression, context: Context): BaseType { | ||
| // For the purposes of types, assume that binary expressions always |
There was a problem hiding this comment.
"a" + "b" is also a binary expression. I don't think this works.
|
See below is what Google's AI mode tells me about possible evaluations, which broadly makes sense to me. The pseudcode you included was a good starting point, but I couldn't get it to quite work. The I've left in a couple things that were useful in debugging (an Google's Take:In TypeScript, a binary expression, which involves an operator acting on two operands, can evaluate to various types depending on the operator and the types of the operands. • Number: Arithmetic operators like +, -, *, /, % when applied to two number operands will result in a number. • String: The + operator, when one or both operands are string types, performs string concatenation and results in a string. • Boolean: Comparison operators (==, ===, !=, !==, <, >, <=, >=) and logical operators (&&, ||) always evaluate to a boolean value. • Any type: If one or both operands in a binary expression are of type any, the result of the expression will also typically be any. • Enum type: When enum members are used in arithmetic operations, they are treated as their underlying number values, and the result will be a number. • Union type: In conditional expressions (ternary operator ? :), the type of the expression will be a union of the types of the two possible results. AI responses may include mistakes. |
|
Please fix the tests. I'll let @arthurfiorette merge. |
|
Added some more tests to try and increase coverage. |
|
Thank you @srsudar |
|
🚀 PR was released in |
3 participants
Overview
We have a type that uses
default: 60 * 5. This breaks the current parser with an error like the one shown below.I think that the fully correct way to handle this would be to evaluate the expression and set it to something like
{ const: 300, type: number}.This implementation instead ignores the
constand widens the type to justnumber. This seems fine for my purposes--curious if this works for the maintainers.This is the error thrown by the test added in this PR before the fix:
Version
Published prerelease version:
v2.5.0-next.10Changelog
🎉 This release contains work from new contributors! 🎉
Thanks for all your work!
❤️ Sam Sudar (@srsudar)
❤️ Orta Therox (@orta)
❤️ James Vaughan (@jamesbvaughan)
❤️ Alex (@alexchexes)
❤️ Cal (@CalLavicka)
❤️ Valentyne Stigloher (@pixunil)
🚀 Enhancement
NewExpressionparser #2346 (@jamesbvaughan)🐛 Bug Fix
--additional-propertiesoption #2305 (@alexchexes)--type "*"is used with multiple exports #2284 (@alexchexes @arthurfiorette)symbol#2282 (@alexchexes)🔩 Dependency Updates
Authors: 9