GoToDefinition fails to find function in path added at runtime?

[rymdbar]

To the best of my understanding this is not a duplicate of any other open issue. Please consider these two files:

Greeting.pm

sub say_it {
    print "Hello perl\n";
}

1;

and

hello

#!/usr/bin/env perl

use v5.36;
use warnings;

use lib './';
use Greeting;

sub hello {
    say_it();
}

hello();

If attempting to GoToDefinition on hello(), it does find sub hello. However when doing the same for say_it() my client gives me the message No definitions found, or to express is with was json-rpc:

{
  "method": "textDocument/definition",
  "jsonrpc": "2.0",
  "id": 2,
  "params": {
    "textDocument": {
      "uri": "file:///hello"
    },
    "position": {
      "character": 0,
      "line": 12
    }
  }
}
{
  "jsonrpc": "2.0",
  "id": 2,
  "result": [
    {
      "uri": "file:///home/cos/hax/hello_polyglot-world/perl/hello",
      "range": {
        "start": {
          "line": 8,
          "character": 0
        },
        "end": {
          "line": 8,
          "character": 500
        }
      }
    }
  ]
}

vs.

{
  "method": "textDocument/definition",
  "jsonrpc": "2.0",
  "id": 3,
  "params": {
    "textDocument": {
      "uri": "file:///hello"
    },
    "position": {
      "character": 4,
      "line": 9
    }
  }
}
{
  "jsonrpc": "2.0",
  "id": 3,
  "result": []
}

That's a bit unexpected to me, given how the description of PerlNavigator states:

Code Navigation ("Go To Definition") anywhere, including to installed modules and compile-time dependencies

I tried running the -MInquistor mentioned in #69, without really understanding the output. It does contain this row though:

say_it t Greeting.pm main 1

Is this expected behaviour? It seems more trivial than the other reported failures of GoToDefinition, but maybe this is a duplicate after all? or another corner case which is hard to address?

[bscan]

Hi @nospam2998, adding things at runtime normally works fine. The only issue is this line:

use lib './';

It adds the path relative to the current working directory, which is not necessarily the same as the script path. If you run the hello script without first changing your directory to that folder, the script itself will fail. Basically, it's not functioning as a relative import.

You could update the script to be either fully pathed:

use lib "/path/to/script";

Or if you want relative pathing

use FindBin qw($Bin);
use lib "$Bin/";

And then in either case, the script can be run regardless of the working directory and Perl Navigator will also find the function from the runtime path. This is a good example of what we were discussing in #146, where it doesn't need the workspace folder, it just uses perl's file resolution logic. However, it doesn't assume any particular working directory for the compilation step. I'm closing this as resolved, but feel free to followup.

[rymdbar]

Nice suggestion to use FindBin. That does indeed solve the issue for my specific example. Maybe it would also be worth making some minor changes to the documentation to make the constraints more clear?

Since the ticket is already closed I will not investigate whether GitHub allows renaming issues, but a more descriptive title might have been GoToDefinition fails to work with relative paths.

The only end-user documentation I've found is the README.md file, which has two possible injection points for a sentence highlighting the need for absolute paths. Either in the bullet point list Currently Implemented Features or in the text section Perl paths. With the Code Navigation point already being to long, the Perl paths seems like the best place. I also suggest moving the including to … dependencies part down there. Another improvement would be to actually strike the confusion-inducing relative path from that text. Essentially this could boil down to something like:

Code Navigation ("Go To Definition") anywhere, including to installed modules and compile-time dependencies.

and

All source code reachable by perl should be findable also by PerlNavigator, including installed modules and compile-time dependencies. If you have a nonstandard install of Perl, that however requires configuring the setting perlnavigator.perlPath. The subfolder $workspaceFolder/lib will be added to your path automatically. You can also add additional include paths that will be added to the perl search path (@inc) via perlnavigator.includePaths. All paths must be absolute, but in includePaths you can prepend relative paths with $workspaceFolder (which will be replaced by the full folder path). If you have a multi-root workspace, each folder will be added to the path.

Please consider the above phrasing and see if you might agree to some of the suggested changes.

As one understands from the filing of this issue, not knowing the cause of a problem causes wild geese to be chased. I filed the simplest possible example, but a fun thing to try is to move the module into the documented to be included sub-directory and doing a use './lib/';. This script will execute correctly, but PerlNavigator will go bananas and instead assume it must be executed from /usr/local/lib/perl5/5.36/mach/, which contains a lib.pm file. Now I understand why, but it was unexpected prior to getting your reply in this ticket.

[bscan]

Sure, updated documentation is always a good idea. Thanks for the draft, I will make some modifications to the docs. I re-opened the ticket to track.

One important clarification.

This script will execute correctly, but PerlNavigator will go bananas and instead assume it must be executed from /usr/local/lib/perl5/5.36/mach/, which contains a lib.pm file.

My core argument is that the script examples provided do not execute correctly. For example:

brian@brian:/tmp$ ls example/
Greeting.pm  hello
brian@brian:/tmp$ example/hello 
Can't locate Greeting.pm in @INC (you may need to install the Greeting module) (@INC entries checked: ./ /usr/local/lib64/perl5/5.40 /usr/local/share/perl5/5.40 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5) at example/hello line 6.
BEGIN failed--compilation aborted at example/hello line 6.

Which is pretty well aligned with the error from the PerlNavigator:

I would argue that use lib './'; and use './lib/' are bugs since we don't know where the user's working directory would be when executing a script. For comparison, Python uses . for relative imports such as from .Greeting import say_it. In Python, the . is a reference to the current package. For perl however, relative imports (meaning relative to a file or module) do not use ., they need FindBin or similar alternative.

Although perhaps I am biased by my own workflow, where I rarely run scripts from their directory. For example, if I have a perl script in /usr/local/bin/, I would never cd /usr/local/bin/ before running it. If this is a common need, we could add an option to allow setting the working directory that the Perl Navigator would use.

[rymdbar]

Your point was already clear, but thanks for elaborating on it. I'm not arguing that you. Absolute paths are definitely more correct than relying on pwd of the caller.

However one thing to mention is that I have a close friend who is fully fluent in perl. One might think that's an odd and irrelevant fact to state here, and it would be. Unless I also added that he neither use any language server nor any linters as he think they just get in his way with bogus messages. For experts that might be the case. For us casual coders, one might argue the tools should be as helpful and forgiving as possible. Including idiot-proof instructions on how to hold the thing.

I'm grateful for you pointing me at FindBin. It was new to me. Yet I'm not expecting software maintainers to train their users in everything vaguely related. My hope is that a reworded README.md might lead to fewer support requests, and thus a higher ratio of the more fun stuff for you. Please feel free to keep the conversion going, if keen on getting more of my input on the phrasing.