Skip to content

Avoid binding readonly properties as special properties on a function symbol #49146

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Avoid binding readonly properties as special properties on a function symbol #49146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6046ffb
Avoid binding `length` property as special property on a function symbol
Andarist May 17, 2022
9438d2b
Avoid binding `name` property as special property on a function symbol
Andarist May 17, 2022
608b28e
Merge remote-tracking branch 'origin/main' into fix/fn-readonly-length
Andarist Feb 20, 2023
261ed86
Merge remote-tracking branch 'origin/main' into fix/fn-readonly-length
Andarist Jun 13, 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
10 changes: 9 additions & 1 deletion src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3285,9 +3285,17 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
function bindSpecialPropertyAssignment(node: BindablePropertyAssignmentExpression) {
// Class declarations in Typescript do not allow property declarations
const parentSymbol = lookupSymbolForPropertyAccess(node.left.expression, container) || lookupSymbolForPropertyAccess(node.left.expression, blockScopeContainer) ;
if (!isInJSFile(node) && !isFunctionSymbol(parentSymbol)) {
const isFunctionSym = isFunctionSymbol(parentSymbol);
if (!isInJSFile(node) && !isFunctionSym) {
return;
}
if (isFunctionSym && isBindableStaticNameExpression(node.left)) {
const declarationName = getDeclarationName(node.left);
// specialcase readonly properties of functions as we don't have access to type info here
if (declarationName === "length" || declarationName === "name") {
return;
}
}
const rootExpr = getLeftmostAccessExpression(node.left);
if (isIdentifier(rootExpr) && lookupSymbolForName(container, rootExpr.escapedText)?.flags! & SymbolFlags.Alias) {
return;
Expand Down
27 changes: 27 additions & 0 deletions tests/baselines/reference/functionReadonlyLength.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
functionReadonlyLength.ts(2,4): error TS2540: Cannot assign to 'length' because it is a read-only property.
functionReadonlyLength.ts(5,4): error TS2540: Cannot assign to 'length' because it is a read-only property.
functionReadonlyLength.ts(8,4): error TS2540: Cannot assign to 'length' because it is a read-only property.
functionReadonlyLength.ts(11,4): error TS2540: Cannot assign to 'length' because it is a read-only property.


==== functionReadonlyLength.ts (4 errors) ====
const f0 = new Function()
f0.length = 1;
~~~~~~
!!! error TS2540: Cannot assign to 'length' because it is a read-only property.

function f1() {};
f1.length = 1;
~~~~~~
!!! error TS2540: Cannot assign to 'length' because it is a read-only property.

const f2 = function () {};
f2.length = 1
~~~~~~
!!! error TS2540: Cannot assign to 'length' because it is a read-only property.

const f3 = () => {}
f3.length = 1
~~~~~~
!!! error TS2540: Cannot assign to 'length' because it is a read-only property.

36 changes: 36 additions & 0 deletions tests/baselines/reference/functionReadonlyLength.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//// [tests/cases/compiler/functionReadonlyLength.ts] ////

=== functionReadonlyLength.ts ===
const f0 = new Function()
>f0 : Symbol(f0, Decl(functionReadonlyLength.ts, 0, 5))
>Function : Symbol(Function, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))

f0.length = 1;
>f0.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
>f0 : Symbol(f0, Decl(functionReadonlyLength.ts, 0, 5))
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))

function f1() {};
>f1 : Symbol(f1, Decl(functionReadonlyLength.ts, 1, 14))

f1.length = 1;
>f1.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
>f1 : Symbol(f1, Decl(functionReadonlyLength.ts, 1, 14))
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))

const f2 = function () {};
>f2 : Symbol(f2, Decl(functionReadonlyLength.ts, 6, 5))

f2.length = 1
>f2.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
>f2 : Symbol(f2, Decl(functionReadonlyLength.ts, 6, 5))
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))

const f3 = () => {}
>f3 : Symbol(f3, Decl(functionReadonlyLength.ts, 9, 5))

f3.length = 1
>f3.length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))
>f3 : Symbol(f3, Decl(functionReadonlyLength.ts, 9, 5))
>length : Symbol(Function.length, Decl(lib.es5.d.ts, --, --))

47 changes: 47 additions & 0 deletions tests/baselines/reference/functionReadonlyLength.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//// [tests/cases/compiler/functionReadonlyLength.ts] ////

=== functionReadonlyLength.ts ===
const f0 = new Function()
>f0 : Function
>new Function() : Function
>Function : FunctionConstructor

f0.length = 1;
>f0.length = 1 : 1
>f0.length : any
Copy link
Contributor Author

@Andarist Andarist May 17, 2022

Choose a reason for hiding this comment

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

this feels wrong - perhaps the .types printer tries to only lookup the "target" symbol of this assignment and doesn't try to look into Function here etc. Should I try to fix this as part of this PR?

Copy link
Contributor

@rbuckton rbuckton May 17, 2022

Choose a reason for hiding this comment

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

This may be fine. What the type writer is saying is that the type of the expression f0.length = 1 is 1, which is correct:

// note: NOT in strict mode. In strict mode, attempting to assign to length will throw
function f(a, b) { }
console.log(f.length); // 2

const i = f.length = 1;
console.log(i); // 1
console.log(f.length); // 2

Copy link
Contributor Author

@Andarist Andarist May 17, 2022

Choose a reason for hiding this comment

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

I was specifically referring to the 9th line that has f0.length : any and it seems that this any is a little bit off here as I would expect this to be of type number 🤔

Copy link
Contributor Author

@Andarist Andarist May 17, 2022

Choose a reason for hiding this comment

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

Ah, but actually this is already the case for new Function case so perhaps this is how it's printed for the readonly LHS. It's still a little bit confusing to me - but at least my PR doesn't really change anything in this regard 🤷‍♂️

>f0 : Function
>length : any
>1 : 1

function f1() {};
>f1 : () => void

f1.length = 1;
>f1.length = 1 : 1
>f1.length : any
>f1 : () => void
>length : any
>1 : 1

const f2 = function () {};
>f2 : () => void
>function () {} : () => void

f2.length = 1
>f2.length = 1 : 1
>f2.length : any
>f2 : () => void
>length : any
>1 : 1

const f3 = () => {}
>f3 : () => void
>() => {} : () => void

f3.length = 1
>f3.length = 1 : 1
>f3.length : any
>f3 : () => void
>length : any
>1 : 1

27 changes: 27 additions & 0 deletions tests/baselines/reference/functionReadonlyName.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
functionReadonlyName.ts(2,4): error TS2339: Property 'name' does not exist on type 'Function'.
functionReadonlyName.ts(5,4): error TS2339: Property 'name' does not exist on type '() => void'.
functionReadonlyName.ts(8,4): error TS2339: Property 'name' does not exist on type '() => void'.
functionReadonlyName.ts(11,4): error TS2339: Property 'name' does not exist on type '() => void'.


==== functionReadonlyName.ts (4 errors) ====
const f0 = new Function()
f0.name = 'foo';
~~~~
!!! error TS2339: Property 'name' does not exist on type 'Function'.

function f1() {};
f1.name = 'foo';
~~~~
!!! error TS2339: Property 'name' does not exist on type '() => void'.

const f2 = function () {};
f2.name = 'foo'
~~~~
!!! error TS2339: Property 'name' does not exist on type '() => void'.

const f3 = () => {}
f3.name = 'foo'
~~~~
!!! error TS2339: Property 'name' does not exist on type '() => void'.

28 changes: 28 additions & 0 deletions tests/baselines/reference/functionReadonlyName.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//// [tests/cases/compiler/functionReadonlyName.ts] ////

=== functionReadonlyName.ts ===
const f0 = new Function()
>f0 : Symbol(f0, Decl(functionReadonlyName.ts, 0, 5))
>Function : Symbol(Function, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))

f0.name = 'foo';
>f0 : Symbol(f0, Decl(functionReadonlyName.ts, 0, 5))

function f1() {};
>f1 : Symbol(f1, Decl(functionReadonlyName.ts, 1, 16))

f1.name = 'foo';
>f1 : Symbol(f1, Decl(functionReadonlyName.ts, 1, 16))

const f2 = function () {};
>f2 : Symbol(f2, Decl(functionReadonlyName.ts, 6, 5))

f2.name = 'foo'
>f2 : Symbol(f2, Decl(functionReadonlyName.ts, 6, 5))

const f3 = () => {}
>f3 : Symbol(f3, Decl(functionReadonlyName.ts, 9, 5))

f3.name = 'foo'
>f3 : Symbol(f3, Decl(functionReadonlyName.ts, 9, 5))

47 changes: 47 additions & 0 deletions tests/baselines/reference/functionReadonlyName.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//// [tests/cases/compiler/functionReadonlyName.ts] ////

=== functionReadonlyName.ts ===
const f0 = new Function()
>f0 : Function
>new Function() : Function
>Function : FunctionConstructor

f0.name = 'foo';
>f0.name = 'foo' : "foo"
>f0.name : any
>f0 : Function
>name : any
>'foo' : "foo"

function f1() {};
>f1 : () => void

f1.name = 'foo';
>f1.name = 'foo' : "foo"
>f1.name : any
>f1 : () => void
>name : any
>'foo' : "foo"

const f2 = function () {};
>f2 : () => void
>function () {} : () => void

f2.name = 'foo'
>f2.name = 'foo' : "foo"
>f2.name : any
>f2 : () => void
>name : any
>'foo' : "foo"

const f3 = () => {}
>f3 : () => void
>() => {} : () => void

f3.name = 'foo'
>f3.name = 'foo' : "foo"
>f3.name : any
>f3 : () => void
>name : any
>'foo' : "foo"

13 changes: 13 additions & 0 deletions tests/cases/compiler/functionReadonlyLength.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @noEmit: true

const f0 = new Function()
f0.length = 1;

function f1() {};
f1.length = 1;

const f2 = function () {};
f2.length = 1

const f3 = () => {}
f3.length = 1
13 changes: 13 additions & 0 deletions tests/cases/compiler/functionReadonlyName.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @noEmit: true

const f0 = new Function()
f0.name = 'foo';

function f1() {};
f1.name = 'foo';

const f2 = function () {};
f2.name = 'foo'

const f3 = () => {}
f3.name = 'foo'