Merge pull request #19741 from Napalys/js/quality/suspicious_method_names

JS: Promote `js/suspicious-method-name-declaration` to the Code Quality suite.

Author: Napalys

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

javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp

@@ -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>
 

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
  */

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"
 }
-

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"
+}

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;
+   }
+}

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
 }

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"
+}

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

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.