refactor(server): decouple analysis from visit logic and simplify definition provider

server/src/analyzer.ts

@@ -0,0 +1,39 @@
+import { Node, parseTree, ParseError } from 'jsonc-parser';
+import { SymbolTable, JRefSymbol } from './symbolTable';
+import { visit, VisitCallback } from './visitor';
+
+export interface DocumentAnalysis {
+  symbols: SymbolTable;
+  errors: ParseError[];
+}
+
+export function analyze(content: string): DocumentAnalysis {
+  const errors: ParseError[] = [];
+  const ast = parseTree(content, errors);
+  const symbols: SymbolTable = new Map();
+
+  visit(ast, (node, path) => {
+    const symbol = createJRefSymbol(node, path);
+    symbols.set(path, symbol);
+  });
+
+  return { symbols, errors };
+}
+
+function createJRefSymbol(node: Node, pointer: string): JRefSymbol {
+  const isReference = isReferenceValue(node);
+  return {
+    pointer,
+    node,
+    isReference,
+    refersTo: isReference ? node.value : null,
+  };
+}
+
+function isReferenceValue(node: Node): boolean {
+  return (
+    node.type === 'string' &&
+    node.parent?.type === 'property' &&
+    node.parent.children?.[0].value === '$ref'
+  );
+}

server/src/providers/definition.ts

@@ -1,11 +1,12 @@
 import path from 'path';
 import { URI } from 'vscode-uri';
+import * as fs from 'fs';
 
 import { DefinitionParams, DefinitionLink, Range } from 'vscode-languageserver/node';
 import { TextDocument } from 'vscode-languageserver-textdocument';
-import { JRefSymbol } from '../visitor';
-import { analyze, ServerContext } from '../utils';
-import * as fs from 'fs';
+import { JRefSymbol, SymbolTable } from '../symbolTable';
+import { ServerContext } from '../utils';
+import { analyze } from '../analyzer';
 
 const defaultTargetRange: Range = {
   start: { line: 0, character: 0 },
@@ -37,50 +38,72 @@ function createDefinitionLink(
   document: TextDocument,
   ref: JRefSymbol,
   context: ServerContext,
-): DefinitionLink[] | undefined {
-  const { documents, documentSymbols } = context;
-  const refValueNode = ref.node;
-  const targetPath = refValueNode.value;
-  const uri = URI.parse(targetPath);
-  const currentDir = path.dirname(URI.parse(document.uri).fsPath);
-  const absolutePath = path.resolve(currentDir, uri.path.slice(1));
-  const targetUri = URI.file(absolutePath).toString();
-  const targetRange = getTargetRange(targetUri);
-
-  function getTargetRange(targetUri: string): Range {
-    let targetDocument = documents.get(targetUri);
-    if (!targetDocument) {
-      try {
-        const filePath = URI.parse(targetUri).fsPath;
-        const content = fs.readFileSync(filePath, 'utf8');
-        targetDocument = TextDocument.create(targetUri, 'jref', 1, content);
-      } catch (e) {
-        return defaultTargetRange;
-      }
-    }
-    let targetSymbolTable = documentSymbols.get(targetDocument);
-    if (!targetSymbolTable) {
-      const { symbols } = analyze(targetDocument.getText());
-      documentSymbols.set(targetDocument, symbols);
-      targetSymbolTable = symbols;
-    }
-    const targetSymbol = targetSymbolTable?.get(uri.fragment);
-    if (!targetSymbol) return defaultTargetRange;
-    return {
-      start: targetDocument.positionAt(targetSymbol.node.offset),
-      end: targetDocument.positionAt(targetSymbol.node.offset + targetSymbol.node.length),
-    };
-  }
+): DefinitionLink[] {
+  const { targetUri, fragment } = resolveTargetUriAndFragment(document.uri, ref.node.value);
+
+  const targetRange = findTargetRange(targetUri, fragment, context);
 
   return [
     {
-      originSelectionRange: {
-        start: document.positionAt(refValueNode.offset + 1), // +1 to skip the opening quote
-        end: document.positionAt(refValueNode.offset + refValueNode.length - 1), // -1 to skip the closing quote
-      },
-      targetUri: URI.file(absolutePath).toString(),
-      targetRange: targetRange,
+      originSelectionRange: createOriginSelectionRange(document, ref),
+      targetUri,
+      targetRange,
       targetSelectionRange: targetRange,
     },
   ];
 }
+
+function resolveTargetUriAndFragment(documentUri: string, targetPath: string) {
+  const uri = URI.parse(targetPath);
+  const currentDir = path.dirname(URI.parse(documentUri).fsPath);
+  const absolutePath = path.resolve(currentDir, uri.path.slice(1));
+  const targetUri = URI.file(absolutePath).toString();
+  return { targetUri, fragment: uri.fragment };
+}
+
+function getOrLoadDocument(targetUri: string, context: ServerContext): TextDocument | undefined {
+  const { documents } = context;
+  const targetDocument = documents.get(targetUri);
+  if (targetDocument) return targetDocument;
+
+  try {
+    const filePath = URI.parse(targetUri).fsPath;
+    const content = fs.readFileSync(filePath, 'utf8');
+    return TextDocument.create(targetUri, 'jref', 1, content);
+  } catch (e) {
+    return;
+  }
+}
+
+function getOrAnalyzeSymbols(targetDocument: TextDocument, context: ServerContext): SymbolTable {
+  const { documentSymbols } = context;
+  let targetSymbolTable = documentSymbols.get(targetDocument);
+  if (targetSymbolTable) return targetSymbolTable;
+
+  const { symbols } = analyze(targetDocument.getText());
+  documentSymbols.set(targetDocument, symbols);
+  return symbols;
+}
+
+function findTargetRange(targetUri: string, fragment: string, context: ServerContext): Range {
+  const targetDocument = getOrLoadDocument(targetUri, context);
+  if (!targetDocument) return defaultTargetRange;
+
+  const targetSymbolTable = getOrAnalyzeSymbols(targetDocument, context);
+  const targetSymbol = targetSymbolTable.get(fragment || ''); // Default to empty string if no fragment
+
+  if (!targetSymbol) return defaultTargetRange;
+
+  return {
+    start: targetDocument.positionAt(targetSymbol.node.offset),
+    end: targetDocument.positionAt(targetSymbol.node.offset + targetSymbol.node.length),
+  };
+}
+
+function createOriginSelectionRange(document: TextDocument, ref: JRefSymbol): Range {
+  const refValueNode = ref.node;
+  return {
+    start: document.positionAt(refValueNode.offset + 1), // +1 to skip the opening quote
+    end: document.positionAt(refValueNode.offset + refValueNode.length - 1), // -1 to skip the closing quote
+  };
+}

server/src/server.ts

@@ -14,9 +14,9 @@ import { ParseError } from 'jsonc-parser';
 import { createParseErrorDiagnostic } from './providers/diagnostics';
 
 import { onDefinition } from './providers/definition';
-import { SymbolTable } from './visitor';
+import { SymbolTable } from './symbolTable';
 import { handleSemanticTokens, tokenTypes } from './providers/semanticTokens';
-import { analyze } from './utils';
+import { analyze } from './analyzer';
 
 // Create a connection for the server, using Node's IPC as a transport.
 // Also include all preview / proposed LSP features.

server/src/symbolTable.ts

@@ -0,0 +1,10 @@
+import { Node } from 'jsonc-parser';
+
+export interface JRefSymbol {
+  pointer: string;
+  node: Node;
+  isReference: boolean;
+  refersTo: string | null;
+}
+
+export type SymbolTable = Map<string, JRefSymbol>;

server/src/test/analyzer.test.ts

@@ -0,0 +1,91 @@
+import * as assert from 'assert';
+import { analyze } from '../analyzer.js';
+
+suite('Analyzer Test Suite', () => {
+  test('Should find a single $ref in a simple object', () => {
+    const text = '{"$ref": "path/to/schema.json"}';
+    const { symbols } = analyze(text);
+
+    assert.strictEqual(symbols.get('/$ref')?.node.type, 'string');
+    assert.strictEqual(symbols.get('/$ref')?.isReference, true);
+    assert.strictEqual(symbols.get('/$ref')?.refersTo, 'path/to/schema.json');
+  });
+
+  test('Should find multiple $refs in nested objects', () => {
+    const text = `{
+            "first": { "$ref": "one.json" },
+            "second": { "inner": { "$ref": "two.json" } }
+        }`;
+    const { symbols } = analyze(text);
+
+    assert.strictEqual(symbols.get('/first/$ref')?.node.type, 'string');
+    assert.strictEqual(symbols.get('/first/$ref')?.isReference, true);
+    assert.strictEqual(symbols.get('/first/$ref')?.refersTo, 'one.json');
+
+    assert.strictEqual(symbols.get('/second/inner/$ref')?.node.type, 'string');
+    assert.strictEqual(symbols.get('/second/inner/$ref')?.isReference, true);
+    assert.strictEqual(symbols.get('/second/inner/$ref')?.refersTo, 'two.json');
+  });
+
+  test('Should find $refs inside arrays', () => {
+    const text = `[
+            { "$ref": "item1.json" },
+            { "other": "value" },
+            { "$ref": "item2.json" }
+        ]`;
+    const { symbols } = analyze(text);
+
+    assert.strictEqual(symbols.get('/0/$ref')?.node.type, 'string');
+    assert.strictEqual(symbols.get('/0/$ref')?.isReference, true);
+    assert.strictEqual(symbols.get('/0/$ref')?.refersTo, 'item1.json');
+
+    assert.strictEqual(symbols.get('/1/other')?.node.type, 'string');
+    assert.strictEqual(symbols.get('/1/other')?.isReference, false);
+    assert.strictEqual(symbols.get('/1/other')?.refersTo, null);
+
+    assert.strictEqual(symbols.get('/2/$ref')?.node.type, 'string');
+    assert.strictEqual(symbols.get('/2/$ref')?.isReference, true);
+    assert.strictEqual(symbols.get('/2/$ref')?.refersTo, 'item2.json');
+  });
+
+  test('Should NOT pick up $ref if the value is not a string', () => {
+    const text = '{"$ref": 123}';
+    const { symbols } = analyze(text);
+
+    assert.strictEqual(symbols.get('/$ref')?.isReference, false);
+    assert.strictEqual(symbols.get('/$ref')?.refersTo, null);
+  });
+
+  test('Should handle empty objects and arrays', () => {
+    const text = '{"obj": {}, "arr": []}';
+    const { symbols } = analyze(text);
+
+    assert.ok(symbols.has('/obj'), 'Should have a symbol for the empty object');
+    assert.strictEqual(symbols.get('/obj')?.node.type, 'object');
+
+    assert.ok(symbols.has('/arr'), 'Should have a symbol for the empty array');
+    assert.strictEqual(symbols.get('/arr')?.node.type, 'array');
+  });
+
+  test('Should handle undefined or null nodes gracefully', () => {
+    const { symbols } = analyze('');
+    assert.strictEqual(symbols.size, 0);
+  });
+
+  test('Should handle array elements', () => {
+    const text = '{"arr": ["a", "b", "c"]}';
+    const { symbols } = analyze(text);
+
+    assert.ok(symbols.has('/arr'), 'Should have a symbol for the array');
+    assert.strictEqual(symbols.get('/arr')?.node.type, 'array');
+
+    assert.ok(symbols.has('/arr/0'), 'Should have a symbol for the first element');
+    assert.strictEqual(symbols.get('/arr/0')?.node.type, 'string');
+
+    assert.ok(symbols.has('/arr/1'), 'Should have a symbol for the second element');
+    assert.strictEqual(symbols.get('/arr/1')?.node.type, 'string');
+
+    assert.ok(symbols.has('/arr/2'), 'Should have a symbol for the third element');
+    assert.strictEqual(symbols.get('/arr/2')?.node.type, 'string');
+  });
+});

server/src/test/definition.test.ts

@@ -2,8 +2,7 @@ import * as assert from 'assert';
 import { URI } from 'vscode-uri';
 import { TextDocument } from 'vscode-languageserver-textdocument';
 import { onDefinition } from '../providers/definition.js';
-import { parseTree } from 'jsonc-parser';
-import { SymbolTable, visit } from '../visitor.js';
+import { analyze } from '../analyzer.js';
 import { DefinitionParams } from 'vscode-languageserver/node';
 import path from 'path';
 
@@ -23,10 +22,7 @@ suite('Definition Test Suite', () => {
     const uri = URI.file(path.resolve('/abs/path/main.jref')).toString();
     const doc = TextDocument.create(uri, 'jref', 1, text);
 
-    const errors: any[] = [];
-    const ast = parseTree(text, errors);
-    const symbols: SymbolTable = new Map();
-    visit(ast, symbols);
+    const { symbols } = analyze(text);
 
     const context = {
       documents: new MockTextDocuments([doc]) as any,
@@ -50,10 +46,7 @@ suite('Definition Test Suite', () => {
     const uri = URI.file(path.resolve('/abs/path/main.jref')).toString();
     const doc = TextDocument.create(uri, 'jref', 1, text);
 
-    const errors: any[] = [];
-    const ast = parseTree(text, errors);
-    const symbols: SymbolTable = new Map();
-    visit(ast, symbols);
+    const { symbols } = analyze(text);
 
     const context = {
       documents: new MockTextDocuments([doc]) as any,
@@ -78,13 +71,9 @@ suite('Definition Test Suite', () => {
     const schemaUri = URI.file(path.resolve('/abs/path/schema.jref')).toString();
     const schemaDoc = TextDocument.create(schemaUri, 'jref', 1, schemaText);
 
-    const mainAst = parseTree(mainText);
-    const mainSymbols: SymbolTable = new Map();
-    visit(mainAst, mainSymbols);
+    const { symbols: mainSymbols } = analyze(mainText);
 
-    const schemaAst = parseTree(schemaText);
-    const schemaSymbols: SymbolTable = new Map();
-    visit(schemaAst, schemaSymbols);
+    const { symbols: schemaSymbols } = analyze(schemaText);
 
     const context = {
       documents: new MockTextDocuments([mainDoc, schemaDoc]) as any,