openrewrite / rewrite-spring

OpenRewrite recipes for Spring projects.
Apache License 2.0
254 stars 75 forks source link

Replacing field injection with constructor injection #557

Open crankydillo opened 2 months ago

crankydillo commented 2 months ago

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 commented 2 months ago

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

What we're left with then is

Anything you'd add to this list?

crankydillo commented 2 months ago

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'.

punkratz312 commented 1 week ago

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

timtebeek commented 1 week ago

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 commented 1 week ago

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 commented 6 days ago

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 commented 4 days ago

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