Replacing field injection with constructor injection

[crankydillo]

What problem are you trying to solve?

We want to convert a bunch of classes that used Spring field injection to constructor injection. We are willing to live with some caveats (e.g. only works if no constructors, select annotations, etc).

Describe the situation before applying the recipe

class A {
   @Autowired
   @Qualifier("B")
    private B b;
}

Describe the situation after applying the recipe

class A {
    private B;

    public A(@Qualifier("B") b) {
        this.b = b;
    }
}

Any additional context

We could likely get value from something that didn't hit all the corner cases. However, if there are a bunch of corner cases that ultimately means an unending feature request on this, it's not like we really need this.

Are you interested in contributing this recipe to OpenRewrite?

Yes.

[timtebeek]

Hi @crankydillo ; Thanks for the offer to help! Agree that this would be a nice addition, and best to start with a limited set of known cases, and reject anything else (at first) that might make it more complicated.

On the known cases to reject I'd include

• has existing explicit constructor
  • could lead to ambiguity
• has implicit Lombok provided constructor
• has fields annotated with more than just @Autowired, like validation or jackson annotations
  • as not all annotations are equally valid on constructor arguments as they are on fields
• class is extends or is extended by another class
  • could lead to failure calling super()
• class has more than say ~10 autowired fields
  • arbitrary cut off, but very large constructors won't work as well

What we're left with then is

• introduce constructor with arguments in same order as fields
• remove autowired annotations from fields
• move any qualifier annotations to arguments

Anything you'd add to this list?

[crankydillo]

RE field->constructor annotations, I was thinking we could move @Autowired and @Autowired + @Qualifier. I know for us that would handle quite a few cases.

I might still do large constructors or maybe make that part configurable. The idea is to create an obvious code smell, which is currently hidden by 'Spring magic'.

[Pankraz76]

When is this coming? Quick fixes applied by this recipe would be very helpful in reducing the boilerplate.

[timtebeek]

hi @punkratz312 ; we don't have a fixed schedule, but you're welcome to push up a draft PR with tests if you'd like to help out. :)

[timtebeek]

Adding a note here that this is also necessary when you want to move away from @org.springframework.beans.factory.annotation.Required, which was removed in Spring Framework 6+.

[martinlippert]

We have this as a quick fix in the Spring Tools for VSCod and Eclipse: https://x.com/springtools4/status/1843922035824787896

It uses org.openrewrite.java.spring.AutowiredFieldIntoConstructorParameterVisitor from rewrite-spring , so improvements to this (e.g. handling the @Qualifier annotation nicely) should be made there, I think.

[martinlippert]

@crankydillo Interested in contributing some improvements to the org.openrewrite.java.spring.AutowiredFieldIntoConstructorParameterVisitor, for example to cover @Qualifier annotations in a nice way?

[crankydillo]

@martinlippert If I can squeeze out some time. Honestly, I didn't know that class you mentioned existed. Wouldn't you say that meets at least the title of this issue? I can change the title to specify we are asking for additional things like @Qualifier. WDYT?

[martinlippert]

Yeah, probably changing the title to something around taking @Qualifier annotations into account makes sense to me.

We could also take a look, but if you plan to contribute this, we would not go down this path to avoid duplicate work (and vice versa). Just let us know whether you plan to work on this or not - both is totally fine.

[martinlippert]

Also adding @BoykoAlex for awareness.

[timtebeek]

Adding two relevant links here, as rewrite-spring for now lacks a recipe; we only have the visitor Alex provided in the past:

• https://github.com/spring-projects/sts4/blob/0f335b26e9c9faaa8759a66ee38a21471288e5e6/headless-services/commons/commons-rewrite/src/main/java/org/springframework/ide/vscode/commons/rewrite/java/ConvertAutowiredFieldIntoConstructorParameter.java#L39
• https://github.com/spring-projects/sts4/blob/0f335b26e9c9faaa8759a66ee38a21471288e5e6/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/reconcilers/AutowiredFieldIntoConstructorParameterReconciler.java#L138

Both are copyrighted under Eclipse Public License; not immediately clear to me if we could adopt similar in rewrite-spring.

[martinlippert]

The reconciler is very specific for the Spring Tools language servers reconciling mechanism and based on ASTs from JDT, so I don't see any value in this for the rewrite-spring repo - as far as I understand.

A recipe itself might make sense for rewrite-spring, but I let @BoykoAlex fill in the details here.

[BoykoAlex]

The first link would be useful for the recipe implementation. Essentially it is a recipe taking two parameters: owner class fully qualified name and the name of the field in the class to convert into the constructor parameter. A recipe converting all autowired fields into constructor parameters would have to be written differently. Scan for autowired fields and then call doAfterVisit(new AutowiredFieldIntoConstructorParameterVisitor(classFqName, fieldName)); for each found field.

[knutwannheden]

@jetbrains-junie The description and comments on this issue provide a lot of good comments. Particularly expanding org.openrewrite.java.spring.AutowiredFieldIntoConstructorParameterVisitor sounds like a good idea.

[jetbrains-junie[bot]]

Hey, it’s Junie by JetBrains! I started processing your request 🚀

[jetbrains-junie[bot]]

A new recipe was implemented to convert Spring field injection to constructor injection, handling @Autowired and @Qualifier annotations. The tests initially failed due to formatting issues and handling of existing constructors, but modifications were made to address these problems. The updated recipe is now being tested in hopes of achieving the desired functionality correctly.