Skip to content

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

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

Merged
merged 5 commits into from
Jun 12, 2025

Conversation

Napalys
Copy link
Contributor

@Napalys Napalys commented Jun 12, 2025

This pull request adds the query js/template-syntax-in-string-literal to the Code Quality suite.

The autofix functionality yields good results.

The MRVA evaluation returned a small number of findings (15), all of which appear to be true positives. I am inclined to mark this query as @precision very-high

@Napalys Napalys added the no-change-note-required This PR does not need a change note label Jun 12, 2025
@github-actions github-actions bot added JS documentation and removed no-change-note-required This PR does not need a change note labels Jun 12, 2025
Copy link
Contributor

github-actions bot commented Jun 12, 2025

QHelp previews:

javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.qhelp

Suspicious method name declaration

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.

Recommendation

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.

Example

The following examples show common mistakes when using these keywords:

This interface mistakenly uses constructor, which creates a method named "constructor" instead of a constructor signature:

// BAD: Using 'constructor' in an interface creates a method, not a constructor signature
interface Point {
   x: number;
   y: number;
   constructor(x: number, y: number); // This is just a method named "constructor"
}

Use new for constructor signatures in interfaces:

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

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

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

Use constructor for constructors in classes:

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

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

// BAD: Using 'function' as a method name is confusing
interface Calculator {
   function(a: number, b: number): number; // This is just a method named "function"
}

Use a descriptive method name instead:

// GOOD: Using descriptive method names instead of 'function'
interface Calculator {
   calculate(a: number, b: number): number; // Clear, descriptive method name
}

References

@Napalys Napalys marked this pull request as ready for review June 12, 2025 08:28
@Copilot Copilot AI review requested due to automatic review settings June 12, 2025 08:28
@Napalys Napalys requested a review from a team as a code owner June 12, 2025 08:28
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@Napalys Napalys added the no-change-note-required This PR does not need a change note label Jun 12, 2025
@Napalys Napalys force-pushed the js/quality/suspicious_method_names branch from 04ac711 to 7b91a57 Compare June 12, 2025 10:19
@Napalys Napalys removed the no-change-note-required This PR does not need a change note label Jun 12, 2025
Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

Minor nit otherwise LGTM

….qhelp

Co-authored-by: Asger F <asgerf@github.com>
@Napalys Napalys merged commit 28ae396 into github:main Jun 12, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants