Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
ql/javascript/ql/src/Declarations/IneffectiveParameterType.ql
ql/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql
ql/javascript/ql/src/Expressions/ExprHasNoEffect.ql
ql/javascript/ql/src/Expressions/MissingAwait.ql
ql/javascript/ql/src/LanguageFeatures/SpuriousArguments.ql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,50 +4,73 @@
<qhelp>
<overview>
<p>
In TypeScript the keywords <code>constructor</code> and <code>new</code> for
member declarations are used to declare constructors in classes and interfaces
respectively.
However, a member declaration with the name <code>new</code> in an interface
or <code>constructor</code> in a class, will declare an ordinary method named
<code>new</code> or <code>constructor</code> rather than a constructor.
Similarly, the keyword <code>function</code> is used to declare functions in
some contexts. However, using the name <code>function</code> for a class
or interface member declaration declares a method named <code>function</code>.
In TypeScript, certain keywords have special meanings for member declarations, and misusing them can create confusion:
</p>

<ul>
<li>In classes, use <code>constructor</code> rather than <code>new</code> to declare constructors. Using <code>new</code> within a class creates a method named "new" and not a constructor signature.</li>
<li>In interfaces, use <code>new</code> rather than <code>constructor</code> to declare constructor signatures. Using <code>constructor</code> within an interface creates a method named "constructor" and not a constructor signature.</li>
<li>Similarly, the keyword <code>function</code> is used to declare functions in some contexts. However, using the name <code>function</code> for a class or interface member declaration declares a method named "function".</li>
</ul>

<p>
When these keywords are misused, TypeScript will interpret them as regular method names rather than their intended special syntax, leading to code that may not work as expected.
</p>

</overview>
<recommendation>

<p>
Declare classes as classes and not as interfaces.
Use the keyword <code>constructor</code> to declare constructors in a class,
use the keyword <code>new</code> to declare constructors inside interfaces,
and don't use <code>function</code> when declaring a call signature in an
interface.
Consider following these guidelines for clearer code:
</p>

<ul>
<li>For classes, use <code>constructor</code> to declare constructors.</li>
<li>For interfaces, use <code>new</code> to declare constructor signatures (call signatures that create new instances).</li>
<li>Avoid accidentally creating methods named <code>function</code> by misusing the <code>function</code> keyword within class or interface declarations.</li>
</ul>

</recommendation>
<example>

<p>
The below example declares an interface <code>Point</code> with 2 fields
and a method called <code>constructor</code>. The interface does not declare
a class <code>Point</code> with a constructor, which was likely what the
developer meant to create.
The following examples show common mistakes when using these keywords:
</p>
<sample src="examples/SuspiciousMethodNameDeclaration.ts" />

<p>
The below example is a fixed version of the above, where the interface is
instead declared as a class, thereby describing the type the developer meant
in the first place.
This interface mistakenly uses <code>constructor</code>, which creates a method named "constructor" instead of a constructor signature:
</p>
<sample src="examples/SuspiciousMethodNameDeclaration.ts" />

<p>
Use <code>new</code> for constructor signatures in interfaces:
</p>
<sample src="examples/SuspiciousMethodNameDeclarationFixed.ts" />

<p>
This class mistakenly uses <code>new</code>, which creates a method named "new" instead of a constructor:
</p>
<sample src="examples/SuspiciousMethodNameDeclarationClass.ts" />

<p>
Use <code>constructor</code> for constructors in classes:
</p>
<sample src="examples/SuspiciousMethodNameDeclarationClassFixed.ts" />

<p>
This interface uses <code>function</code> as a method name, which declares a method named "function" rather than declaring a function:
</p>
<sample src="examples/SuspiciousMethodNameDeclarationFunction.ts" />

<p>
Use a descriptive method name instead:
</p>
<sample src="examples/SuspiciousMethodNameDeclarationFunctionFixed.ts" />

</example>
<references>

<li>TypeScript Handbook: <a href="https://www.typescriptlang.org/docs/handbook/2/classes.html#constructors">Classes - Constructors</a>.</li>
<li>TypeScript specification: <a href="https://github.com/microsoft/TypeScript/blob/30cb20434a6b117e007a4959b2a7c16489f86069/doc/spec-ARCHIVED.md#3.8.9">Constructor Type Literals</a>.</li>
<li>TypeScript specification: <a href="https://github.com/microsoft/TypeScript/blob/30cb20434a6b117e007a4959b2a7c16489f86069/doc/spec-ARCHIVED.md#8.3.1">Constructor Parameters</a>.</li>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
* @problem.severity warning
* @id js/suspicious-method-name-declaration
* @precision high
* @tags correctness
* @tags quality
* reliability
* correctness
* typescript
* methods
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
declare class Point {
// BAD: Using 'constructor' in an interface creates a method, not a constructor signature
interface Point {
x: number;
y: number;
constructor(x : number, y: number);
constructor(x: number, y: number); // This is just a method named "constructor"
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// BAD: Using 'new' in a class creates a method, not a constructor
class Point {
x: number;
y: number;
new(x: number, y: number) {}; // This is just a method named "new"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// GOOD: Using 'constructor' for constructors in classes
class Point {
x: number;
y: number;
constructor(x: number, y: number) { // This is a proper constructor
this.x = x;
this.y = y;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// GOOD: Using 'new' for constructor signatures in interfaces
interface Point {
x: number;
y: number;
new(x: number, y: number): Point; // This is a proper constructor signature
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// BAD: Using 'function' as a method name is confusing
interface Calculator {
function(a: number, b: number): number; // This is just a method named "function"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// GOOD: Using descriptive method names instead of 'function'
interface Calculator {
calculate(a: number, b: number): number; // Clear, descriptive method name
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: queryMetadata
---
* Added `reliability` tag to the `js/suspicious-method-name-declaration` query.
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,14 @@
| tst.ts:16:3:16:21 | function(): number; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:37:3:37:21 | function(): number; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:48:3:48:13 | new(): Quz; | The member name 'new' does not declare a constructor, but 'constructor' does in class declarations. |
| tst.ts:60:3:60:21 | function(): number; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:64:3:64:24 | constru ... number; | The member name 'constructor' does not declare a constructor in interfaces, but it does in classes. |
| tst.ts:74:3:74:30 | functio ... string; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:75:3:75:30 | functio ... number; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:76:3:76:24 | functio ... ): any; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:80:3:80:23 | abstrac ... : void; | The member name 'new' does not declare a constructor, but 'constructor' does in class declarations. |
| tst.ts:84:3:84:30 | abstrac ... number; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:93:5:93:21 | function(): void; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:98:3:98:21 | function(): number; | The member name 'function' does not declare a function, it declares a method named 'function'. |
| tst.ts:110:3:110:24 | constru ... number; | The member name 'constructor' does not declare a constructor in interfaces, but it does in classes. |
| tst.ts:116:3:116:24 | constru ... number; | The member name 'constructor' does not declare a constructor in interfaces, but it does in classes. |
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,70 @@ declare class Quz {

var bla = new Foo();
var blab = new Baz();


interface X {
constructor: () => string; // Just a property, not a method.
}

type A = {
function(): number; // $ Alert
};

type B = {
constructor(): number; // $ Alert
new(): number;
};

class StaticMethods {
static function(): void {}
static new(): void {}
}

interface Overloaded {
function(x: string): string; // $Alert
function(x: number): number; // $Alert
function(x: any): any; // $Alert
}

abstract class AbstractFoo {
abstract new(): void; // $Alert
}

abstract class AbstractFooFunction {
abstract function(): number; // $Alert
}

abstract class AbstractFooConstructor {
constructor(){}
}

declare module "some-module" {
interface ModuleInterface {
function(): void; // $Alert
}
}

type Intersection = {
function(): number; // $Alert
} & {
other(): string;
};

type Union = {
new(): number;
} | {
valid(): string;
};

type Union2 = {
constructor(): number; // $Alert
} | {
valid(): string;
};

type Intersection2 = {
constructor(): number; // $Alert
} & {
other(): string;
};