ponder-lab / migrate-skeletal-implementation-to-interface-refactoring

A refactoring prototype plug-in for Eclipse that migrates Java skeletal implementations to enhanced interfaces.

Migrate Skeletal Implementation to Interface Refactoring

Screenshot

Demonstration

(click to view)

Introduction

The skeletal implementation pattern is a software design pattern consisting of defining an abstract class that provides a partial interface implementation. However, since Java allows only single class inheritance, if implementers decide to extend a skeletal implementation, they will not be allowed to extend any other class. Also, discovering the skeletal implementation may require a global analysis. Java 8 enhanced interfaces alleviate these problems by allowing interfaces to contain (default) method implementations, which implementers inherit. Java classes are then free to extend a different class, and a separate abstract class is no longer needed; developers considering implementing an interface need only examine the interface itself. Both of these benefits improve software modularity.

This prototype refactoring plug-in for Eclipse represents ongoing work in developing an automated refactoring tool that would assist developers in taking advantage of the enhanced interface feature for their legacy Java software.

Usage

Currently, the prototype refactoring works only via the package explorer and the outline views (see issues #2 and #65). You can either select a single method to migrate or select a class, package, or (multiple) projects. In the latter case, the tool will find methods in the enclosing item(s) that are eligible for migration.

Installation

A beta version of our tool is available via an Eclipse update site at: https://raw.githubusercontent.com/ponder-lab/Migrate-Skeletal-Implementation-to-Interface-Refactoring/master/edu.cuny.citytech.defaultrefactoring.updatesite. Please choose the latest version.

You may also install the tool via the Eclipse Marketplace by dragging this icon to your running Eclipse workspace: .

Limitations

The research prototype refactoring is conservative. While tool should not produce any type-incorrect or semantic-inequivalent code, it may not refactor all code that may be safe to refactor.

Contributing

See the contribution guide.

Publications

Raffi Khatchadourian and Hidehiko Masuhara. Proactive empirical assessment of new language feature adoption via automated refactoring: The case of Java 8 default methods. In International Conference on the Art, Science, and Engineering of Programming, volume 2 of Programming '18, pages 6:1--6:30. AOSA, March 2018. [ bib | DOI | http ]

Raffi Khatchadourian and Hidehiko Masuhara. Defaultification refactoring: A tool for automatically converting Java methods to default. In International Conference on Automated Software Engineering, ASE '17, pages 984--989, Piscataway, NJ, USA, October 2017. ACM/IEEE, IEEE Press. [ bib | DOI | http ]

Raffi Khatchadourian and Hidehiko Masuhara. Automated refactoring of legacy Java software to default methods. In International Conference on Software Engineering, ICSE '17, pages 82--93, Piscataway, NJ, USA, May 2017. ACM/IEEE, IEEE Press. [ bib | DOI | http ]

Citation

Please cite this work as follows:

@inproceedings{Khatchadourian2017b,
  author = {Khatchadourian, Raffi and Masuhara, Hidehiko},
  title = {Defaultification Refactoring: A Tool for Automatically Converting {Java} Methods to Default},
  booktitle = {International Conference on Automated Software Engineering},
  year = {2017},
  series = {ASE '17},
  pages = {984--989},
  address = {Piscataway, NJ, USA},
  month = oct,
  organization = {ACM/IEEE},
  publisher = {IEEE Press},
  isbn = {978-1-5386-2684-9},
  acmid = {3155691},
  doi = {10.1109/ASE.2017.8115716},
  location = {Urbana-Champaign, IL, USA},
  numpages = {6},
}

migrate-skeletal-implementation-to-interface-refactoring's Issues

Deal with javadoc

Need to deal with javadoc at some point. Specifically, the javadoc between the source and target methods may differ. Do we merge them? Pick one?

Deal with deprecated actionSets

actionSets are now deprecated. To get the refactoring to show up in the menu, we'll need to deal with that.

Initial test cases

Very basic test cases.

Review probable skeletal implemeantors

Please query the database for probable skeletal implementations. These are abstract classes that extend only one interface. Please look at each of these classes and see if they follow the skeletal implementation pattern? A few of them should have "Abstract" in there names, which is promising.

Can't change visibility of migrated method

All methods in interfaces are public. You can't have any non-public method in an interface, as far as I know. Thus, any migrated must be public as well.

I suppose that this is more relevant to the case where we migrate methods to interfaces from classes that do not implement the target interface.

Add popular Libraries on Github

http://blog.takipi.com/we-analyzed-60678-libraries-on-github-here-are-the-top-100/

CSV file of methods

I think it may be helpful to also have a table of methods. That way, we can eliminate classes that extend interfaces that have no methods (e.g., java.io.Serializable). There may also be some other uses. One thing that makes me hesistant, however, is that we can also use the JDT to find out this information as opposed to a SQL query. What do you think, @mdarefin?

Complete setup

Setup the refactoring plugin. https://app.asana.com/0/16544941486330/36417414412066.

Check that there are no conflicts in target type

If moving a method from source to target, need to ensure that there will be no methods in the target that will conflict with the moved method, e.g., with the same or similar signature.

This is probably pretty similar to the PullUp case but perhaps there are some special considerations for existing default methods in the target interface.

Initial method body preconditions

Currently, there are no precondition checks for the body of the methods. Start with the most stringent, e.g., no statements at all, and then relax, e.g., no instance field accesses.

UI doesn't work

https://github.com/khatchad/Java-8-Interface-Refactoring/issues/41The UI doesn't seem to be working. Not sure if it ever worked but it doesn't seem to do anything at the moment.

Initial precondition checks

Very basic checks.

Wrong error message for compilaton unit that doesn't compile

If a method is part of a compilation unit with structural errors, the context will be of the method, not the compilation unit.

Need at least one candidate target method (and probably only one)

We're not checking if the method to move has any candidate target methods in the target interface. There should probably be only one such entity.

-- Error Log from JUnit --
Class: edu.cuny.citytech.defaultrefactoring.ui.tests.MigrateSkeletalImplementationToInterfaceRefactoringTest
Method: testTargetInterfaceWithNoMethods
Actual: null
Expected: null
Stack Trace:
junit.framework.AssertionFailedError: Precondition was supposed to fail.
at junit.framework.Assert.fail(Assert.java:57)
at junit.framework.Assert.assertTrue(Assert.java:22)
at junit.framework.TestCase.assertTrue(TestCase.java:192)
at edu.cuny.citytech.refactoring.common.tests.RefactoringTest.assertFailedPrecondition(RefactoringTest.java:34)
at edu.cuny.citytech.refactoring.common.tests.RefactoringTest.assertFailedPrecondition(RefactoringTest.java:46)
at edu.cuny.citytech.refactoring.common.tests.RefactoringTest.helperFail(RefactoringTest.java:108)
at edu.cuny.citytech.refactoring.common.tests.RefactoringTest.helperFail(RefactoringTest.java:148)
at edu.cuny.citytech.defaultrefactoring.ui.tests.MigrateSkeletalImplementationToInterfaceRefactoringTest.testTargetInterfaceWithNoMethods(MigrateSkeletalImplementationToInterfaceRefactoringTest.java:205)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at junit.framework.TestCase.runTest(TestCase.java:176)
at junit.framework.TestCase.runBare(TestCase.java:141)
at junit.framework.TestResult$1.protect(TestResult.java:122)
at junit.framework.TestResult.runProtected(TestResult.java:142)
at junit.framework.TestResult.run(TestResult.java:125)
at junit.framework.TestCase.run(TestCase.java:129)
at junit.extensions.TestDecorator.basicRun(TestDecorator.java:23)
at junit.extensions.TestSetup$1.protect(TestSetup.java:23)
at junit.framework.TestResult.runProtected(TestResult.java:142)
at junit.extensions.TestSetup.run(TestSetup.java:27)
at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:131)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62)
at org.eclipse.pde.internal.junit.runtime.PlatformUITestHarness$1.run(PlatformUITestHarness.java:47)
at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:135)
at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4024)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3700)
at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:1127)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:337)
at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1018)
at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:156)
at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:654)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:337)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:598)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:139)
at org.eclipse.pde.internal.junit.runtime.NonUIThreadTestApplication.runApp(NonUIThreadTestApplication.java:54)
at org.eclipse.pde.internal.junit.runtime.UITestApplication.runApp(UITestApplication.java:47)
at org.eclipse.pde.internal.junit.runtime.NonUIThreadTestApplication.start(NonUIThreadTestApplication.java:48)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:380)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:235)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:669)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:608)
at org.eclipse.equinox.launcher.Main.run(Main.java:1515)
at org.eclipse.equinox.launcher.Main.main(Main.java:1488)

Removed skeletal implementations must consider declared type parameters

Before removing a skeletal implemetor, we would need to check that any type parameters declared by the class can moved to the interface.

Complete initial refactoring

Very basic initial refactoring that changes nothing.

Ensure that exception types between the source and target methods match

Workaround for #43.

Additional preconditions for strictfp

It turns out that there are some interest cases regarding the strictfp keyword. Consider the following input source code:

interface I {
    void m();
}

abstract class A implements I {
    strictfp public void m() {
    }   
}

Migrating A.m() to I would produce the following output source code:

interface I {
    default strictfp void m() {
    }
}

abstract class A implements I {
}

We would need to check that types inheriting I.m() are compatible with the new strictfp body.

Also, we have an issue when classes and interfaces are strictfp. Suppose we have this input:

interface I {
    void m();
}

strictfp abstract class A implements I {
    public void m() {
    }   
}

Here, A.m() is strictfp due to the modifier on the class. The output should be:

interface I {
    default strictfp void m() {
    }
}

strictfp abstract class A implements I {
}

or:

strictfp interface I {
    default void m() {
    }
}

strictfp abstract class A implements I {
}

Again, here we need to check that the strictfp conversion is compatible in the hierarchy.

There's also another case. Suppose we have this input:

strictfp interface I {
    void m();
}

abstract class A implements I {
    public void m() {
    }   
}

Here, the interface is strictfp but the source method isn't. This should be a failed precondition because I is strictfp, which would also make I.m() strictfp. That may not preserve semantics.

Extract text

Extract text from bug reports and asana task and put in paper.

Foreign key constraints violated when interface is outside of project

Hm. I don't think this is quite going to work as-is given the way I setup the database. If a class implements an interface that is not part of the project, e.g., java.io.Serializable, there will not be an entry for that interface in neither interfaces.csv nor types.csv. That will cause foreign key constraint failures.

But also the problem is that types.csv sort of assumes that the types are in the workspace.

Conditions on the target interface

There doesn't seem to be any conditions on the target interface. For now, we should check that the target interface exactly one method that matches the source interface? I am thinking no parameters, void return type, etc.

getCandidateTypes() does not consider target method declarations

The method edu.cuny.citytech.defaultrefactoring.core.refactorings.MigrateSkeletalImplementationToInterfaceRefactoringProcessor.getCandidateTypes(Optional<IProgressMonitor>) simply returns all super interfaces of the declaring type (from getDeclaringType()). However, not all of these are really candidates. This isn't a PullUp refactoring. We need an interface method to convert to default and tack on the body.

I believe that this is a key difference between the two refactorings. If we were going to allow for something like this, we should alter the PullUp refactoring, allowing the developer to PullUp methods to interfaces (and making them default).

Should specify dependency versions

Add minimum versions to framework dependencies.

Simple refactoring

Now that simple checks have been done, let's get a simple refactoring going.

Need to deal with imports when the method is migrated

Deal with exception type mismatches between the source and target methods

Consider the following valid input source code:

package p;

import java.io.IOException;

interface I {
    void m() throws Exception;
}

abstract class A implements I {
    public void m() throws IOException {
    }

    void n() throws IOException {
        m();
    }

    void q(I i) throws Exception {
        i.m();
    }
}

Suppose that A.m() is the source method. In migrating it to I, we now have a mismatch between the exception types thrown. Specifically, I.m() declares that it throws Exception, while A.m() declares that it throws IOException. In fact, picking either of these exception types for the target method would result in an error.

There are two issues here, I believe. First, we should not allow exception type mismatches (will create a new issue for this). Second, we need an issue to possibly relax this precondition. In the above situation, there's just nothing we can do. However, using intra-procedural analysis, it is possible to see which exception type to pick.

Add a ICompilationUnit to selectable elements

We currently have an IType selectable by the refactoring. Also add an ICompiilationUnit (which is the java file) and get the type(s) out of there.

Fix PullUp/PushDown to work with default methods?

In Eclipse, currently, when you pull up a method to an interface, it does some very funky stuff. I wonder if we should somehow fix that. Moreover, also need to consider PushDown.

Compute effectively functional interfaces

We saw with #35 that functional interfaces should not be refactored, i.e., they should fail preconditions. However, for #35, we are simply using the @FunctionalInterface annotation to determine if a particular interface is functional. However, this annotation just serves as a hint to the compiler to verify the constraints that make up functional interfaces. Developers are free to create and use functional interfaces without this annotation. As such, refactoring these interfaces will cause problems, particularly when they are used in lambda expressions and method references.

This task is to expand the edu.cuny.citytech.defaultrefactoring.core.refactorings.MigrateSkeletalImplementationToInterfaceRefactoringProcessor.isInterfaceFunctional(IType) method to find functional interfaces not having the @FunctionalInterface annotation.

Review notes from Hidehiko

[--- This issue is linked with an Asana task

attachments Notes.txt

---[aa]

Check accesses of the source method in the destination interface

https://github.com/khatchad/Java-8-Interface-Refactoring/issues/38 

The PullUp refactoring checks that the types accessed from the source method are accessible in the destination type. Is this also applicable for us? See org.eclipse.jdt.internal.corext.refactoring.structure.PullUpRefactoringProcessor.checkAccessedTypes(IProgressMonitor, ITypeHierarchy).

Seems to be more general in org.eclipse.jdt.internal.corext.refactoring.structure.PullUpRefactoringProcessor.checkAccesses(IProgressMonitor).

Forgot some declared type preconditions

I think I forgot to add a few preconditions for the declared type regarding its super type, etc.

Implement a declared methods CSV file

Check accesses of the source method in the destination interface

The PullUp refactoring checks that the types accessed from the source method are accessible in the destination type. Is this also applicable for us? See org.eclipse.jdt.internal.corext.refactoring.structure.PullUpRefactoringProcessor.checkAccessedTypes(IProgressMonitor, ITypeHierarchy).

Seems to be more general in org.eclipse.jdt.internal.corext.refactoring.structure.PullUpRefactoringProcessor.checkAccesses(IProgressMonitor).

Set up CI with basic test case

Add initial test case. Set up CI.

Add eclipse/jgit to subjects

https://github.com/eclipse/jgit

Subtype `org.eclipse.jdt.internal.corext.refactoring.structure.HierarchyProcessor`

I believe that edu.cuny.citytech.defaultrefactoring.core.refactorings.MigrateSkeletalImplementationToInterfaceRefactoring should actually subtype org.eclipse.jdt.internal.corext.refactoring.structure.HierarchyProcessor. Modeling a sibling like org.eclipse.jdt.internal.corext.refactoring.structure.PullUpRefactoringProcessor should help with #5.

Refactor methods that don't implement interfaces

For this phase, only methods that have interface counterparts should be migratable.

Do we really need annotations to be consistent between source and target methods?

I think I will create a precondition that states that annotations, as well as their field values, must be the same between the source and target methods. Of course, the order doesn't matter but the set does. Unlike strictfp, the annotations are really about the method itself, whereas strictfp is about the body of the method. Is there anyway to get around this? To relax it?

Need to be careful when adding modifiers to target method

When adding modifiers to the target method, we need to be more careful. For example, the target method may already have the modifier.

The target interface should not be a @FunctionalInterface

If we modify a @FunctionalInterface such that the method is now default, what happens to the lambda expressions using the @FunctionalInterface? My guess is that they won't work anymore but need to investigate.

Target method should not already be a default method

Refactoring should infer the target interface

I feel that this refactoring is quite different from the PullUp in some fundamental ways. First, PullUp's purpose is to reduce code redundancy in a single hierarchy. We, on the other hand, are trying to migrate an entire project to use this new construct. As such, I feel that our refactoring should somehow select the target interface. In our case, I don't think it really matters which interface we use but just that we use an interface. This isn't really the case in PullUp.

Review this content

http://www.oracle.com/technetwork/articles/java/bloch-effective-08-qa-140880.html

Classes extending classes

We need to put back the extends relation in this new model.

Removed skeletal implementations can't be instantiated

If we are going to eliminate a skeletal implementation, we need to make sure that it is never instantiated. If we are only doing abstract classes (which actually isn't necessary), then we don't need to worry about this.

Are we checking the type hierarchy of the input method?

UI doesn't work

The UI doesn't seem to be working. Not sure if it ever worked but it doesn't seem to do anything at the moment.

Deal with visibility

A lot of times, when you pull up a method to an interface, you may have visibility problems. It looks like the PullUp refactoring in Eclipse deals with this by issuing a warning and doing something extremely funky.

Fix coverage report

Started but not working. Looks like only the test module is getting reports, at least locally.