feat: Enhance symbol resolution by adding constructor support and improving test coverage

Author: lukecotter

lana/src/display/OpenFileInPackage.ts

@@ -24,6 +24,7 @@ export class OpenFileInPackage {
 
     const parts = symbolName.slice(0, symbolName.indexOf('('));
 
+    // findSymbol displays an error message if not found, so no need to duplicate here but it would probably be better here.
     const paths = await context.findSymbol(parts);
     if (!paths.length) {
       return;

lana/src/salesforce/ApexParser/ApexSymbolLocator.ts

@@ -8,7 +8,12 @@ import {
   CommonTokenStream,
 } from '@apexdevtools/apex-parser';
 import { CharStreams } from 'antlr4ts';
-import { ApexVisitor, type ApexMethodNode, type ApexNode } from './ApexVisitor';
+import {
+  ApexVisitor,
+  type ApexConstructorNode,
+  type ApexMethodNode,
+  type ApexNode,
+} from './ApexVisitor';
 
 export type SymbolLocation = {
   character: number;
@@ -27,7 +32,7 @@ export function parseApex(apexCode: string): ApexNode {
 }
 
 export function getMethodLine(rootNode: ApexNode, fullyQualifiedSymbol: string): SymbolLocation {
-  const result: SymbolLocation = { character: 0, line: 1, isExactMatch: true };
+  const result: SymbolLocation = { character: 0, line: 1, isExactMatch: false };
 
   if (fullyQualifiedSymbol.indexOf('(') === -1) {
     return result;
@@ -41,24 +46,23 @@ export function getMethodLine(rootNode: ApexNode, fullyQualifiedSymbol: string):
   // MyClass(args) - constuctor, MyClass.InnerClass(args) - constuctor
 
   // Find the Namespace of the supplied symbol, if there is one.
+  //  strip all whitespace to make comparisons easier
+  let symbolToFind = normalizeText(fullyQualifiedSymbol);
   const outerClassNode = rootNode.children?.[0];
-  let outerClassName = outerClassNode?.name ?? '';
-  const endNsIndex = fullyQualifiedSymbol.indexOf(outerClassName);
-  const namespace = endNsIndex > 0 ? fullyQualifiedSymbol.slice(0, endNsIndex - 1) : '';
+  let outerClassName = normalizeText(outerClassNode?.name ?? '');
+  const endNsIndex = symbolToFind.indexOf(outerClassName);
+  const namespace = endNsIndex > 0 ? symbolToFind.slice(0, endNsIndex - 1) : '';
   if (namespace) {
     outerClassName = namespace + '.' + outerClassName;
     // Remove the leading namespace as most likely the source code will not have it, for symbols in this file.
-    fullyQualifiedSymbol = fullyQualifiedSymbol.replace(namespace + '.', '');
+    symbolToFind = symbolToFind.replace(namespace + '.', '');
   }
 
-  // strip all whitespace to make comparisons easier
-  fullyQualifiedSymbol = fullyQualifiedSymbol.replaceAll(' ', '').toLowerCase();
-
   // This is the index of the first '(' which indicates method args or constructor args.
-  const methodArgsIndex = fullyQualifiedSymbol.indexOf('(');
+  const methodArgsIndex = symbolToFind.indexOf('(');
   // We can't tell the difference between InnerClass constructor and outer class method call.
   // As such className could either be the class name or the method name, we need to check.
-  const className = fullyQualifiedSymbol.slice(0, methodArgsIndex);
+  const className = symbolToFind.slice(0, methodArgsIndex);
   let currentRoot: ApexNode | undefined = rootNode;
   // Keep iterating until we find the last symbol that is a class.
   // The next symbol might be a method or might be invalid.
@@ -76,71 +80,94 @@ export function getMethodLine(rootNode: ApexNode, fullyQualifiedSymbol: string):
     result.character = currentRoot.idCharacter ?? 0;
   }
 
-  // TODO: enchance to find constructors as well as methods
-
   // This is the method name before the args list, this may actually be a class name though so we need to check.
   // e.g for MyClass.InnerClass(args) we get InnerClass(args) but is this a method of InnerClass constructor?
-  const qualifiedMethodName = fullyQualifiedSymbol.slice(className.lastIndexOf('.') + 1);
+  const qualifiedMethodName = symbolToFind.slice(className.lastIndexOf('.') + 1);
   if (qualifiedMethodName && currentRoot) {
-    const methodNode = findMethodNode(currentRoot, qualifiedMethodName, outerClassName);
-
+    let methodNode: ApexMethodNode | ApexConstructorNode | undefined = findMethodNode(
+      currentRoot,
+      qualifiedMethodName,
+      outerClassName,
+    );
     if (!methodNode) {
-      result.line = currentRoot.line ?? 1;
-      result.isExactMatch = false;
-      result.missingSymbol = qualifiedMethodName;
-    } else {
+      methodNode = findConstructorNode(currentRoot, qualifiedMethodName, outerClassName);
+    }
+
+    if (methodNode) {
       result.line = methodNode.line;
       result.character = methodNode.idCharacter;
+      result.isExactMatch = true;
+      return result;
     }
   }
 
+  result.line = currentRoot.line ?? 1;
+  result.isExactMatch = false;
+  // keep the original case for error messages.
+  result.missingSymbol = fullyQualifiedSymbol.slice(className.lastIndexOf('.') + 1);
   return result;
 }
 
 function findClassNode(root: ApexNode, symbol: string, namespace: string): ApexNode | undefined {
-  const classNode = root.children?.find(
-    (child) => child.name === symbol && child.nature === 'Class',
-  );
-  if (classNode) {
-    return classNode;
-  }
-
-  if (namespace) {
-    return root.children?.find(
-      (child) => child.name === symbol.replaceAll(namespace + '.', '') && child.nature === 'Class',
-    );
-  }
+  const symbolWithoutNamespace = symbol.replaceAll(namespace + '.', '');
+  return root.children?.find((child) => {
+    if (child.nature === 'Class') {
+      const normalizedChildName = normalizeText(child.name ?? '');
+      return normalizedChildName === symbol || normalizedChildName === symbolWithoutNamespace;
+    }
 
-  return undefined;
+    return false;
+  });
 }
 
 function findMethodNode(
   root: ApexNode,
   symbol: string,
   outerClassName: string,
 ): ApexMethodNode | undefined {
-  const [methodName, args = ''] = symbol.slice(0, -1).split('(');
-  let params = args;
-
-  const methodNode = root.children?.find(
-    (child) =>
-      child.name === methodName &&
-      child.nature === 'Method' &&
-      (params === undefined || (child as ApexMethodNode).params.toLowerCase() === params),
-  ) as ApexMethodNode;
-
-  if (methodNode) {
-    return methodNode;
-  }
+  const [methodName, params = ''] = symbol.slice(0, -1).split('(');
+  // Try again but with the class name removed from args list. args from the debug log are fully qualified but they are not necessarily in the file,
+  // as we only need to qualify for external types to the file.
+  // (MyClass.ObjectArg) vs (ObjectArg) where MyClass is current class.
+  const paramsWithoutClassName = params.replaceAll(outerClassName + '.', '');
+
+  return root.children?.find((child) => {
+    if (child.nature === 'Method' && normalizeText(child.name ?? '') === methodName) {
+      const methodChild = child as ApexMethodNode;
+      const methodParams = normalizeText(methodChild.params);
+      return (
+        params === undefined || methodParams === params || methodParams === paramsWithoutClassName
+      );
+    }
+    return false;
+  }) as ApexMethodNode;
+}
 
+function findConstructorNode(
+  root: ApexNode,
+  symbol: string,
+  outerClassName: string,
+): ApexConstructorNode | undefined {
+  const [constructorName, params = ''] = symbol.slice(0, -1).split('(');
   // Try again but with the class name removed from args list. args from the debug log are fully qualified but they are not necessarily in the file,
   // as we only need to qualify for external types to the file.
   // (MyClass.ObjectArg) vs (ObjectArg) where MyClass is current class.
-  params = params.replaceAll(outerClassName + '.', '');
-  return root.children?.find(
-    (child) =>
-      child.name === methodName &&
-      child.nature === 'Method' &&
-      (params === undefined || (child as ApexMethodNode).params.toLowerCase() === params),
-  ) as ApexMethodNode;
+  const paramsWithoutClassName = params.replaceAll(outerClassName + '.', '');
+
+  return root.children?.find((child) => {
+    if (child.nature === 'Constructor' && normalizeText(child.name ?? '') === constructorName) {
+      const constructorChild = child as ApexConstructorNode;
+      const constructorParams = normalizeText(constructorChild.params);
+      return (
+        params === undefined ||
+        constructorParams === params ||
+        constructorParams === paramsWithoutClassName
+      );
+    }
+    return false;
+  }) as ApexConstructorNode;
+}
+
+function normalizeText(text: string): string {
+  return text?.replaceAll(' ', '').toLowerCase();
 }

lana/src/salesforce/ApexParser/ApexVisitor.ts

@@ -4,12 +4,13 @@
 import type {
   ApexParserVisitor,
   ClassDeclarationContext,
+  ConstructorDeclarationContext,
   FormalParametersContext,
   MethodDeclarationContext,
 } from '@apexdevtools/apex-parser';
 import type { ErrorNode, ParseTree, RuleNode, TerminalNode } from 'antlr4ts/tree';
 
-type ApexNature = 'Class' | 'Method';
+type ApexNature = 'Constructor' | 'Class' | 'Method';
 
 /**
  * Represents a node in the Apex syntax tree.
@@ -18,7 +19,7 @@ type ApexNature = 'Class' | 'Method';
 export interface ApexNode {
   /** The type of Apex construct (Class or Method) */
   nature?: ApexNature;
-  /** The name of the class or method, in lower case */
+  /** The name of the class or method */
   name?: string;
   /** Child nodes (nested classes or methods) */
   children?: ApexNode[];
@@ -41,13 +42,9 @@ export interface ApexClassNode extends ApexNode {
   idCharacter: number;
 }
 
-/**
- * Represents a method declaration node in the Apex syntax tree.
- * All properties are required (non-optional) to ensure complete method metadata.
- */
-export interface ApexMethodNode extends ApexNode {
+export interface ApexParamNode extends ApexNode {
   /** Indicates this node represents a method declaration */
-  nature: 'Method';
+  nature: 'Method' | 'Constructor';
   /** Comma-separated list of parameter types for the method */
   params: string;
   /** Line number where the method is declared */
@@ -56,6 +53,20 @@ export interface ApexMethodNode extends ApexNode {
   idCharacter: number;
 }
 
+/**
+ * Represents a method declaration node in the Apex syntax tree.
+ * All properties are required (non-optional) to ensure complete method metadata.
+ */
+export interface ApexMethodNode extends ApexParamNode {
+  /** Indicates this node represents a method declaration */
+  nature: 'Method';
+}
+
+export interface ApexConstructorNode extends ApexParamNode {
+  /** Indicates this node represents a method declaration */
+  nature: 'Constructor';
+}
+
 type VisitableApex = ParseTree & {
   accept<Result>(visitor: ApexParserVisitor<Result>): Result;
 };
@@ -87,24 +98,39 @@ export class ApexVisitor implements ApexParserVisitor<ApexNode> {
 
     return {
       nature: 'Class',
-      name: ident.text.toLowerCase(),
+      name: ident.text ?? '',
       children: ctx.children?.length ? this.visitChildren(ctx).children : [],
       line: start.line,
       idCharacter: ident.start.charPositionInLine ?? 0,
     };
   }
 
+  visitConstructorDeclaration(ctx: ConstructorDeclarationContext): ApexConstructorNode {
+    const { start } = ctx;
+    const idContexts = ctx.qualifiedName().id();
+    const constructorName = idContexts[idContexts.length - 1];
+
+    return {
+      nature: 'Constructor',
+      name: constructorName?.text ?? '',
+      children: ctx.children?.length ? this.visitChildren(ctx).children : [],
+      params: this.getParameters(ctx.formalParameters()),
+      line: start.line,
+      idCharacter: start.charPositionInLine ?? 0,
+    };
+  }
+
   visitMethodDeclaration(ctx: MethodDeclarationContext): ApexMethodNode {
     const { start } = ctx;
     const ident = ctx.id();
 
     return {
       nature: 'Method',
-      name: ident.text.toLowerCase(),
+      name: ident.text ?? '',
       children: ctx.children?.length ? this.visitChildren(ctx).children : [],
       params: this.getParameters(ctx.formalParameters()),
       line: start.line,
-      idCharacter: ident.start.charPositionInLine,
+      idCharacter: ident.start.charPositionInLine ?? 0,
     };
   }
 
@@ -118,11 +144,7 @@ export class ApexVisitor implements ApexParserVisitor<ApexNode> {
 
   private getParameters(ctx: FormalParametersContext): string {
     const paramsList = ctx.formalParameterList()?.formalParameter();
-    return (
-      paramsList
-        ?.map((param) => param.typeRef().text.replaceAll(' ', '').toLowerCase())
-        .join(',') ?? ''
-    );
+    return paramsList?.map((param) => param.typeRef().text).join(',') ?? '';
   }
 
   private forNode(node: ApexNode, anonHandler: (n: ApexNode) => void) {