erlang-ls / erlang_ls

The Erlang Language Server
https://erlang-ls.github.io/
Apache License 2.0
630 stars 136 forks source link

Go to definition/find references does not work from attributes referencing functions or via macros #417

Open dszoboszlay opened 4 years ago

dszoboszlay commented 4 years ago

Describe the bug Consider the following module:

-module(test).
-export([foo/0, bar/0, baz/0]).
-deprecated([foo/0]).

-define(MYSELF, test).

foo() -> ?MODULE:bar().
bar() -> ?MYSELF:baz().
baz() -> ok.

I would expect go to definition/find references would work from every place foo, bar or baz is mentioned, but it does not.

To Reproduce Try to use go to definition on the function name in the following places:

Or try to find references for the baz/0 function, and none will turn up.

Expected behavior Go to definition/find references would work from all places.

Actual behavior It does not work across module attributes (except for -export) or macros (except for ?MODULE).

Context

robertoaloi commented 4 years ago

Thanks for the report @dszoboszlay , we will look into this.

robertoaloi commented 4 years ago

As a reference for other contributors, the deprecated attribute is used to inform xref (not the compiler) about deprecations:

https://erlang.org/doc/man/xref.html#deprecated_function

robertoaloi commented 4 years ago

Support for the deprecated attribute is something that is easy to add, so we can do that. I am not sure if we can generalize it for custom attributes (some information is lost for wild attributes, making it really hard to fetch position information), but I will check.

The second point requires more effort. We support ?MODULE because it's extremely widely spread, but processing generic macros is another story.

Contributions are also welcome :)

jfacorro commented 4 years ago

IMHO we shouldn't process macros, that gets very complicated very quickly.

dszoboszlay commented 4 years ago

I don't think you should try to find function names in arbitrary attributes. But there are a number of attributes defined in OTP that would make sense to parse (export, import, compile, deprecated and maybe some more).

Regarding processing macros: don't you already have to do that? Compiling the module would involve running the preprocessor too.

jfacorro commented 4 years ago

Regarding processing macros: don't you already have to do that? Compiling the module would involve running the preprocessor too.

Compiling is different, since the compiler takes care of everything. We only use the compiler for the warning. The Erlang standard library doesn't expose an API to use just the pre-processor.

We are currently avoiding expanding macros through the usage of a fork of eep_dodger, which attempts to dodge macros by parsing and return them as a syntax tree.

robertoaloi commented 4 years ago

We can do things incrementally, starting from the simple ones. I will probably split this ticket into two or three, and we can address them separately.

dszoboszlay commented 4 years ago

The Erlang standard library doesn't expose an API to use just the pre-processor.

The 'P' option for the compiler would "produce a listing of the parsed code, after preprocessing and parse transforms", which I think is what you need. Just take care of respecting the -file(FileName, LineNumber). attributes inserted by the compiler in order to map lines back to positions in the original sources.

alanz commented 4 years ago

@michalmuskala will your new parser help here?

michalmuskala commented 4 years ago

Not directly, but it could make it simpler.

The parser tries to keep as much information as possible (e.g. retaining full AST for values of attributes instead of converting to forms), and makes macros first-class elements of the AST. With those changes, it's easier to process those elements retaining full information about positions, etc. But it would still require custom implementation for how to do it all.