Add magic number quick fix

[karlvr]

I think this will conflict with the other quick fix when merged. I'm happy to rebase this PR when the time comes. I might do other quick fixes together in one PR, assuming that you're generally happy with any additional quick fix implementations!

[jdneo]

😃 Thanks for all the great contributions! I'll try my best to review them when I have time.

Just feel free to send PRs if you want. I would suggest one quick fix per PR, it will be easier for me to review

[jdneo]

Great work!

While reviewing this PR. I found a new problem which is on the Java Language server side maybe.

I found that when the Checkstyle extension is disabled. User can find refactors like extract to constant at the code like Thread.sleep(1000).

But when the Checkstyle extension is enabled and report the magic number violation, refactors like extract ... from the Java Language Server disappear when right click on the 1000.

I'm now trying to figure out the root cause.

[karlvr]

@jdneo It would be great, by the by, to be able to trigger the extract to constant command. Or actually, it feels like once we've recovered the extract to constant quick fix that we don't need a magic number quick fix at all!

[jdneo]

Ok it turns out that the code here: https://github.com/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/RefactorProcessor.java#L140

It skips a lot of refactors when the location has errors, but it does not check the source(reporter) of the errors.

So a fix could be, we need to take the error sources into consideration (This information is contained in the LSP body). Only consider it as errored when it do have errors come from the Java Language Server itself.

I'll raise a PR to fix this in the Language Server side.

it feels like once we've recovered the extract to constant quick fix that we don't need a magic number quick fix at all!

Yes the extract to constant refactor can definitely solve the magic number violation (I guess that's why the Eclipse Checkstyle plugin does not have such quickfix). One potential gap is that, the extract to constant is in the refactor list instead of the quickfix list. So the user can only find it in the yellow bulb, but no linked quickfix for this error.

[jdneo]

Here it is: eclipse-jdtls/eclipse.jdt.ls#1643

[karlvr]

@jdneo great investigation. If it's practical to trigger the refactor from the checkstyle plugin, maybe that's how to implement this quick fix. I have no idea whether that's possible!

[jdneo]

@karlvr Seems not because the quickfix and refactor are two different kinds of Code Actions in VS Code, so probably no way to link them.

My suggestion is wait a little bit before we add the magic number quick fix into the extension. If we see more users request for this support, that can be a good chance to merge this PR. What do you think?

[karlvr]

@jdneo I'm absolutely happy with that... it's a shame that we can't trigger the refactor. Maybe that's another enhancement to the bigger Java project to expose its functionality for other extensions to trigger :-) Definitely outside of my desired scope.

[jdneo]

Mark this PR to backlog for now.