JohnnyMorganz / luau-lsp

Language Server Implementation for Luau
MIT License
250 stars 61 forks source link

getTextDocumentFromModuleName incorrectly forces a resolution via `Uri::file`, which breaks for non-file-based URIs (e.g., monaco) #802

Open Arc157 opened 1 month ago

Arc157 commented 1 month ago

Hello, I'm trying to upgrade my project from using version 1.16.2 of luau-lsp to the latest version (1.33.1), but during this process I encountered an issue. The language server in itself does seem to work, but requests like hover no longer get a correct response in version 1.16.3, but if I just revert to 1.16.2 it works fine.

1.16.2:

{
  jsonrpc: '2.0',
  id: 12,
  method: 'textDocument/hover',
  params: {
    textDocument: { uri: 'inmemory://model/1' },
    position: { line: 0, character: 0 }
  }
}
{
  id: 12,
  jsonrpc: '2.0',
  result: {
    contents: {
      kind: 'markdown',
      value: '```lua\n' +
        'function print(string): ()\n' +
        '```\n' +
        '----------\n' +
        'Prints all provided values to the output.\n' +
        '\n' +
        '[Learn More](https://create.roblox.com/docs/reference/engine/globals/LuaGlobals#print)'
    }
  }
}

1.16.3:

{
  jsonrpc: '2.0',
  id: 14,
  method: 'textDocument/hover',
  params: {
    textDocument: { uri: 'inmemory://model/1' },
    position: { line: 0, character: 0 }
  }
}
{ id: 14, jsonrpc: '2.0', result: null }

Here's my server.ts file:

import { WebSocketServer } from 'ws'
import { resolve } from 'path';
import { IWebSocket, WebSocketMessageReader, WebSocketMessageWriter } from 'vscode-ws-jsonrpc';
import { createConnection, createServerProcess, forward } from 'vscode-ws-jsonrpc/server';
import { Message, InitializeRequest, InitializeParams } from 'vscode-languageserver-protocol';

const port = 8080;
const wss = new WebSocketServer({ port });
wss.on('connection', (webSocket) => {
    const socket: IWebSocket = {
        send: (content) =>
            webSocket.send(content, (error) => {
                if (error) throw error;
            }),
        onMessage: (callback) => webSocket.on('message', callback),
        onError: (callback) => webSocket.on('error', callback),
        onClose: (callback) => webSocket.on('close', callback),
        dispose: () => webSocket.close(),
    };
    if (webSocket.readyState === webSocket.OPEN) {
        launch(socket);
    } else {
        webSocket.on('open', () => launch(socket));
    }
});

export const launch = (socket: IWebSocket) => {
    const reader = new WebSocketMessageReader(socket);
    const writer = new WebSocketMessageWriter(socket);
    const socketConnection = createConnection(reader, writer, () => socket.dispose());
    const serverConnection = createServerProcess(
        'LuaU',
        resolve(process.cwd(), './luau-lsp.exe'),
        ["lsp", "--docs=./en-us.json", "--definitions=./globalTypes.d.luau", "--base-luaurc=./.luaurc"]
    );

    forward(socketConnection, serverConnection, (message) => {
        if (Message.isRequest(message)) {
            if (message.method === InitializeRequest.type.method) {
                const initializeParams = message.params as InitializeParams;
                initializeParams.processId = process.pid;
            }
        }
        console.log(message);
        return message;
    });
};

Any help is appreciated!

JohnnyMorganz commented 1 month ago

Seems the same issue as https://github.com/JohnnyMorganz/luau-lsp/issues/583#issuecomment-2226895942

Can you double check the URI used to open the document is the same URI used to send the request?

Arc157 commented 1 month ago

Seems the same issue as #583 (comment)

Can you double check the URI used to open the document is the same URI used to send the request?

I'm not sure how to debug that, my client is based upon monaco-languageclient (https://github.com/TypeFox/monaco-languageclient). Do you know by any chance where that behaviour is defined?

Arc157 commented 1 month ago

Seems the same issue as #583 (comment)

Can you double check the URI used to open the document is the same URI used to send the request?

Alright, I did some digging and turns out they're the same

Arc157 commented 1 month ago

Alright, I managed to fix this issue by just simply adjusting this code from https://github.com/JohnnyMorganz/luau-lsp/blob/main/src/WorkspaceFileResolver.cpp:

const TextDocument* WorkspaceFileResolver::getTextDocumentFromModuleName(const Luau::ModuleName& name) const
{
    // Handle untitled: files
    if (Luau::startsWith(name, "untitled:"))
        return getTextDocument(Uri::parse(name));

    if (auto filePath = platform->resolveToRealPath(name))
        return getTextDocument(Uri::file(*filePath));

    return nullptr;
}
const TextDocument* WorkspaceFileResolver::getTextDocumentFromModuleName(const Luau::ModuleName& name) const
{
    // Handle untitled: files
    if (Luau::startsWith(name, "untitled:"))
        return getTextDocument(Uri::parse(name));

    if (auto filePath = platform->resolveToRealPath(name)) {
        auto it = managedFiles.find(filePath->generic_string());
        if (it != managedFiles.end()) {
            return &it->second;
    }
        return getTextDocument(Uri::file(*filePath));
    }

    return nullptr;
}

I'll push a pull request soon.

JohnnyMorganz commented 1 month ago

Ah I think the problem is the incorrect call to Uri::file for a non-file-based URI, rather than the added managed files lookup (getTextDocument already handles lookup in managed files)

Arc157 commented 1 month ago

Ah I think the problem is the incorrect call to Uri::file for a non-file-based URI, rather than the added managed files lookup (getTextDocument already handles lookup in managed files)

Is there a prebuilt API within luau-lsp to detect if its a non-file-based URI, or does that have to be implemented?