From 41f4236b86e54c64b3c7c570cd85157d4ffb4c66 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 11 Jun 2025 14:01:57 +0200 Subject: [PATCH 1/5] JS: expanded `suspicious-method-name-declaration` test suite --- .../SuspiciousMethodNameDeclaration.expected | 11 +++ .../SuspiciousMethodNameDeclaration/tst.ts | 67 +++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/javascript/ql/test/query-tests/Declarations/SuspiciousMethodNameDeclaration/SuspiciousMethodNameDeclaration.expected b/javascript/ql/test/query-tests/Declarations/SuspiciousMethodNameDeclaration/SuspiciousMethodNameDeclaration.expected index 3c827d16fce6..ee80f65d1adf 100644 --- a/javascript/ql/test/query-tests/Declarations/SuspiciousMethodNameDeclaration/SuspiciousMethodNameDeclaration.expected +++ b/javascript/ql/test/query-tests/Declarations/SuspiciousMethodNameDeclaration/SuspiciousMethodNameDeclaration.expected @@ -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. | diff --git a/javascript/ql/test/query-tests/Declarations/SuspiciousMethodNameDeclaration/tst.ts b/javascript/ql/test/query-tests/Declarations/SuspiciousMethodNameDeclaration/tst.ts index e958681f9352..12a6087b3a33 100644 --- a/javascript/ql/test/query-tests/Declarations/SuspiciousMethodNameDeclaration/tst.ts +++ b/javascript/ql/test/query-tests/Declarations/SuspiciousMethodNameDeclaration/tst.ts @@ -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; +}; From 60e3b0c8e737dd73335335b671e16dd90568b888 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 11 Jun 2025 16:54:15 +0200 Subject: [PATCH 2/5] JS: Update `qhelp` and added more examples. --- .../SuspiciousMethodNameDeclaration.qhelp | 67 +++++++++++++------ .../SuspiciousMethodNameDeclaration.ts | 6 +- .../SuspiciousMethodNameDeclarationClass.ts | 6 ++ ...spiciousMethodNameDeclarationClassFixed.ts | 9 +++ .../SuspiciousMethodNameDeclarationFixed.ts | 2 + ...SuspiciousMethodNameDeclarationFunction.ts | 4 ++ ...ciousMethodNameDeclarationFunctionFixed.ts | 4 ++ 7 files changed, 73 insertions(+), 25 deletions(-) create mode 100644 javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationClass.ts create mode 100644 javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationClassFixed.ts create mode 100644 javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFunction.ts create mode 100644 javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFunctionFixed.ts diff --git a/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp b/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp index e59f290ae6df..ea761a5672e2 100644 --- a/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp +++ b/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp @@ -4,50 +4,73 @@

-In TypeScript the keywords constructor and new for -member declarations are used to declare constructors in classes and interfaces -respectively. -However, a member declaration with the name new in an interface -or constructor in a class, will declare an ordinary method named -new or constructor rather than a constructor. -Similarly, the keyword function is used to declare functions in -some contexts. However, using the name function for a class -or interface member declaration declares a method named function. +In TypeScript, certain keywords have special meanings for member declarations, and misusing them can create confusion: +

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

+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.

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

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

-The below example declares an interface Point with 2 fields -and a method called constructor. The interface does not declare -a class Point with a constructor, which was likely what the -developer meant to create. +The following examples show common mistakes when using these keywords:

-

-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 constructor, which creates a method named "constructor" instead of a constructor signature:

+ +

+Use new for constructor signatures in interfaces: +

+

+This class mistakenly uses new, which creates a method named "new" instead of a constructor: +

+ + +

+Use constructor for constructors in classes: +

+ + +

+This interface uses function as a method name, which declares a method named "function" rather than declaring a function: +

+ + +

+Use a descriptive method name instead: +

+ + +
  • TypeScript Handbook: Classes - Constructors.
  • TypeScript specification: Constructor Type Literals.
  • TypeScript specification: Constructor Parameters.
    • diff --git a/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclaration.ts b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclaration.ts index b7b925aa15a9..6558267cef4b 100644 --- a/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclaration.ts +++ b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclaration.ts @@ -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" } - diff --git a/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationClass.ts b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationClass.ts new file mode 100644 index 000000000000..2a078c8b468b --- /dev/null +++ b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationClass.ts @@ -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" +} diff --git a/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationClassFixed.ts b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationClassFixed.ts new file mode 100644 index 000000000000..0e072a4e0316 --- /dev/null +++ b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationClassFixed.ts @@ -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; + } +} diff --git a/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFixed.ts b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFixed.ts index 850038f4dbfe..d5dacdbb9515 100644 --- a/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFixed.ts +++ b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFixed.ts @@ -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 } diff --git a/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFunction.ts b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFunction.ts new file mode 100644 index 000000000000..37749d4ff85b --- /dev/null +++ b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFunction.ts @@ -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" +} diff --git a/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFunctionFixed.ts b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFunctionFixed.ts new file mode 100644 index 000000000000..a5b3b747faa4 --- /dev/null +++ b/javascript/ql/src/Declarations/examples/SuspiciousMethodNameDeclarationFunctionFixed.ts @@ -0,0 +1,4 @@ +// GOOD: Using descriptive method names instead of 'function' +interface Calculator { + calculate(a: number, b: number): number; // Clear, descriptive method name +} From c5a1421405957cf858d7f4506e40246596da82fa Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 11 Jun 2025 16:58:28 +0200 Subject: [PATCH 3/5] JS: promote `suspicious-method-name-declaration` to quality query. --- .../query-suite/javascript-code-quality.qls.expected | 1 + .../ql/src/Declarations/SuspiciousMethodNameDeclaration.ql | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected index 451a8b4bf273..e5d496d5e9cb 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected @@ -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 diff --git a/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql b/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql index c185e5c4d04e..fafa234f5f78 100644 --- a/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql +++ b/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql @@ -6,7 +6,9 @@ * @problem.severity warning * @id js/suspicious-method-name-declaration * @precision high - * @tags correctness + * @tags quality + * reliability + * correctness * typescript * methods */ From 7b91a57eb15f4b6d64e7a2cea5ba676225d957c8 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 12 Jun 2025 12:18:48 +0200 Subject: [PATCH 4/5] JS: add change note. --- .../ql/src/change-notes/2025-06-12-suspicious-method-name.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/src/change-notes/2025-06-12-suspicious-method-name.md diff --git a/javascript/ql/src/change-notes/2025-06-12-suspicious-method-name.md b/javascript/ql/src/change-notes/2025-06-12-suspicious-method-name.md new file mode 100644 index 000000000000..dfee27ffdd33 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-06-12-suspicious-method-name.md @@ -0,0 +1,4 @@ +--- +category: queryMetadata +--- +* Added `reliability` tag to the `js/suspicious-method-name-declaration` query. From e6d26912e066b4cfff9f126177f2525546acdbe8 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 12 Jun 2025 13:10:27 +0200 Subject: [PATCH 5/5] Update javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp Co-authored-by: Asger F --- .../ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp b/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp index ea761a5672e2..652d8b544d0f 100644 --- a/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp +++ b/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp @@ -10,7 +10,7 @@ In TypeScript, certain keywords have special meanings for member declarations, a
    • In classes, use constructor rather than new to declare constructors. Using new within a class creates a method named "new" and not a constructor signature.
    • In interfaces, use new rather than constructor to declare constructor signatures. Using constructor within an interface creates a method named "constructor" and not a constructor signature.
      • -
    • Similarly, the keyword function is used to declare functions in some contexts. However, using the name function for a class or interface member declaration declares a method named function.
      • +
    • Similarly, the keyword function is used to declare functions in some contexts. However, using the name function for a class or interface member declaration declares a method named "function".