From 578d85223edfb12ea85bd02d9bf4357c48e6fd46 Mon Sep 17 00:00:00 2001
From: Mark Wiemer <7833360+mark-wiemer@users.noreply.github.com>
Date: Sat, 2 Nov 2024 06:53:50 -0700
Subject: [PATCH] Improve included method detection (#554)

---
 .vscode/dev.code-snippets               |  4 +-
 ahk2                                    |  2 +-
 changelog.md                            |  6 +++
 src/common/codeUtil.ts                  |  5 +-
 src/common/out.ts                       |  5 ++
 src/parser/parser.ts                    | 17 ++++--
 src/providers/defProvider.ts            | 67 ++++++++++++------------
 src/providers/defProvider.utils.test.ts | 57 ++++++++++++++++++++
 src/providers/defProvider.utils.ts      | 69 +++++++++++++++++++++++++
 9 files changed, 190 insertions(+), 42 deletions(-)
 create mode 100644 src/providers/defProvider.utils.test.ts
 create mode 100644 src/providers/defProvider.utils.ts

diff --git a/.vscode/dev.code-snippets b/.vscode/dev.code-snippets
index 8f5f4a8a..a62abe73 100644
--- a/.vscode/dev.code-snippets
+++ b/.vscode/dev.code-snippets
@@ -21,12 +21,12 @@
         "scope": "javascript,typescript",
         "prefix": "suite",
         "body": [
-            "suite('$1', () => {",
+            "suite($1.name, () => {",
             "    const tests: [",
             "        name: string,",
             "        args: Parameters<typeof $1>,",
             "        expected: ReturnType<typeof $1>,",
-            "    ][] = [$0];",
+            "    ][] = [[$0]];",
 
             "    tests.forEach(([name, args, expected]) =>",
             "        test(name, () => assert.strictEqual($1(...args), expected)),",
diff --git a/ahk2 b/ahk2
index c05e25ed..22d94d87 160000
--- a/ahk2
+++ b/ahk2
@@ -1 +1 @@
-Subproject commit c05e25ede71fb57a15c9e1fa18606068a2a8a88e
+Subproject commit 22d94d87c46270b6729f18ef3350a7605c700842
diff --git a/changelog.md b/changelog.md
index 71c3cc95..428e6dee 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## 6.4.0 - unreleased
+
+### New features
+
+-   Hovering over a filename in an `#include` directive now provides a link to that document in your IDE
+
 ## 6.3.0 - 2024-10-19 🕳️
 
 ### New features
diff --git a/src/common/codeUtil.ts b/src/common/codeUtil.ts
index d4f86949..970f4d32 100644
--- a/src/common/codeUtil.ts
+++ b/src/common/codeUtil.ts
@@ -20,10 +20,10 @@ export class CodeUtil {
     }
 
     /**
-     * Concats an array and an item or array of items. Impure, @see array is modified
+     * Concats an array and an item or array of items. Impure, `array` is modified
      * @param array The initial array
      * @param items Either an item to add to the end of the array,
-     * or another array to concat to the end of @see array
+     * or another array to concat to the end of `array`
      */
     public static join(array: unknown[], items: unknown) {
         if (!array || !items) {
@@ -54,7 +54,6 @@ export class CodeUtil {
 }
 
 /** Whether the current active text editor is for an AHK v1 file */
-// todo use LSP for all v2 functionality
 export const isV1 = (): boolean =>
     vscode.window.activeTextEditor?.document.languageId === LanguageId.ahk1;
 
diff --git a/src/common/out.ts b/src/common/out.ts
index 56d185ea..a6b7aabf 100644
--- a/src/common/out.ts
+++ b/src/common/out.ts
@@ -4,6 +4,10 @@ import * as vscode from 'vscode';
 export class Out {
     private static outputChannel: vscode.OutputChannel;
 
+    /**
+     * Logs the given value without focusing the output view.
+     * Prepends all logs with `new Date().toISOString()`.
+     */
     public static debug(value: Error | string) {
         Out.log(value, false);
     }
@@ -12,6 +16,7 @@ export class Out {
      * Logs the given value. Traces errors to console before logging.
      * Prepends all logs with `new Date().toISOString()`.
      * @param value The value to log
+     * @param focus whether to focus the output view. Defaults to true.
      */
     public static log(value: Error | string, focus = true) {
         if (value instanceof Error) {
diff --git a/src/parser/parser.ts b/src/parser/parser.ts
index 7c1a17d8..873b3d01 100644
--- a/src/parser/parser.ts
+++ b/src/parser/parser.ts
@@ -53,6 +53,7 @@ export class Parser {
         document: vscode.TextDocument,
         options: BuildScriptOptions = {},
     ): Promise<Script> {
+        const funcName = 'buildScript';
         if (options.usingCache && documentCache.get(document.uri.path)) {
             return documentCache.get(document.uri.path);
         }
@@ -135,22 +136,32 @@ export class Parser {
             }
         }
         const script: Script = { methods, labels, refs, variables, blocks };
+        Out.debug(`${funcName} document.uri.path: ${document.uri.path}`);
+        Out.debug(`${funcName} script: ${JSON.stringify(script)}`);
         documentCache.set(document.uri.path, script);
         return script;
     }
 
+    /**
+     * Finds the best reference to the method.
+     * If a method of this name exists in the current file, returns that method.
+     * Otherwise, searches through document cache to find the matching method.
+     * Matches are not case-sensitive and only need to match method name.
+     */
     public static async getMethodByName(
         document: vscode.TextDocument,
         name: string,
+        localCache = documentCache,
     ) {
         name = name.toLowerCase();
-        for (const method of documentCache.get(document.uri.path).methods) {
+        for (const method of localCache.get(document.uri.path).methods) {
             if (method.name.toLowerCase() === name) {
                 return method;
             }
         }
-        for (const filePath of documentCache.keys()) {
-            for (const method of documentCache.get(filePath).methods) {
+        // todo this should prioritize included files first.
+        for (const filePath of localCache.keys()) {
+            for (const method of localCache.get(filePath).methods) {
                 if (method.name.toLowerCase() === name) {
                     return method;
                 }
diff --git a/src/providers/defProvider.ts b/src/providers/defProvider.ts
index 5fe5a558..1af0bef9 100644
--- a/src/providers/defProvider.ts
+++ b/src/providers/defProvider.ts
@@ -1,13 +1,15 @@
 import * as vscode from 'vscode';
 import { Parser } from '../parser/parser';
-import { existsSync } from 'fs';
+import { resolveIncludedPath } from './defProvider.utils';
+import { Out } from 'src/common/out';
+import { stat } from 'fs/promises';
 
 export class DefProvider implements vscode.DefinitionProvider {
     public async provideDefinition(
         document: vscode.TextDocument,
         position: vscode.Position,
     ): Promise<vscode.Location | vscode.Location[] | vscode.LocationLink[]> {
-        const fileLink = await this.tryGetFileLink(document, position);
+        const fileLink = await tryGetFileLink(document, position);
         if (fileLink) {
             return fileLink;
         }
@@ -84,36 +86,35 @@ export class DefProvider implements vscode.DefinitionProvider {
 
         return null;
     }
+}
 
-    public async tryGetFileLink(
-        document: vscode.TextDocument,
-        position: vscode.Position,
-        workFolder?: string,
-    ): Promise<vscode.Location> | undefined {
-        const { text } = document.lineAt(position.line);
-        const includeMatch = text.match(/(?<=#include).+?\.(ahk|ext)\b/i);
-        if (!includeMatch) {
-            return undefined;
-        }
-        const parent = workFolder
-            ? workFolder
-            : document.uri.path.substr(0, document.uri.path.lastIndexOf('/'));
-        const targetPath = vscode.Uri.file(
-            includeMatch[0]
-                .trim()
-                .replace(/(%A_ScriptDir%|%A_WorkingDir%)/, parent)
-                .replace(/(%A_LineFile%)/, document.uri.path),
-        );
-        if (existsSync(targetPath.fsPath)) {
-            return new vscode.Location(targetPath, new vscode.Position(0, 0));
-        } else if (workFolder) {
-            return this.tryGetFileLink(
-                document,
-                position,
-                vscode.workspace.workspaceFolders?.[0].uri.fsPath,
-            );
-        } else {
-            return undefined;
-        }
-    }
+//* Utilities requiring the vscode API
+
+/**
+ * If the position is on an `#Include` line
+ * and the included path is an existing file,
+ * returns a Location at the beginning of the included file.
+ *
+ * Otherwise returns undefined.
+ *
+ ** Currently assumes the working directory is the script path and
+ * does not respect previous `#include dir` directives
+ */
+async function tryGetFileLink(
+    document: vscode.TextDocument,
+    position: vscode.Position,
+): Promise<vscode.Location> | undefined {
+    /** @example '/c:/path/to/file.ahk' */
+    const docPath = document.uri.path;
+    const { text } = document.lineAt(position.line);
+    /** @example 'c:/path/to/included.ahk' */
+    const resolvedPath = resolveIncludedPath(docPath, text);
+    Out.debug(`resolvedPath: ${resolvedPath}`);
+    const fsStat = await stat(resolvedPath);
+    return fsStat.isFile()
+        ? new vscode.Location(
+              vscode.Uri.file(resolvedPath),
+              new vscode.Position(0, 0),
+          )
+        : undefined;
 }
diff --git a/src/providers/defProvider.utils.test.ts b/src/providers/defProvider.utils.test.ts
new file mode 100644
index 00000000..44215954
--- /dev/null
+++ b/src/providers/defProvider.utils.test.ts
@@ -0,0 +1,57 @@
+import { suite, test } from 'mocha';
+import assert from 'assert';
+import { getIncludedPath, resolveIncludedPath } from './defProvider.utils';
+
+suite(getIncludedPath.name, () => {
+    const tests: [
+        name: string,
+        args: Parameters<typeof getIncludedPath>,
+        expected: ReturnType<typeof getIncludedPath>,
+    ][] = [
+        ['comma', ['#include , a b.ahk'], 'a b.ahk'],
+        ['no hash', ['include , a b.ahk'], undefined],
+        [
+            'no comma nor extra space',
+            ['#include path/to/file.ahk'],
+            'path/to/file.ahk',
+        ],
+        ['ah1', ['#include a.ah1'], 'a.ah1'],
+        ['ahk1', ['#include a.ahk1'], 'a.ahk1'],
+        ['ext', ['#include a.ext'], 'a.ext'],
+        ['preceding whitespace', ['   #include a.ahk'], 'a.ahk'],
+        ['directory', ['#include a'], 'a'],
+        ['non-whitespace preceding char', [';#include a'], undefined],
+        ['escaped `;` with whitespace', ['#include a `;b.ahk'], 'a `;b.ahk'],
+        ['escaped `;` without whitespace', ['#include a`;b.ahk'], 'a`;b.ahk'],
+        ['unescaped `;` without whitespace', ['#include a;b.ahk'], 'a;b.ahk'],
+        ['unescaped `;` with whitespace', ['#include a ;b.ahk'], 'a'],
+        ['unescaped valid `%`', ['#include %A_ScriptDir%'], '%A_ScriptDir%'],
+        ['unescaped `<` and `>`', ['#include <foo>'], '<foo>'],
+    ];
+    tests.forEach(([name, args, expected]) =>
+        test(name, () =>
+            assert.strictEqual(getIncludedPath(...args), expected),
+        ),
+    );
+});
+
+suite(resolveIncludedPath.name, () => {
+    const tests: [
+        name: string,
+        args: Parameters<typeof resolveIncludedPath>,
+        expected: ReturnType<typeof resolveIncludedPath>,
+    ][] = [
+        ['relative file', ['/c:/main.ahk', 'a.ahk'], 'c:\\a.ahk'],
+        ['absolute file', ['/c:/users/main.ahk', 'd:/b.ahk'], 'd:\\b.ahk'],
+        ['with single dot', ['/c:/main.ahk', './c.ahk'], 'c:\\c.ahk'],
+        ['with double dot', ['/c:/users/main.ahk', '../d.ahk'], 'c:\\d.ahk'],
+    ];
+    tests.forEach(([name, args, expected]) =>
+        test(name, () =>
+            assert.strictEqual(
+                resolveIncludedPath(args[0], `#include ${args[1]}`),
+                expected,
+            ),
+        ),
+    );
+});
diff --git a/src/providers/defProvider.utils.ts b/src/providers/defProvider.utils.ts
new file mode 100644
index 00000000..f657caaa
--- /dev/null
+++ b/src/providers/defProvider.utils.ts
@@ -0,0 +1,69 @@
+//* Utilities not requiring the vscode API
+
+import { isAbsolute, join, normalize } from 'path';
+
+/**
+ ** Returns the string representing the included path after the `#include`.
+ ** Only works for actual `#include` directives, not comments or strings containing `#include`.
+ ** Does not resolve or normalize the included path.
+ * @example
+ * getIncludedPath('#include , a b.ahk') === 'a b.ahk'
+ * getIncludedPath('  #include path/to/file.ahk') === 'path/to/file.ahk'
+ * getIncludedPath('include , a b.ahk') === undefined // no `#`
+ * getIncludedPath('; #include , a b.ahk') === undefined
+ * getIncludedPath('x := % "#include , a b.ahk"') === undefined
+ * getIncludedPath('#include a') === 'a'
+ * getIncludedPath('#include %A_ScriptDir%') === '%A_ScriptDir%'
+ * getIncludedPath('#include <myLib>') === '<myLib>'
+ * getIncludedPath('#include semi-colon ;and-more.ahk') === 'semi-colon'
+ * getIncludedPath('#include semi-colon`;and-more.ahk') === 'semi-colon`;and-more.ahk'
+ */
+export const getIncludedPath = (ahkLine: string): string | undefined =>
+    ahkLine.match(/^\s*#include\s*,?\s*(.+?)( ;.*)?$/i)?.[1];
+
+const normalizeIncludedPath = (
+    includedPath: string,
+    basePath: string,
+    parentGoodPath: string,
+): string =>
+    normalize(
+        includedPath
+            .trim()
+            .replace(/`;/g, ';') // only semi-colons are escaped
+            .replace(/(%A_ScriptDir%|%A_WorkingDir%)/, parentGoodPath)
+            .replace(/(%A_LineFile%)/, basePath),
+    );
+
+/**
+ * Returns the absolute, normalized path included by a #include directive.
+ * Does not check if that path is a to a folder, or if that path exists.
+ *
+ * @param basePath
+ * The path to include from, usually the script's path.
+ *
+ * This may be a different path if the including script has a preceding `#include dir`
+ *
+ * @param includedPath The path that's included in the `#include` directive
+ */
+export const resolveIncludedPath = (
+    /**
+     * The path of the current script, namely `vscode.document.uri.path`:
+     * @example '/c:/path/to/file.ahk'
+     */
+    basePath: string,
+    /** Line of text from the including script. */
+    ahkLine: string,
+): string | undefined => {
+    const includedPath = getIncludedPath(ahkLine);
+    /** @example 'c:/path/to' */
+    const parentGoodPath = basePath.substring(1, basePath.lastIndexOf('/'));
+    const normalizedPath = normalizeIncludedPath(
+        includedPath,
+        basePath,
+        parentGoodPath,
+    );
+    const absolutePath = isAbsolute(includedPath)
+        ? normalize(includedPath)
+        : join(parentGoodPath, normalizedPath);
+    return absolutePath;
+};