Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Support find references #104

Closed
faustinoaq opened this issue May 5, 2018 · 28 comments
Closed

Support find references #104

faustinoaq opened this issue May 5, 2018 · 28 comments

Comments

@faustinoaq
Copy link
Member

faustinoaq commented May 5, 2018

Hi community!

Find references is a feature I have no idea or clue on how to implement,

This is the only missing feature to complete basic LSP support see: https://langserver.org/

screenshot_20180504_205743
screenshot_20180504_205839

So, WDYT about this scry capability?

@faustinoaq faustinoaq added this to To do in Crystal Tools via automation May 5, 2018
@bew
Copy link
Contributor

bew commented May 5, 2018

So, WDYT about this scry capability?

Crazy 😛

I think this would need a special tool from the compiler, similar to tool implementation. We should be able to find the references by looking at the instantiations of a specific method (the compiler does that internally at some point)

@faustinoaq
Copy link
Member Author

So crystal tool references would be just like crystal tool implementation but reversed, right?

def foo # here you use go to references
end

foo  # here you use go to implementation

@crystal-lang-tools/scry @bcardiff @RX14 @asterite @ysbaddaden @straight-shoota WDYT?

@RX14
Copy link

RX14 commented May 6, 2018

Why can't scry implement this internally?

Does scry actually shell out to crystal tool implementation?!

@faustinoaq
Copy link
Member Author

Does scry actually shell out to crystal tool implementation?!

Yep 😅

We used to use implementations direct from compiler but scry was too slow and too big, please see: #53

@RX14
Copy link

RX14 commented May 6, 2018

I don't understand that... the compiler has to do the same amount of work as scry, and use the same memory, so where's the benefit to shelling out??

@faustinoaq
Copy link
Member Author

faustinoaq commented May 6, 2018

Using the compiler:

screenshot_20180304_083813

Using crystal command via shell:

screenshot_20180304_083640

That way scry can be friendly with pc resources

@RX14
Copy link

RX14 commented May 6, 2018

But you reach the same peak memory consumption either way. It's just you've moved the memory usage into crystal instead of scry. How is that a real benefit? And the beneifit you get from the extra memory consumption is way way faster completion because you've already parsed the code, you just need to run the analysis.

@faustinoaq
Copy link
Member Author

faustinoaq commented May 6, 2018

How is that a real benefit?

The memory was not being released, so scry got very heavy using even a couple of GBs

Also scry was compiling too slow since we needed the entire compiler (just like compiling crystal itself)

Now scry compiles very very fast:

$ time crystal build src/scry.cr -s -p --no-debug
Parse:                             00:00:00.000316202 (   0.25MB)
Semantic (top level):              00:00:00.404959721 (  52.33MB)
Semantic (new):                    00:00:00.002962556 (  52.33MB)
Semantic (type declarations):      00:00:00.032710793 (  52.33MB)
Semantic (abstract def check):     00:00:00.002463025 (  52.33MB)
Semantic (ivars initializers):     00:00:00.001851865 (  52.33MB)
Semantic (cvars initializers):     00:00:00.240542928 (  68.33MB)
Semantic (main):                   00:00:01.663311243 ( 196.77MB)
Semantic (cleanup):                00:00:00.006314623 ( 196.77MB)
Semantic (recursive struct check): 00:00:00.002034422 ( 196.77MB)
Codegen (crystal):                 00:00:01.197870686 ( 228.77MB)
Codegen (bc+obj):                  00:00:05.765841086 ( 228.77MB)
Codegen (linking):                 00:00:00.761140190 ( 228.77MB)
                                          
Codegen (bc+obj):
 - no previous .o files were reused
crystal build src/scry.cr -s -p --no-debug  14.42s user 0.81s system 149% cpu 10.159 total

Previously scry compilation used a huge amout of RAM (above 1GB) and took several minutes to compile in my pc (even Travis) 😅

@faustinoaq
Copy link
Member Author

faustinoaq commented May 6, 2018

Actually, Scry packages were around 20MB in the past, see #3 (comment) (even bigger uncompressed)

Now, release packages are very small:

screenshot_20180506_104142

Even statically typed (linux)

@faustinoaq
Copy link
Member Author

So, I think crystal tool references would be nice to have 😉

@RX14
Copy link

RX14 commented May 6, 2018

So scry doesn't do anything fancy and breaks as soon as there's a syntax error in your code. Oh well, not a very useful tool after all.

@faustinoaq
Copy link
Member Author

So scry doesn't do anything fancy and breaks as soon as there's a syntax error in your code. Oh well, not a very useful tool at all.

No,no, @RX14 scry is super useful, this tool will allow us to support almost every editor, even Visual Studio when Windows port for crystal is ready 😉

@RX14
Copy link

RX14 commented May 6, 2018

It's not useful for your find references or your go to implementation or for all of your "smart features" to break as soon as your code stops compiling. You need scry the most when you're in the middle of a refactor: when your code doesn't compile and you need to navigate it to find out why. And that is why scry isn't useful.

Real IDEs don't break in that situation.

This is why you can't build IDE features on top of a compiler: compilers exist to be strict about code, IDEs cannot be strict about code.

@faustinoaq
Copy link
Member Author

@RX14 not at all! 😅

So scry doesn't do anything fancy

Wrong 😅 , this tool does autocompletion, symbol listing, and a bunch of other nice features, (I'm working on rename feature right now 😉 ) we still use Crystal:;Parser

...breaks as soon as there's a syntax error in your code...

Wrong again 😅 We catch compiler bugs and return a ResponseError, then scry continues working without any issue. Also, compile time errors like syntax errors and semantic errors are very useful to see what do you need to fix. We use diagnostics to notify them 👍

...Oh well, not a very useful tool at all.

Wrong, again, again 😉 as I said before, this tool is the implementation of Language Server Protocol, the idea behind one server to rule them all 💪 . Right now I have diagnostics, completion, symbols and a bunch of first-class IDE features almost out-the-box on Intellij-Idea thanks to scry and the Language server protocol 😄

It's not useful for your find references or your go to implementation or for all of your "smart features" to break as soon as your code stops compiling. You need scry the most when you're in the middle of a refactor: when your code doesn't compile and you need to navigate it to find out why. And that is why scry isn't useful.

And finally wrong 😅 "your code stops compiling. You need scry the most when you're in the middle of a refactor". Scry never stops, this tool is resilient to compiler failures on crystal code 💪 "You need scry the most when you're in the middle of a refactor: when your code doesn't compile and you need to navigate it to find out why. And that is why scry isn't useful." We're already trying to execute scry request concurrently, also scry does support CancelRequest to stop slow requests or cancel failed responses 🚀

@faustinoaq
Copy link
Member Author

faustinoaq commented May 6, 2018

So, finally I still think crystal tool references would be very useful for scry and crystal devs 😉

@RX14
Copy link

RX14 commented May 6, 2018

@faustinoaq I don't think you understood my point. Scry keeps running when your code doesn't compile, but does it return results? If not, it's fairly pointless.

@faustinoaq
Copy link
Member Author

Scry keeps running when your code doesn't compile, but does it return results?

Yep, if the code doesn't compile then you get a diagnostic with problems to fix :-)

By example, if you want to format your code and you have syntax errors, you will get an error diagnostic first, then you fix the error in that file, and the format feature is working again.

@faustinoaq
Copy link
Member Author

Same with implementations, symbols, hover and rename, except completion, code completion is resilient to errors.

@faustinoaq
Copy link
Member Author

References feature would behave same way

@RX14
Copy link

RX14 commented May 6, 2018

@faustinoaq you're missing my point again. I don't want to get the compile error back. I want to get the same result as if the code compiled. If you go use a java IDE, and you have 200+ compile errors in your file, it will STILL give you completions (it won't tell you to fix your code), it will STILL give you all the refactorings and goto definition/goto usages. It won't tell you to gix fix your compile errors, it'll work anyway. What I mean by scry breaks is that it doesn't give you the same results when your code doesn't compile as when it does compile. And that makes it useless.

If i'm editing code, it's either because it's broken or incomplete: either way it'll generate a compile error. I don't need jump to definition/usages unless i'm editing code. So the only time I actually need the damn thing to work it's broken.

@faustinoaq
Copy link
Member Author

faustinoaq commented May 6, 2018

I want to get the same result as if the code compiled. If you go use a java IDE, and you have 200+ compile errors in your file, it will STILL give you completions (it won't tell you to fix your code)

Yep, scry can do that 😉

@RX14
Copy link

RX14 commented May 6, 2018

The way you described it working implied that that can't work.

@faustinoaq
Copy link
Member Author

Yep, because it depends on the feature, completion and workspace symbols are resilent to errors, although, document symbols, formatting, implementations and hover, are based on crystal tool

However, scry already uses dir scopes based on .scry_main.cr file to get these features working even if you have 200+ compile errors in your workspace. Also, if you have diagnostic issues in some file and you want to use features based on crystal tool inside that file, then you can comment the issues to get the feature working.

Right now I know scry have some limitations (because is based on crystal tool), but I think this project is getting better everyday thank to @crystal-lang-tools/scry team 💯

Perhaps in the future we can make crystal tool resilient to errors as well, like a --ignore-errors flag WDYT? 😉

So, again, I still think at least some support for Crystal::References or some workaround would be very useful 👍

@faustinoaq
Copy link
Member Author

faustinoaq commented May 6, 2018

The way you described it working implied that that can't work.

@RX14 see my comment: (above your comment)

Same with implementations, symbols, hover and rename, except completion, code completion is resilient to errors.

#104 (comment)

Thanks to @laginha87 🎉

@faustinoaq
Copy link
Member Author

Oh, BTW using full compiler/crystal/** won't solve the crystal tool issue (just brings more issues), so #53 was good anyway. 😅

I think a flag like --ignore-errors for crystal tool would be really nice WDYT?

Something like Java: "give me all, doesn't matter errors..." 😎

@RX14
Copy link

RX14 commented May 6, 2018

Well that's certainly better than nothing I guess. I hope you can expand the other tools to be resilient to compile errors too. --ignore-errors in the compiler is not going to happen so i'd suggest reimplementing a lot of this logic in scry. Finding out the type of a variable when there's compile errors is far harder in crystal than other languages.

@faustinoaq
Copy link
Member Author

Yep, I think scry is a nice start and I guess --ignore-errors is very hard to implement in the compiler, so, maybe we can inject "ghost" code snippets to fix the code and ensure crystal tools to work properly.

Like this:

if build_failure?
  try_to_fix_code # Maybe, even commenting out the error lines
else
  return_response
end

WDYT?

@faustinoaq
Copy link
Member Author

faustinoaq commented May 6, 2018

I think this would need a special tool from the compiler, similar to tool implementation. We should be able to find the references by looking at the instantiations of a specific method (the compiler does that internally at some point)

@asterite @RX14 Do you know when/where the compiler does that? 😅

Ref: #104 (comment)

@bcardiff bcardiff closed this as completed Jun 1, 2023
Crystal Tools automation moved this from To do to Done Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Crystal Tools
  
Done
Development

No branches or pull requests

4 participants