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

Fixing #442: Impossible to define static 'length' function on class #12065

Merged
merged 17 commits into from Jan 17, 2017
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/compiler/checker.ts
Expand Up @@ -14771,6 +14771,49 @@ namespace ts {
}
}

// Static members may conflict with non-configurable non-writable built-in Function object properties
Copy link
Member

Choose a reason for hiding this comment

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

Please use jsdoc format.

// see https://github.com/microsoft/typescript/issues/442.
Copy link
Member

Choose a reason for hiding this comment

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

instead of a github issue number, can you reference the spec the way you do below instead?

function checkClassForStaticPropertyNameConflicts(node: ClassLikeDeclaration) {
const message = Diagnostics.Static_property_0_conflicts_with_built_in_property_Function_0_of_constructor_function_1;
const className = getSymbolOfNode(node).name;
Copy link
Member

Choose a reason for hiding this comment

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

getNameOfSymbol(getSymbolOfNode(node)) would be better here. You may have to move getNameOfSymbol to make it accessible outside getSymbolDisplayBuilder.

Copy link
Member

Choose a reason for hiding this comment

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

Or just node.name, but that doesn't work for anonymous classes, unfortunately. I think that means some of the other error reporting in checkClassLikeDeclaration is broken, because it uses node.name.

for (const member of node.members) {
const isStatic = forEach(member.modifiers, (m: Modifier) => m.kind === SyntaxKind.StaticKeyword);
Copy link
Member

Choose a reason for hiding this comment

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

should be getModifierFlags(node) & ModifierFlags.Static -- this calculates the modifiers once and then caches them

Copy link
Member

Choose a reason for hiding this comment

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

Yikes, looks like checkClassForDuplicateDeclarations also uses this code. We should change that pretty soon.

const isMethod = member.kind === SyntaxKind.MethodDeclaration;
const memberNameNode = member.name;
if (isStatic && memberNameNode) {
const memberName = getPropertyNameForPropertyNameNode(memberNameNode);
if (languageVersion <= ScriptTarget.ES5) { // ES3, ES5
// see also http://www.ecma-international.org/ecma-262/5.1/#sec-15.3.3
// see also http://www.ecma-international.org/ecma-262/5.1/#sec-15.3.5
if (memberName === "prototype" ||
memberName === "name" ||
memberName === "length" ||
memberName === "caller" ||
memberName === "arguments"
) {
error(memberNameNode, message, memberName, className);
}
}
else { // ES6+
// see also http://www.ecma-international.org/ecma-262/6.0/#sec-properties-of-the-function-constructor
// see also http://www.ecma-international.org/ecma-262/6.0/#sec-function-instances
if (memberName === "prototype") {
error(memberNameNode, message, memberName, className);
}
else if ((
memberName === "name" ||
memberName === "length" ||
memberName === "caller" ||
memberName === "arguments") &&
isMethod === false
) {
error(memberNameNode, message, memberName, className);
}
}
}
}
}

function checkObjectTypeForDuplicateDeclarations(node: TypeLiteralNode | InterfaceDeclaration) {
const names = createMap<boolean>();
for (const member of node.members) {
Expand Down Expand Up @@ -17259,6 +17302,7 @@ namespace ts {
const staticType = <ObjectType>getTypeOfSymbol(symbol);
checkTypeParameterListsIdentical(node, symbol);
checkClassForDuplicateDeclarations(node);
checkClassForStaticPropertyNameConflicts(node);

const baseTypeNode = getClassExtendsHeritageClauseElement(node);
if (baseTypeNode) {
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/diagnosticMessages.json
Expand Up @@ -1983,7 +1983,10 @@
"category": "Error",
"code": 2697
},

"Static property '{0}' conflicts with built-in property 'Function.{0}' of constructor function '{1}'.": {
"category": "Error",
"code": 2699
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down Expand Up @@ -2268,7 +2271,6 @@
"category": "Message",
"code": 4090
},

"The current host does not support the '{0}' option.": {
"category": "Error",
"code": 5001
Expand Down
10 changes: 10 additions & 0 deletions tests/baselines/reference/propertyNamedPrototype.errors.txt
@@ -0,0 +1,10 @@
tests/cases/conformance/classes/propertyMemberDeclarations/propertyNamedPrototype.ts(3,12): error TS2699: Static property 'prototype' conflicts with built-in property 'Function.prototype' of constructor function 'C'.


==== tests/cases/conformance/classes/propertyMemberDeclarations/propertyNamedPrototype.ts (1 errors) ====
class C {
prototype: number; // ok
static prototype: C; // error
~~~~~~~~~
!!! error TS2699: Static property 'prototype' conflicts with built-in property 'Function.prototype' of constructor function 'C'.
}
@@ -1,43 +1,43 @@
tests/cases/compiler/staticMemberOfClassAndPublicMemberOfAnotherClassAssignment.ts(12,1): error TS2322: Type 'C' is not assignable to type 'A'.
Property 'name' is missing in type 'C'.
Property 'prop' is missing in type 'C'.
tests/cases/compiler/staticMemberOfClassAndPublicMemberOfAnotherClassAssignment.ts(13,1): error TS2322: Type 'typeof B' is not assignable to type 'A'.
Property 'name' is missing in type 'typeof B'.
Property 'prop' is missing in type 'typeof B'.
tests/cases/compiler/staticMemberOfClassAndPublicMemberOfAnotherClassAssignment.ts(16,5): error TS2322: Type 'C' is not assignable to type 'B'.
Property 'name' is missing in type 'C'.
Property 'prop' is missing in type 'C'.
tests/cases/compiler/staticMemberOfClassAndPublicMemberOfAnotherClassAssignment.ts(17,1): error TS2322: Type 'typeof B' is not assignable to type 'B'.
Property 'name' is missing in type 'typeof B'.
Property 'prop' is missing in type 'typeof B'.


==== tests/cases/compiler/staticMemberOfClassAndPublicMemberOfAnotherClassAssignment.ts (4 errors) ====
interface A {
name();
prop();
}
class B {
public name() { }
public prop() { }
}
class C {
public static name() { }
public static prop() { }
}

var a: A = new B();
a = new C(); // error name is missing
a = new C(); // error prop is missing
~
!!! error TS2322: Type 'C' is not assignable to type 'A'.
!!! error TS2322: Property 'name' is missing in type 'C'.
a = B; // error name is missing
!!! error TS2322: Property 'prop' is missing in type 'C'.
a = B; // error prop is missing
~
!!! error TS2322: Type 'typeof B' is not assignable to type 'A'.
!!! error TS2322: Property 'name' is missing in type 'typeof B'.
!!! error TS2322: Property 'prop' is missing in type 'typeof B'.
a = C;

var b: B = new C(); // error name is missing
var b: B = new C(); // error prop is missing
~
!!! error TS2322: Type 'C' is not assignable to type 'B'.
!!! error TS2322: Property 'name' is missing in type 'C'.
b = B; // error name is missing
!!! error TS2322: Property 'prop' is missing in type 'C'.
b = B; // error prop is missing
~
!!! error TS2322: Type 'typeof B' is not assignable to type 'B'.
!!! error TS2322: Property 'name' is missing in type 'typeof B'.
!!! error TS2322: Property 'prop' is missing in type 'typeof B'.
b = C;
b = a;

Expand Down
@@ -1,21 +1,21 @@
//// [staticMemberOfClassAndPublicMemberOfAnotherClassAssignment.ts]
interface A {
name();
prop();
}
class B {
public name() { }
public prop() { }
}
class C {
public static name() { }
public static prop() { }
}

var a: A = new B();
a = new C(); // error name is missing
a = B; // error name is missing
a = new C(); // error prop is missing
a = B; // error prop is missing
a = C;

var b: B = new C(); // error name is missing
b = B; // error name is missing
var b: B = new C(); // error prop is missing
b = B; // error prop is missing
b = C;
b = a;

Expand All @@ -29,21 +29,21 @@ c = a;
var B = (function () {
function B() {
}
B.prototype.name = function () { };
B.prototype.prop = function () { };
return B;
}());
var C = (function () {
function C() {
}
C.name = function () { };
C.prop = function () { };
return C;
}());
var a = new B();
a = new C(); // error name is missing
a = B; // error name is missing
a = new C(); // error prop is missing
a = B; // error prop is missing
a = C;
var b = new C(); // error name is missing
b = B; // error name is missing
var b = new C(); // error prop is missing
b = B; // error prop is missing
b = C;
b = a;
var c = new B();
Expand Down
@@ -0,0 +1,92 @@
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(4,12): error TS2699: Static property 'name' conflicts with built-in property 'Function.name' of constructor function 'StaticName'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(9,12): error TS2699: Static property 'name' conflicts with built-in property 'Function.name' of constructor function 'StaticNameFn'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(15,12): error TS2699: Static property 'length' conflicts with built-in property 'Function.length' of constructor function 'StaticLength'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(20,12): error TS2699: Static property 'length' conflicts with built-in property 'Function.length' of constructor function 'StaticLengthFn'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(26,12): error TS2699: Static property 'prototype' conflicts with built-in property 'Function.prototype' of constructor function 'StaticPrototype'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(31,12): error TS2300: Duplicate identifier 'prototype'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(31,12): error TS2699: Static property 'prototype' conflicts with built-in property 'Function.prototype' of constructor function 'StaticPrototypeFn'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(37,12): error TS2699: Static property 'caller' conflicts with built-in property 'Function.caller' of constructor function 'StaticCaller'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(42,12): error TS2699: Static property 'caller' conflicts with built-in property 'Function.caller' of constructor function 'StaticCallerFn'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(48,12): error TS2699: Static property 'arguments' conflicts with built-in property 'Function.arguments' of constructor function 'StaticArguments'.
tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts(53,12): error TS2699: Static property 'arguments' conflicts with built-in property 'Function.arguments' of constructor function 'StaticArgumentsFn'.


==== tests/cases/conformance/classes/propertyMemberDeclarations/staticPropertyNameConflictsEs5.ts (11 errors) ====

// static name
class StaticName {
static name: number; // error
~~~~
!!! error TS2699: Static property 'name' conflicts with built-in property 'Function.name' of constructor function 'StaticName'.
name: string; // ok
}

class StaticNameFn {
static name() {} // error
~~~~
!!! error TS2699: Static property 'name' conflicts with built-in property 'Function.name' of constructor function 'StaticNameFn'.
name() {} // ok
}


class StaticLength {
static length: number; // error
~~~~~~
!!! error TS2699: Static property 'length' conflicts with built-in property 'Function.length' of constructor function 'StaticLength'.
length: string; // ok
}

class StaticLengthFn {
static length() {} // error
~~~~~~
!!! error TS2699: Static property 'length' conflicts with built-in property 'Function.length' of constructor function 'StaticLengthFn'.
length() {} // ok
}


class StaticPrototype {
static prototype: number; // error
~~~~~~~~~
!!! error TS2699: Static property 'prototype' conflicts with built-in property 'Function.prototype' of constructor function 'StaticPrototype'.
prototype: string; // ok
}

class StaticPrototypeFn {
static prototype() {} // error
~~~~~~~~~
!!! error TS2300: Duplicate identifier 'prototype'.
~~~~~~~~~
!!! error TS2699: Static property 'prototype' conflicts with built-in property 'Function.prototype' of constructor function 'StaticPrototypeFn'.
prototype() {} // ok
}


class StaticCaller {
static caller: number; // error
~~~~~~
!!! error TS2699: Static property 'caller' conflicts with built-in property 'Function.caller' of constructor function 'StaticCaller'.
caller: string; // ok
}

class StaticCallerFn {
static caller() {} // error
~~~~~~
!!! error TS2699: Static property 'caller' conflicts with built-in property 'Function.caller' of constructor function 'StaticCallerFn'.
caller() {} // ok
}


class StaticArguments {
static arguments: number; // error
~~~~~~~~~
!!! error TS2699: Static property 'arguments' conflicts with built-in property 'Function.arguments' of constructor function 'StaticArguments'.
arguments: string; // ok
}

class StaticArgumentsFn {
static arguments() {} // error
~~~~~~~~~
!!! error TS2699: Static property 'arguments' conflicts with built-in property 'Function.arguments' of constructor function 'StaticArgumentsFn'.
arguments() {} // ok
}