Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

various OpenRewrite based quick fixes are broken #1126

Closed
martinlippert opened this issue Oct 19, 2023 · 16 comments
Closed

various OpenRewrite based quick fixes are broken #1126

martinlippert opened this issue Oct 19, 2023 · 16 comments
Assignees

Comments

@martinlippert
Copy link
Member

martinlippert commented Oct 19, 2023

I have a code block in a class that looks like this:

	@Bean
	public String someBean() {
		return "somebean";
	}

The public is underlined, the quick fix to remove the public modifier appears (three variants), but executing the first variant to remove the public modifier at this location results in:

            @Bean
            String someBean() {
		return "somebean";
	}

(the indentation is wrong)

This happens when I execute the quick fix by clicking on it. Executing the quick fix by pressing Enter results in an error popup to show up and the Enter key press to be inserted into the editor:

	@Bean
	
	public String someBean() {
		return "somebean";
	}

The error on the language server side looks like this:

18:05:29.120 [ForkJoinPool.commonPool-worker-4] ERROR o.e.lsp4j.jsonrpc.RemoteEndpoint - Internal error: java.lang.NullPointerException: Property must not be null: edit
java.util.concurrent.CompletionException: java.lang.NullPointerException: Property must not be null: edit
	at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:332)
	at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:347)
	at java.base/java.util.concurrent.CompletableFuture$UniAccept.tryFire(CompletableFuture.java:708)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture.completeExceptionally(CompletableFuture.java:2162)
	at reactor.core.publisher.MonoToCompletableFuture.onError(MonoToCompletableFuture.java:77)
	at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onError(FluxOnAssembly.java:544)
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:136)
	at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onNext(FluxOnAssembly.java:539)
	at reactor.core.publisher.MonoCompletionStage$MonoCompletionStageSubscription.apply(MonoCompletionStage.java:125)
	at reactor.core.publisher.MonoCompletionStage$MonoCompletionStageSubscription.apply(MonoCompletionStage.java:71)
	at java.base/java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:934)
	at java.base/java.util.concurrent.CompletableFuture$UniHandle.tryFire(CompletableFuture.java:911)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1773)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1760)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
Caused by: java.lang.NullPointerException: Property must not be null: edit
	at org.eclipse.lsp4j.util.Preconditions.checkNotNull(Preconditions.java:29)
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Assembly trace from producer [reactor.core.publisher.MonoFlatMap] :
	reactor.core.publisher.Mono.flatMap(Mono.java:3100)
	org.springframework.ide.vscode.commons.languageserver.util.SimpleLanguageServer.executeCommand(SimpleLanguageServer.java:318)
Error has been observed at the following site(s):
	*__Mono.flatMap ⇢ at org.springframework.ide.vscode.commons.languageserver.util.SimpleLanguageServer.executeCommand(SimpleLanguageServer.java:318)
Original Stack Trace:
		at org.eclipse.lsp4j.util.Preconditions.checkNotNull(Preconditions.java:29)
		at org.eclipse.lsp4j.ApplyWorkspaceEditParams.<init>(ApplyWorkspaceEditParams.java:41)
		at org.eclipse.lsp4j.ApplyWorkspaceEditParams.<init>(ApplyWorkspaceEditParams.java:45)
		at org.springframework.ide.vscode.commons.languageserver.util.SimpleLanguageServer.lambda$executeCommand$3(SimpleLanguageServer.java:319)
		at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:132)
		at reactor.core.publisher.MonoCompletionStage$MonoCompletionStageSubscription.apply(MonoCompletionStage.java:125)
		at reactor.core.publisher.MonoCompletionStage$MonoCompletionStageSubscription.apply(MonoCompletionStage.java:71)
		at java.base/java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:934)
		at java.base/java.util.concurrent.CompletableFuture$UniHandle.tryFire(CompletableFuture.java:911)
		at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
		at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1773)
		at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1760)
		at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
		at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
		at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
		at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
		at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)

The convert autowired field to constructor param is broken as well, results in weird source code content.

All this happens using the latest nightly builds for STS 4.20.1 on Eclipse 4.29. It all works in the 4.20.0 release. I haven't tried the latest nightly builds in VSCode yet.

@BoykoAlex
Copy link
Contributor

Ouch! Thanks for bringing this up. I was just trying this stuff with native image yesterday and thought it is a problem with native image... I suspect now it is something changed in OR dependencies we consume

@martinlippert
Copy link
Member Author

test results from trying things in VSCode:

  • the broken indentation for removing the public is broken in VSCode as well
  • the exceptions from the language server seem to be caused by the press of the return key being inserted into the editor/document and then parsing screws up. The autowired field conversation works nicely in VSCode - and it works nicely in Eclipse when using the mouse to activate the quick fix (instead of the return key)

@martinlippert
Copy link
Member Author

So it looks to me like the indentation problem is a problem with the OR recipe, whereas the other issues could be caused by LSP4E dealing with the quick fix selection via the keyboard in a weird way. But this is just guessing.

@BoykoAlex
Copy link
Contributor

If you apply the quickfix for removing public modifier to file it works fine... must be something about my attempt to apply recipe to a subtree of CU...

@martinlippert
Copy link
Member Author

If you apply the quickfix for removing public modifier to file it works fine... must be something about my attempt to apply recipe to a subtree of CU...

Plus the editor taking the enter key press as input (in addition to executing the quick fix)

@BoykoAlex
Copy link
Contributor

The editor consumed return key press is fixed with: 7361713

@BoykoAlex
Copy link
Contributor

The issue with the indentation is openrewrite/rewrite#3641

@BoykoAlex
Copy link
Contributor

Should be fixed the 4.20.1 release. Will double-check and if everything is working in RCs close this and open a new issue for the indentation only to fix in OR

@BoykoAlex
Copy link
Contributor

Fixed for 4.20.1. The rest to be addressed in #1135

@bohni
Copy link

bohni commented Nov 3, 2023

I still get the NullpointerException with Quickfix (Fresh Install of Eclipse 2023-09 and STS 4.20.1 via Marketplace)

Edit 2023-11-03 14:08: Tried current nightly (https://dist.springsource.com/snapshot/TOOLS/sts4/nightly/e4.29) and NullpointerException is still thrown.

java.util.concurrent.CompletionException: java.lang.NullPointerException: Property must not be null: edit
{
    "jsonrpc": "2.0",
    "id": "14",
    "method": "workspace/executeCommand",
    "params": {
        "command": "sts.vscode-spring-boot.codeAction",
        "arguments": [
            "org.openrewrite.rewrite",
            {
                "recipeId": "org.openrewrite.java.spring.security5.AuthorizeHttpRequests",
                "recipeScope": 1.0,
                "docUris": [
                    "file:///[...]MyConfig.java"
                ],
                "label": "Replace with AuthorizeHttpRequestsConfigurer and use \u0027HttpSecurity.authorizeHttpRequests(...) and related types in file",
                "typeStubs": [],
                "preferred": false
            }
        ]
    }
}
public class MyConfig
{
	@Bean
	Customizer<AuthorizeHttpRequestsConfigurer<HttpSecurity>.AuthorizationManagerRequestMatcherRegistry> authorizeHttpRequestsCustomizer()
	{

@BoykoAlex

@BoykoAlex
Copy link
Contributor

@bohni might be an issue with this specific quickfix - I'll have a look

@BoykoAlex
Copy link
Contributor

@bohni Can you paste your entire MyConfig source contents with the comment near the underlined code to which quickfix is about to be applied? The method header doesn't have a quickfix... not for me at lease...

@bohni
Copy link

bohni commented Nov 3, 2023

@BoykoAlex I will do on Monday, when I'm back at work.

Thx for investigating.

@bohni
Copy link

bohni commented Nov 6, 2023

@BoykoAlex

Edit: Quick fix does only show up, if class is not in default package...
Edit2: Simplified code

package net.bohni;

import java.util.stream.Stream;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.PropertySource;
import org.springframework.scheduling.annotation.EnableAsync;
import org.springframework.security.config.Customizer;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configurers.AuthorizeHttpRequestsConfigurer;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;

@Configuration
@PropertySource(value= { "classpath:env.properties", "classpath:my-service.properties" }, ignoreResourceNotFound=true)
@EnableAsync
public class MyConfig
{
	@Bean
	Customizer<AuthorizeHttpRequestsConfigurer<HttpSecurity>.AuthorizationManagerRequestMatcherRegistry> authorizeHttpRequestsCustomizer()
	{
		return null;
	}
}

image

Both entries lead to the same error:
image

@BoykoAlex
Copy link
Contributor

@bohni Lets move the issue above in a dedicated issue: #1141. I have a feeling this never worked or wasn;t marked previously... Do you recall having this issue in 4.19.1?

@bohni
Copy link

bohni commented Nov 7, 2023

It was not marked with the version I ran before.
I did not check the version number, but looking at the changelog and the release dates, it should have been 4.19.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants