Add CFR class file viewer #3247
Conversation
|
Timeout on |
|
FTR, the way I actually used CFR (I don't have it installed) is via http://www.javadecompilers.com/... This looks really helpful. How does one use it? |
|
I've added it into the "Metals Analyse Source" file context menu to go with the other file analysis tools.
|
There was a problem hiding this comment.
@Arthurm1 Thx for this new decoder!
Mostly looks good, I have a bit of minor comments/questions.
I haven't added any Java tests to FileDecoderProviderLspSuite because I don't think didOpen triggers a java file compilation with Metals yet, so it won't compile a single Java file and therefore can't test the analysis of the output - or am I wrong?
I've double-checked that - didOpen triggers compilation for java files.
metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala
Outdated
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala
Outdated
Show resolved
Hide resolved
|
@dos65 I was looking in the wrong area for class files. I've now added tests for java files and for missing files leading to errors. |
Arthurm1
force-pushed
the
class_file_viewer
branch
from
d8de58f
to
439ec9e
Compare
| } catch { | ||
| case NonFatal(e) => | ||
| scribe.error(e.toString()) | ||
| Future.successful(DecoderResponse.failed(path.toURI, e)) | ||
| } | ||
| } | ||
|
|
||
| private def decodeCFRFromClassFile( |
There was a problem hiding this comment.
Could we run CFR using ShellRunner and download the dependency on demand? This would mean we wouldn't need to add the additional dependency to Metals.
I see there is a main that can be used https://github.com/leibnitz27/cfr/blob/9f5a97cf76d94d3ac695887abc6bbfb740c70a9c/src/org/benf/cfr/reader/Main.java
There was a problem hiding this comment.
Could do. It's only 2MB but I guess it all adds up. Is there someplace where this is already done - where I can nick the code?
There was a problem hiding this comment.
Yeah, it all adds up and especially if it's a tool used by more advanced developers this might not be needed to download at all
That should be for example in the NewProjectProvider, it uses gitter8 by downloading it and running with the current java home.
There was a problem hiding this comment.
There was a problem hiding this comment.
Done - ShellRunner#runJava made this very easy to change.
Adds a class file decompiler to the list of source analysis tools
@s5bug put me onto this while discussing an issue with Scala 3. It was easier to understand what was going on than using
javapso I thought it might help to integrate it into MetalsThis includes a fix for using
javapon java source files - it was looking in the wrong dir for the class file. I haven't added any Java tests toFileDecoderProviderLspSuitebecause I don't thinkdidOpentriggers a java file compilation with Metals yet, so it won't compile a single Java file and therefore can't test the analysis of the output - or am I wrong?Other options besides CFR are fernflower and procyon. I figure if CFR isn't good enough then users can raise issues and other decompilers can be added/substituted.
I imagine this will also be handy for implementing scalameta/metals-feature-requests#210
@ckipp01 Hopefully this is easy to add to VI. In the VSCode PR I've also setup the
.cfrfile extension to use Java textmate grammar. That's built into VSCode - hence I've just amended the language contribution point to use an additional extension.License question: Do I have to include a reference to CFR in
NOTICE.md? I've added a dependency on CFR but not included source code.