diff --git a/client/src/test/extension.test.ts b/client/src/test/extension.test.ts index c814a3c..48d3820 100644 --- a/client/src/test/extension.test.ts +++ b/client/src/test/extension.test.ts @@ -15,7 +15,7 @@ suite('Extension Test Suite', () => { test('Extension should be present', () => { assert.ok(vscode.extensions.getExtension('ntoulasm.jref-language-server-extension')); - }); + }).timeout(5000); test('Should activate the extension when a .jref file is opened', async () => { const ext = vscode.extensions.getExtension('ntoulasm.jref-language-server-extension'); @@ -108,4 +108,56 @@ suite('Extension Test Suite', () => { const hasResults = locations && locations.length > 0; assert.strictEqual(hasResults, false, 'Should not return a definition for non $ref keys'); }).timeout(5000); + + test('Should provide a definition for $ref values with fragments', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'jref-test-fragment-')); + const targetFile = path.join(tmpDir, 'schema.jref'); + const sourceFile = path.join(tmpDir, 'main.jref'); + + const schemaContent = '{"definitions": {"target": {}}}'; + fs.writeFileSync(targetFile, schemaContent); + fs.writeFileSync(sourceFile, '{"$ref": "schema.jref#/definitions/target"}'); + + // Open the target schema document first so the server indexes it + const schemaDoc = await vscode.workspace.openTextDocument(targetFile); + await vscode.window.showTextDocument(schemaDoc); + + const doc = await vscode.workspace.openTextDocument(sourceFile); + await vscode.window.showTextDocument(doc); + + await sleep(2000); + + // Character 12 is inside "schema.jref#/definitions/target" + const position = new vscode.Position(0, 12); + + const locations = await vscode.commands.executeCommand( + 'vscode.executeDefinitionProvider', + doc.uri, + position, + ); + + assert.ok( + locations && Array.isArray(locations) && locations.length > 0, + 'Should return an array of LocationLinks', + ); + + const link = locations[0]; + const resolvedPath = link.targetUri.fsPath.toLowerCase(); + const expectedPath = targetFile.toLowerCase(); + + assert.strictEqual( + resolvedPath, + expectedPath, + `Expected to point to ${expectedPath} but got ${resolvedPath}`, + ); + + // The target range should point to the value of "target" property in schema.jref + // {"definitions": {"target": {}}} + // ^--- {} starts at character 27 + assert.strictEqual( + link.targetRange.start.character, + 27, + `Expected target character 27, but got ${link.targetRange.start.character}`, + ); + }).timeout(10000); }); diff --git a/server/src/definition.ts b/server/src/definition.ts index c75f714..2158133 100644 --- a/server/src/definition.ts +++ b/server/src/definition.ts @@ -1,41 +1,67 @@ import path from 'path'; import { URI } from 'vscode-uri'; -import { DefinitionParams, DefinitionLink, TextDocuments } from 'vscode-languageserver/node'; -import { Node } from 'jsonc-parser'; +import { DefinitionParams, DefinitionLink, TextDocuments, Range } from 'vscode-languageserver/node'; import { TextDocument } from 'vscode-languageserver-textdocument'; +import { JRefSymbol, SymbolTable } from './visitor'; interface ServerContext { documents: TextDocuments; - documentRefs: WeakMap>; + documentSymbols: WeakMap; } +const defaultTargetRange: Range = { + start: { line: 0, character: 0 }, + end: { line: 0, character: 0 }, +}; + export function onDefinition( params: DefinitionParams, context: ServerContext, ): DefinitionLink[] | undefined { - const { documents, documentRefs } = context; + const { documents, documentSymbols } = context; const document = documents.get(params.textDocument.uri); if (!document) return; - const refs = documentRefs.get(document); - if (!refs || refs.length === 0) return; + const symbols = documentSymbols.get(document); + if (!symbols || symbols.size === 0) return; + const refs = Array.from(symbols.values()).filter((symbol) => symbol.isReference); const offset = document.offsetAt(params.position); const targetRef = refs.find((ref) => { - const value = ref.children![1]; - return offset >= value.offset && offset <= value.offset + value.length; + return offset >= ref.node.offset && offset <= ref.node.offset + ref.node.length; }); if (!targetRef) return; - return createDefinitionLink(document, targetRef); + return createDefinitionLink(document, targetRef, context); } -function createDefinitionLink(document: TextDocument, ref: Node): DefinitionLink[] | undefined { - const refValueNode = ref.children![1]; +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, targetPath); + const absolutePath = path.resolve(currentDir, uri.path.slice(1)); + const targetDocument = documents.get(URI.file(absolutePath).toString()); + const targetRange = getTargetRange(targetDocument); + + function getTargetRange(targetDocument: TextDocument | undefined): Range { + if (!targetDocument) return defaultTargetRange; + const targetSymbolTable = documentSymbols.get(targetDocument); + if (!targetSymbolTable) return defaultTargetRange; + 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), + }; + } + return [ { originSelectionRange: { @@ -43,14 +69,8 @@ function createDefinitionLink(document: TextDocument, ref: Node): DefinitionLink end: document.positionAt(refValueNode.offset + refValueNode.length - 1), // -1 to skip the closing quote }, targetUri: URI.file(absolutePath).toString(), - targetRange: { - start: { line: 0, character: 0 }, - end: { line: 0, character: 0 }, - }, - targetSelectionRange: { - start: { line: 0, character: 0 }, - end: { line: 0, character: 0 }, - }, + targetRange: targetRange, + targetSelectionRange: targetRange, }, ]; } diff --git a/server/src/server.ts b/server/src/server.ts index 8840406..b0e3559 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -14,14 +14,14 @@ import { Node, ParseError, parseTree } from 'jsonc-parser'; import { createParseErrorDiagnostic } from './diagnostics'; import { onDefinition } from './definition'; -import { visit } from './visitor'; +import { JRefSymbol, SymbolTable, visit } from './visitor'; // Create a connection for the server, using Node's IPC as a transport. // Also include all preview / proposed LSP features. let connection = createConnection(ProposedFeatures.all); // Create a simple text document manager. let documents: TextDocuments = new TextDocuments(TextDocument); -const documentRefs: WeakMap> = new WeakMap(); +const documentSymbols: WeakMap = new WeakMap(); connection.onInitialize((params: InitializeParams) => { const result: InitializeResult = { @@ -38,9 +38,9 @@ connection.onInitialize((params: InitializeParams) => { documents.onDidChangeContent((change: TextDocumentChangeEvent) => { const errors: ParseError[] = []; const ast: Node | undefined = parseTree(change.document.getText(), errors); - const references: Array = []; - visit(ast, references); - documentRefs.set(change.document, references); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); + documentSymbols.set(change.document, symbols); sendDiagnostics(change.document, errors); }); @@ -51,7 +51,7 @@ function sendDiagnostics(document: TextDocument, parseErrors: ParseError[]) { }); } -connection.onDefinition((params) => onDefinition(params, { documents, documentRefs })); +connection.onDefinition((params) => onDefinition(params, { documents, documentSymbols })); // Make the text document manager listen on the connection // for open, change and close text document events diff --git a/server/src/test/definition.test.ts b/server/src/test/definition.test.ts index 45fe657..edbd357 100644 --- a/server/src/test/definition.test.ts +++ b/server/src/test/definition.test.ts @@ -2,8 +2,8 @@ import * as assert from 'assert'; import { URI } from 'vscode-uri'; import { TextDocument } from 'vscode-languageserver-textdocument'; import { onDefinition } from '../definition.js'; -import { Node, parseTree } from 'jsonc-parser'; -import { visit } from '../visitor.js'; +import { parseTree } from 'jsonc-parser'; +import { SymbolTable, visit } from '../visitor.js'; import { DefinitionParams } from 'vscode-languageserver/node'; import path from 'path'; @@ -25,12 +25,12 @@ suite('Definition Test Suite', () => { const errors: any[] = []; const ast = parseTree(text, errors); - const refs: Node[] = []; - visit(ast, refs); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); const context = { documents: new MockTextDocuments([doc]) as any, - documentRefs: new WeakMap([[doc, refs]]), + documentSymbols: new WeakMap([[doc, symbols]]), }; const params: DefinitionParams = { @@ -52,12 +52,12 @@ suite('Definition Test Suite', () => { const errors: any[] = []; const ast = parseTree(text, errors); - const refs: Node[] = []; - visit(ast, refs); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); const context = { documents: new MockTextDocuments([doc]) as any, - documentRefs: new WeakMap([[doc, refs]]), + documentSymbols: new WeakMap([[doc, symbols]]), }; const params: DefinitionParams = { @@ -68,4 +68,45 @@ suite('Definition Test Suite', () => { const result = onDefinition(params, context); assert.ok(!result); }); + + test('Should return a definition for $ref with fragment', () => { + const mainText = '{"$ref": "schema.jref#/definitions/target"}'; + const mainUri = URI.file(path.resolve('/abs/path/main.jref')).toString(); + const mainDoc = TextDocument.create(mainUri, 'jref', 1, mainText); + + const schemaText = '{"definitions": {"target": {}}}'; + 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 schemaAst = parseTree(schemaText); + const schemaSymbols: SymbolTable = new Map(); + visit(schemaAst, schemaSymbols); + + const context = { + documents: new MockTextDocuments([mainDoc, schemaDoc]) as any, + documentSymbols: new WeakMap([ + [mainDoc, mainSymbols], + [schemaDoc, schemaSymbols], + ]), + }; + + const params: DefinitionParams = { + textDocument: { uri: mainUri }, + position: { line: 0, character: 12 }, // Inside "schema.jref#/definitions/target" + }; + + const result = onDefinition(params, context); + + assert.ok(result && result.length > 0); + const link = result![0]; + assert.ok(link.targetUri.endsWith('schema.jref')); + // Target range should point to the "target" property value in schema.jref + // {"definitions": {"target": {}}} + // ^--- value {} is at character 27 + assert.strictEqual(link.targetRange.start.character, 27); + }); }); diff --git a/server/src/test/visitor.test.ts b/server/src/test/visitor.test.ts index c465af9..6e5b02f 100644 --- a/server/src/test/visitor.test.ts +++ b/server/src/test/visitor.test.ts @@ -1,18 +1,17 @@ import * as assert from 'assert'; -import { Node, parseTree } from 'jsonc-parser'; -import { visit } from '../visitor.js'; +import { parseTree } from 'jsonc-parser'; +import { SymbolTable, visit } from '../visitor.js'; suite('Visitor Test Suite', () => { test('Should find a single $ref in a simple object', () => { const text = '{"$ref": "path/to/schema.json"}'; const ast = parseTree(text); - const acc: Node[] = []; - visit(ast, acc); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); - assert.strictEqual(acc.length, 1); - assert.strictEqual(acc[0].type, 'property'); - assert.strictEqual(acc[0].children![0].value, '$ref'); - assert.strictEqual(acc[0].children![1].value, 'path/to/schema.json'); + 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', () => { @@ -21,12 +20,16 @@ suite('Visitor Test Suite', () => { "second": { "inner": { "$ref": "two.json" } } }`; const ast = parseTree(text); - const acc: Node[] = []; - visit(ast, acc); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); - assert.strictEqual(acc.length, 2); - assert.strictEqual(acc[0].children![1].value, 'one.json'); - assert.strictEqual(acc[1].children![1].value, 'two.json'); + 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', () => { @@ -36,35 +39,67 @@ suite('Visitor Test Suite', () => { { "$ref": "item2.json" } ]`; const ast = parseTree(text); - const acc: Node[] = []; - visit(ast, acc); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); + + 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(acc.length, 2); - assert.strictEqual(acc[0].children![1].value, 'item1.json'); - assert.strictEqual(acc[1].children![1].value, 'item2.json'); + 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 ast = parseTree(text); - const acc: Node[] = []; - visit(ast, acc); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); - assert.strictEqual(acc.length, 0); + 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 ast = parseTree(text); - const acc: Node[] = []; - visit(ast, acc); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); - assert.strictEqual(acc.length, 0); + 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 acc: Node[] = []; - visit(undefined, acc); - assert.strictEqual(acc.length, 0); + const symbols: SymbolTable = new Map(); + visit(undefined, symbols); + assert.strictEqual(symbols.size, 0); + }); + + test('Should handle array elements', () => { + const text = '{"arr": ["a", "b", "c"]}'; + const ast = parseTree(text); + const symbols: SymbolTable = new Map(); + visit(ast, symbols); + + 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'); }); }); diff --git a/server/src/visitor.ts b/server/src/visitor.ts index 80e7dd4..0208d43 100644 --- a/server/src/visitor.ts +++ b/server/src/visitor.ts @@ -1,45 +1,65 @@ import { Node } from 'jsonc-parser'; -const visitFunctions: Record) => void> = { - object: visitCompositeNode, - array: visitCompositeNode, +export interface JRefSymbol { + pointer: string; + node: Node; + isReference: boolean; + refersTo: string | null; +} + +export type SymbolTable = Map; + +const visitFunctions: Record void> = { + object: visitObject, + array: visitArray, property: visitProperty, }; -function isCompositeNode(node: Node): boolean { - return node.type === 'object' || node.type === 'array'; +function visitObject(node: Node, acc: SymbolTable, path: string) { + node?.children?.forEach((child) => { + visit(child, acc, path); + }); } -function visitCompositeNode(node: Node, acc: Array) { - const children = node?.children || []; - for (const child of children) { - visit(child, acc); - } +function visitArray(node: Node, acc: SymbolTable, path: string) { + node?.children?.forEach((child, index) => { + visit(child, acc, path + '/' + index); + }); } -function visitProperty(node: Node, acc: Array) { - const children = node?.children || []; - if (children.length !== 2) { - /* incomplete property */ - return; - } - const key = children[0]; - const value = children[1]; - const isReference = key.type === 'string' && key.value === '$ref' && value.type === 'string'; - if (isReference) { - acc.push(node); - } - - if (isCompositeNode(value)) { - visit(value, acc); - } +function visitProperty(node: Node, acc: SymbolTable, path: string) { + if (node.children?.length !== 2) return; + const key = node.children[0].value; + const value = node.children[1]; + visit(value, acc, `${path}/${key}`); } -export function visit(node: Node | undefined, acc: Array) { +export function visit(node: Node | undefined, acc: SymbolTable, path: string = '') { if (!node?.type) { console.error('Node type is undefined'); return; } + const symbol = createJRefSymbol(node, path); + acc.set(path, symbol); + const visit = visitFunctions[node.type]; - visit?.(node, acc); + visit?.(node, acc, path); +} + +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' + ); } diff --git a/testfiles/directory.jref b/testfiles/directory.jref new file mode 100644 index 0000000..2f66cf5 --- /dev/null +++ b/testfiles/directory.jref @@ -0,0 +1,22 @@ +{ + "offices": [ + { + "site": "Athens HQ", + "support": { + "landline": "+30-210-555-0199", + "voip": "ext-701" + } + }, + { + "site": "London Branch", + "support": { + "landline": "+44-20-7946-0123", + "voip": "ext-202" + } + } + ], + "roles": { + "engineering": ["Junior Dev", "Senior Dev", "Lead"], + "management": ["PM", "Director"] + } +} \ No newline at end of file diff --git a/testfiles/person.jref b/testfiles/person.jref index 8be7e82..43bda54 100644 --- a/testfiles/person.jref +++ b/testfiles/person.jref @@ -1,5 +1,15 @@ { "name": "Jason", "address": { "$ref": "address.jref" }, - "addresses": [{ "$ref": "address.jref" }] + "phones": [{ "$ref": "phone1.jref#/phone" }, { "$ref": "phone2.jref#/phone" }], + "title": { + "$ref": "directory.jref#/roles/engineering/1" + }, + "workLocation": { + "siteName": { "$ref": "directory.jref#/offices/0/site" }, + "emergencyContact": { "$ref": "directory.jref#/offices/0/support/landline" } + }, + "directDial": { + "$ref": "directory.jref#/offices/0/support/voip" + } } \ No newline at end of file diff --git a/testfiles/phone1.jref b/testfiles/phone1.jref new file mode 100644 index 0000000..8180dca --- /dev/null +++ b/testfiles/phone1.jref @@ -0,0 +1,3 @@ +{ + "phone": "123-456-7890" +} \ No newline at end of file