Is it missing the Push Down Method refactoring?

[osmarleandro]

Summary

In the source code of osmarleandro/spring-boot@2787fad commit, I applied a single Push Down Method refactoring to customize() method in ManagementWebServerFactoryCustomizer class.

RefDiff detects a single Move Method refactoring, where the customize() method was moved to ServletManagementChildContextConfiguration class.

But, the ServletManagementChildContextConfiguration class has a nested class called ServletManagementWebServerFactoryCustomizer, which extends the ManagementWebServerFactoryCustomizer and receive the customize() method from the Push Down Method operation.

Code example

Diff fragment between the commit osmarleandro/spring-boot@2787fad and their parent.

@@ -61,22 +60,7 @@ public abstract class ManagementWebServerFactoryCustomizer<T extends Configurabl
-       @Override
-       public final void customize(T factory) {
-               ManagementServerProperties managementServerProperties = BeanFactoryUtils
-                               .beanOfTypeIncludingAncestors(this.beanFactory, ManagementServerProperties.class);
-               // Customize as per the parent context first (so e.g. the access logs go to
-               // the same place)
-               customizeSameAsParentContext(factory);
-               // Then reset the error pages
-               factory.setErrorPages(Collections.emptySet());
-               // and add the management-specific bits
-               ServerProperties serverProperties = BeanFactoryUtils.beanOfTypeIncludingAncestors(this.beanFactory,
-                               ServerProperties.class);
-               customize(factory, managementServerProperties, serverProperties);
-       }

@@ -126,6 +128,21 @@ class ServletManagementChildContextConfiguration {
                static class ServletManagementWebServerFactoryCustomizer
			extends ManagementWebServerFactoryCustomizer<ConfigurableServletWebServerFactory> {
                [...] 
+               @Override
+               public final void customize(ConfigurableServletWebServerFactory factory) {
+                       ManagementServerProperties managementServerProperties = BeanFactoryUtils
+                                       .beanOfTypeIncludingAncestors(this.beanFactory, ManagementServerProperties.class);
+                       // Customize as per the parent context first (so e.g. the access logs go to
+                       // the same place)
+                       customizeSameAsParentContext(factory);
+                       // Then reset the error pages
+                       factory.setErrorPages(Collections.emptySet());
+                       // and add the management-specific bits
+                       ServerProperties serverProperties = BeanFactoryUtils.beanOfTypeIncludingAncestors(this.beanFactory,
+                                       ServerProperties.class);
+                       customize(factory, managementServerProperties, serverProperties);
+               }
+
        }

Environment details

RefDiff 2.0

Steps to reproduce

1. Run RefDiff and pass as input the commit osmarleandro/spring-boot@2787fad.

Actual results

MOVE	{Method customize(T) at spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/server/ManagementWebServerFactoryCustomizer.java:64}	{Method customize(ConfigurableServletWebServerFactory) at spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/ServletManagementChildContextConfiguration.java:131})

Expected results

A single instance of the Push Down Method refactoring applied to customize() method in ManagementWebServerFactoryCustomizer class.

[danilofes]

This is a limitation of our implementation. When looking for PUSH_DOWN candidates, we enforce the signature of the method is the same. However, we don't deal with the case of replacing a parameterized type with a concrete type:

• void customize(T factory)
• void customize(ConfigurableReactiveWebServerFactory factory)
  Thus, this refactoring ends up beeing incorrectly classified as a MOVE, as we allow changes in method signature when detecting MOVE..

[osmarleandro]

Hi @danilofes, thanks for your attention and support.

Considering the Java plugin, can the implementation be improved to detect generic types?