`ImplicitWebAnnotationNames` removes `@RequestParam` argument even when `-parameters` is missing

[sambsnyd]

After running the spring boot 3.4 migration some Spring @RequestParam annotations had their arguments removed, which caused IllegalStateException during a subsequent test pass.

So in a function like:

@GetMapping
String foo(@RequestParam("a") String a) { ...}

Some part of the migration was changing it to:

@GetMapping
String foo(@RequestParam String a) { ...}

This would be fine if the compiler configuration included the -parameters argument, but it didn't, so this change is erroneous.

The recipe responsible is org.openrewrite.java.spring.ImplicitWebAnnotationNames. In the short term I'm going to remove it from spring boot best practices. Ideally it would be fixed to only make its change if -parameters is part of the compiler configuration.

[timtebeek]

Hmm; typically we've used the best practices recipes as going above and beyond what a mere upgrade recipe would do.

rewrite-spring/src/main/resources/META-INF/rewrite/best-practices.yml

Lines 19 to 29 in 30da14a

	name: org.openrewrite.java.spring.boot2.SpringBoot2BestPractices
	displayName: Spring Boot 2.x best practices
	description: Applies best practices to Spring Boot 2 applications.
	tags:
	- spring
	- boot
	recipeList:
	# Note that we do not upgrade to Spring Boot 2.x here, as the 2.0 recipe includes SpringBoot2BestPractices itself
	- org.openrewrite.java.spring.NoRequestMappingAnnotation
	# https://github.com/openrewrite/rewrite-spring/issues/736
	# - org.openrewrite.java.spring.ImplicitWebAnnotationNames

But in this case the 2.x best practices are applied as part of UpgradeSpringBoot_2_0

rewrite-spring/src/main/resources/META-INF/rewrite/spring-boot-20.yml

Line 85 in 30da14a

- org.openrewrite.java.spring.boot2.SpringBoot2BestPractices

Would you be ok with adding ImplicitWebAnnotationNames to SpringBoot3BestPracticesOnly where it's a more deliberate choice? You'll note there's other changes there that are optional.

rewrite-spring/src/main/resources/META-INF/rewrite/best-practices.yml

Lines 47 to 61 in 30da14a

	name: org.openrewrite.java.spring.boot3.SpringBoot3BestPracticesOnly
	displayName: Spring Boot 3.3 best practices (only)
	description: Applies best practices to Spring Boot 3 applications, without chaining in upgrades to Spring Boot.
	tags:
	- spring
	- boot
	recipeList:
	# These steps go above & beyond what's needed for a pure upgrade, and are not be included with 3.x upgrades themselves
	- org.openrewrite.java.spring.boot3.EnableVirtualThreads
	- org.openrewrite.java.spring.framework.BeanMethodsNotPublic # Intentionally not in 2.x
	# Replace literals with constants and simplify MediaType parse calls
	- org.openrewrite.java.spring.http.ReplaceStringLiteralsWithHttpHeadersConstants
	- org.openrewrite.java.spring.http.ReplaceStringLiteralsWithMediaTypeConstants
	- org.openrewrite.java.spring.http.SimplifyMediaTypeParseCalls
	- org.openrewrite.java.spring.http.SimplifyWebTestClientCalls

Making the recipe safe by default would be best, but I'm not sure we'd reliably be able to determine if -parameters is set in any number of ways. Perhaps you'd want to apply only when detected to be safe then?