eclipse-m2e / m2e-core

Eclipse Public License 2.0
113 stars 116 forks source link

Quick-fix on Java editor to suggest dependecies on missing classes #623

Open laeubi opened 2 years ago

laeubi commented 2 years ago

Lets say I have created a new module in my workspace with a simple archetype and thus no dependencies declared.

What would be really useful: I get a quickfix offering me to add a dependency to my pom from the above choices.

mickaelistria commented 2 years ago

That's a good idea. Since this is more about Java editor than pom.xml, it's not something we can have in lemminx-maven, but instead something that would fit in org.eclipse.m2e.jdt bundle.

plaird commented 2 years ago

This is the org.eclipse.jdt.ui.classpathFixProcessors extension point (provide a subclass of DefaultClasspathFixProcessor). In the Bazel Eclipse plugin, we do something similar to this idea. But we have an index of the total set of jars available in the workspace, and can provide the Bazel label (serves the role of Maven gav) to add to the build . I don't know how m2e would be able to suggest the right GAV just based on classname or even jar but maybe it can be derived from the default proposal generated by JDT.

HannesWell commented 2 years ago

Thank you for the suggestion.

I don't know how m2e would be able to suggest the right GAV just based on classname or even jar but maybe it can be derived from the default proposal generated by JDT.

The Maven-Central Search API provides means to get the GAV of all artifacts that contain a class with given classname or fully-qualified classname, so in general this would be feasible. However the number of results could become very high if the name is popular. In that case we can only hope that the 'right' artifact is in the beginning of the list.

laeubi commented 2 years ago

@plaird I tried this out but it seems this extension point is only used when you select "Fix Project Setup ..." in an extra dialog but I actually want it at the qucikfix where it is mentions to "Create Class missing" and alike ...

mickaelistria commented 2 years ago

wouldn't a markerResolution extension suffice here? II guess that answer is "not entirely" because JDT doesn't use marker until file is saved, but maybe it would be a good 1st increment.

laeubi commented 2 years ago

Yes that seems how it is handled by PDE... its just a bit odd to have a org.eclipse.jdt.ui.classpathFixProcessors that seem to handle this case already and then have to implement something else also... I assume this was from while back where proposals are not allowed to run asyncronous and I might do some more heavy computation here but even thats not clear here (e.g. one don't get a ProgressMonitor ...).

fbricon commented 2 years ago

FYI that very feature existed pre m2e-1.0. It was removed, for some reason, before the project was contributed to the Eclipse Foundation. I've seen the code in the past in a branch of the m2e git repo, but looks like the old branches have been deleted. It might still exist in someone's fork somewhere.

mickaelistria commented 2 years ago

FYI that very feature existed pre m2e-1.0. It was removed, for some reason, before the project was contributed to the Eclipse Foundation. I've seen the code in the past in a branch of the m2e git repo, but looks like the old branches have been deleted. It might still exist in someone's fork somewhere.

I think a more modern version would use search.maven.org while previous implementation was using index; so they may not be that much to reuse. Or maybe we're not talking about external deps but instead of looking up inside the workspace for the missing class? Both actually seem to be relatively different story, maybe worth splitting it into 2 dedicated tickets.

Side-question: can those markerResolution be reused by JDT-LS? If not, are you aware of the right way to submit a quick-fix that can serve both Eclipse IDE and JDT-LS?

laeubi commented 2 years ago

@fbricon I think you can see al branches here: https://github.com/eclipse-m2e/m2e-core/branches/all

Or maybe we're not talking about external deps but instead of looking up inside the workspace for the missing class?

Yes that something I like to implement as a first step ..

laeubi commented 2 years ago

I'm making some progress, at least the quickfix is now shown!

grafik

fbricon commented 2 years ago

For posterity, this is the commit that deleted that feature initially (based on index-search): https://github.com/tesla/m2e-core-tests/commit/1fcfd033a949aa6c669ffe18b5d885a26922814d

dstango commented 1 year ago

I don't know if this is related, but in Eclipse 2022-12 I get a proposal to add a maven dependency, which is quite annoying:

image

This quick fix is not appropriate, as "l" is not a class, but a variable that has not been declared.

I sometimes like to type in my variables this way -- typing their name and assign an expression and let then eclipse add in the declaration of the type by pressing ctrl+1 and return. This doesn't work any more :-(, as the dependency proposal gets in the way.

Although I don't feel it to be an appropriate proposal in this scenario, it might be sufficient to put the proposal further down. I guess this function is used as often als the other options, so I'd consider it sensible to move it further down.

laeubi commented 1 year ago

This quick fix is not appropriate, as "l" is not a class, but a variable that has not been declared.

DO you like to propose a patch?

I sometimes like to type in my variables this way -- typing their name and assign an expression and let then eclipse add in the declaration of the type by pressing ctrl+1 and return.

CTRL+2+L should immediately do this but it seems broken (at least under Linux)

dstango commented 1 year ago

This quick fix is not appropriate, as "l" is not a class, but a variable that has not been declared.

DO you like to propose a patch?

I never worked on m2e, so I don't know my way around. If you can give me a hint where I might look, I can see, if I get my head around it -- maybe changing the priority of the suggestion could be a first approximation.

I sometimes like to type in my variables this way -- typing their name and assign an expression and let then eclipse add in the declaration of the type by pressing ctrl+1 and return.

CTRL+2+L should immediately do this but it seems broken (at least under Linux)

yes, I know and use that shortcut, too. Yet the scope of Ctrl+2+L seems to be different - typing it after the final ";" of a line doesn't invoke it, if I'm correct. And habits are strong ... ;-) ... Anyhow Ctrl+1 usually has a good first proposal for many situations, so that's where I often go. Most of the proposed changes of Ctrl+1 are local in the file I'm editing -- the inappropriate change of the pom.xml (different file) though behaves not as friendly with Ctrl-Z, it seems (yet I didn't double-check this).