Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vscode.executeDefinitionProvider fails to return result if the document is not shown in a window #7124

Closed
HO-COOH opened this issue Mar 7, 2021 · 3 comments

Comments

@HO-COOH
Copy link

HO-COOH commented Mar 7, 2021

Type: LanguageService

Describe the bug

  • OS and Version: Windows 10 20H2
  • VS Code Version: 1.54.0
  • C/C++ Extension Version: v1.2.2
  • Other extensions you installed (and if the issue persists after disabling them):
  • Does this issue involve using SSH remote to run the extension on a remote machine?: No

I am developing a vscode extension, using the vscode's built-in vscode.executeDefinitionProvider to get the definition (file path) of an include directive, using the cpptools as the language server. I found out that a call to vscode.executeDefinitionProvider would fail if the uri of a document is not displayed in a window, not even after vscode.workspace.openTextDocument but it must be opened in a window after a call to vscode.window.showTextDocument. Previously I raised this issue in vscode's repo and the maintainer told me to forward it here

Steps to reproduce

function getDefinition(uri: vscode.Uri, position: vscode.Position)
{
    return vscode.commands.executeCommand("vscode.executeDefinitionProvider", uri, position) as Thenable<(vscode.Location | vscode.LocationLink)[]>;
}
function getCurrentUri()
{
    return vscode.window.activeTextEditor?.document.uri;
}
function getFileUri(definition: (vscode.Location | vscode.LocationLink)[])
{
    return (definition[0] as vscode.Location).uri;
}

async function testGood()
{
    const uri = getCurrentUri()!;
    const pos = new vscode.Position(0, 10); //the position of an include statement
    const includeDefinition = await getDefinition(uri, pos);   //ok, but returns a bad vscode.Location, which contains a bad vscode.Uri
    const uri2 = getFileUri(includeDefinition);

    const pos2 = new vscode.Position(8, 10); //the position of another include statement in the file denoted as "uri2", which is not opened yet
    
    vscode.workspace.openTextDocument(uri2).then(async doc => //"uri2" is opened
    {
        vscode.window.showTextDocument(doc).then(async () => //"uri2" is shown in a window
        {
            const definition2 = await commands.definitionProvider(doc.uri, pos2); //SUCCESS
        });
    });
}

async function testBad()
{
    const uri = getCurrentUri()!;
    const pos = new vscode.Position(0, 10); //the position of an include statement
    const includeDefinition = await getDefinition(uri, pos);   //ok, but returns a bad vscode.Location, which contains a bad vscode.Uri
    const uri2 = getFileUri(includeDefinition);

    const pos2 = new vscode.Position(8, 10); //the position of another include statement in the file denoted as "uri2", which is not opened yet
    
    //directly using the uri2 to getDefinition() would fail like mentioned in #118085, but open it then get the uri property would results in a good uri
    vscode.workspace.openTextDocument(uri2).then(async doc => //"uri2" is opened but not shown in a window
    {
        const definition2 = await getDefinition(doc.uri, pos2); //STILL FAILS HERE
    });
}
  1. Test this extension with a random C++ file. I hard coded the vscode.Position() in the code for testing purpose, but you can change it to whatever position.

The definition2 of testGood() function will returns a good result like this

-> definition2
(1) [B]
0:B {uri: m, range: B}
range:B {_start: B, _end: B}
_end:B {_line: 0, _character: 1}
_start:B {_line: 0, _character: 0}
end (get):ƒ end(){return this._end}
isEmpty (get):ƒ isEmpty(){return this._start.isEqual(this._end)}
isSingleLine (get):ƒ isSingleLine(){return this._start.line===this._end.line}
start (get):ƒ start(){return this._start}
__proto__:S
uri:m {scheme: 'file', authority: '', path: '/C:/Program Files (x86)/Microsoft Visual Studi…C/Tools/MSVC/14.28.29333/include/yvals_core.h', query: '', fragment: '', …}
__proto__:u
length:1
__proto__:Array(0)

But the testBad() function returns an empty array of definition.

-> definition2
(0) []
length:0
__proto__:Array(0)

Expected behavior
testBad() should return the same result as testGood().

Invalid opened file instance. Ignoring IntelliSense message for file c:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\utility.
@sean-mcmanus sean-mcmanus self-assigned this Mar 8, 2021
@sean-mcmanus
Copy link
Collaborator

Yeah, this is "by design". See the comment and code at https://github.com/microsoft/vscode-cpptools/blob/main/Extension/src/LanguageServer/protocolFilter.ts#L69 . We intentionally avoid sending a didOpen to cpptools unless a window is visible to avoid creating a TU when a hover is done. If you wanted to override that, you'd have to add some way to force the didOpen to be sent. Our Go to Definition implementation relies on having didOpen be called.

@sean-mcmanus sean-mcmanus removed their assignment Mar 8, 2021
@HO-COOH
Copy link
Author

HO-COOH commented Mar 9, 2021

@sean-mcmanus Thanks for that clarification. The code seems to show even after the open document event is fired, it still checks whether the document is actually opened as one visible tab first. So I suppose it is impossible to get definition in the background?

@sean-mcmanus
Copy link
Collaborator

It's not impossible...it would just require changes to our extension's code (open source TypeScript) and it would need to be done in some way that doesn't negatively affect performance too much for users who aren't using the background Go to Definition feature. Our implementation of Find All References essentially does a lot of Go to Definitions in the background without a document open.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants