From 057be89273c2cc09aad47e199f29541b7ffb4874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=A9rez=20Hidalgo?= Date: Mon, 18 Sep 2017 18:20:34 +0200 Subject: [PATCH 1/9] ImplementMethods refactor: Minimum test & prepare refactor stage --- .../implementations/ImplementMethods.scala | 32 +++++++++++ .../ImplementMethodsTest.scala | 56 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala create mode 100644 src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala diff --git a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala new file mode 100644 index 00000000..0db73d9a --- /dev/null +++ b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala @@ -0,0 +1,32 @@ +package scala.tools.refactoring.implementations + +import scala.tools.refactoring.common.InteractiveScalaCompiler +import scala.tools.refactoring.{MultiStageRefactoring, analysis} + +abstract class ImplementMethods extends MultiStageRefactoring with analysis.Indexes with InteractiveScalaCompiler { + + import global._ + + override type PreparationResult = Seq[DefDef] + + override def prepare(s: Selection): Either[PreparationError, PreparationResult] = { + val methodsToImplement = for { + selectedClassDeclaration <- index.declaration(s.enclosingTree.symbol).toSeq collect { + case traitDeclaration: ClassDef => traitDeclaration + } + unimplementedMethod <- selectedClassDeclaration.impl.body collect { + case methodDeclaration: DefDef if methodDeclaration.rhs.isEmpty => methodDeclaration + } + } yield unimplementedMethod + if(methodsToImplement.isEmpty) Left { + PreparationError("There are not methods to implement") + } + else Right(methodsToImplement) + } + + override type RefactoringParameters = Unit + + override def perform(selection: Selection, prepared: PreparationResult, params: RefactoringParameters) = { + ??? + } +} diff --git a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala new file mode 100644 index 00000000..92604fac --- /dev/null +++ b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala @@ -0,0 +1,56 @@ +package scala.tools.refactoring +package tests.implementations + +import scala.tools.refactoring.implementations.ImplementMethods +import scala.tools.refactoring.tests.util.{TestHelper, TestRefactoring} + +class ImplementMethodsTest extends TestHelper with TestRefactoring { + outer => + + val fromSource = + """ + |trait T { + | def f(x: Int): String + |} + | + |object Obj extends /*(*/T/*)*/ { + |} + """.stripMargin + + val toSource = + """ + |trait T { + | def f(x: Int): String + |} + | + |object Obj extends T { + | def g(): Int = { + | ??? Left(PreparationError("crash!")) + + | } + |} + """.stripMargin + + /*@Test + def addMethod() = { + global.ask { () => + val refactoring = new AddMethod { + val global: Global = outer.global + val file = addToCompiler(UniqueNames.basename(), fromSource) + val change = addMethod(file, "Obj", "g", List(Nil), Nil, Some("Int"), AddToObject) + } + assertEquals(toSource, Change.applyChanges(refactoring.change, fromSource)) + } + }*/ + + def implementMethods(pro: FileSet) = new TestRefactoringImpl(pro) { + override val refactoring = new ImplementMethods with TestProjectIndex + val changes = performRefactoring(()) + }.changes + + @Test + def doRefactor() = new FileSet() { + fromSource becomes toSource + } applyRefactoring implementMethods + +} From 5231769aa4bd64fcef5a459e700578648d760c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=83=C2=83=C3=82=C2=A9rez=20Hidalgo?= Date: Mon, 18 Sep 2017 19:32:47 +0200 Subject: [PATCH 2/9] Unfinished perform stage --- .../implementations/ImplementMethods.scala | 45 ++++++++++++++++--- .../ImplementMethodsTest.scala | 34 +++++++------- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala index 0db73d9a..1fa0eea3 100644 --- a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala +++ b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala @@ -1,32 +1,63 @@ package scala.tools.refactoring.implementations import scala.tools.refactoring.common.InteractiveScalaCompiler +import scala.tools.refactoring.transformation.TreeFactory import scala.tools.refactoring.{MultiStageRefactoring, analysis} -abstract class ImplementMethods extends MultiStageRefactoring with analysis.Indexes with InteractiveScalaCompiler { +abstract class ImplementMethods extends MultiStageRefactoring with analysis.Indexes with TreeFactory with InteractiveScalaCompiler { import global._ - override type PreparationResult = Seq[DefDef] + case class PreparationResult(targetTemplate: Template, methodsToImplement: Seq[DefDef]) override def prepare(s: Selection): Either[PreparationError, PreparationResult] = { + val methodsToImplement = for { - selectedClassDeclaration <- index.declaration(s.enclosingTree.symbol).toSeq collect { + selectedTemplateDeclaration <- index.declaration(s.enclosingTree.symbol).toSeq collect { case traitDeclaration: ClassDef => traitDeclaration } - unimplementedMethod <- selectedClassDeclaration.impl.body collect { + unimplementedMethod <- selectedTemplateDeclaration.impl.body collect { case methodDeclaration: DefDef if methodDeclaration.rhs.isEmpty => methodDeclaration } } yield unimplementedMethod - if(methodsToImplement.isEmpty) Left { + + val targetTemplate = s.expandToNextEnclosingTree.flatMap { + _.selectedSymbolTree collect { + case target: Template => target + } + } + + if(targetTemplate.isEmpty) Left { + PreparationError("No target class in selection") + } + else if(methodsToImplement.isEmpty) Left { PreparationError("There are not methods to implement") } - else Right(methodsToImplement) + else Right(PreparationResult(targetTemplate.get, methodsToImplement)) } override type RefactoringParameters = Unit override def perform(selection: Selection, prepared: PreparationResult, params: RefactoringParameters) = { - ??? + import prepared._ + + val findTemplate = filter { + case t: Template => t == targetTemplate + } + + val addMethods = transform { + case tpl @ Template(_, _, body) if tpl == prepared.targetTemplate => + val methodsBody = Block(Ident("???") :: Nil, EmptyTree) + val methodWithRhs = methodsToImplement.map(_ copy (rhs = methodsBody)) + tpl.copy(body = body ++ methodWithRhs).replaces(tpl) + } + + val transformation = topdown { + matchingChildren { + findTemplate &> + addMethods + } + } + Right(transformFile(selection.file, transformation)) } } diff --git a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala index 92604fac..76a6d0f6 100644 --- a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala +++ b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala @@ -9,40 +9,38 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { val fromSource = """ + |package implementMethods + | |trait T { - | def f(x: Int): String + | def f(x: Int): String |} | |object Obj extends /*(*/T/*)*/ { + | + | def g(x: Int): Int = 1 + | |} """.stripMargin val toSource = """ + |package implementMethods + | |trait T { - | def f(x: Int): String + | def f(x: Int): String |} | - |object Obj extends T { - | def g(): Int = { - | ??? Left(PreparationError("crash!")) - + |object Obj extends /*(*/T/*)*/ { + | + | def g(x: Int): Int = 1 + | + | def f(x: Int): String = { + | ??? | } + | |} """.stripMargin - /*@Test - def addMethod() = { - global.ask { () => - val refactoring = new AddMethod { - val global: Global = outer.global - val file = addToCompiler(UniqueNames.basename(), fromSource) - val change = addMethod(file, "Obj", "g", List(Nil), Nil, Some("Int"), AddToObject) - } - assertEquals(toSource, Change.applyChanges(refactoring.change, fromSource)) - } - }*/ - def implementMethods(pro: FileSet) = new TestRefactoringImpl(pro) { override val refactoring = new ImplementMethods with TestProjectIndex val changes = performRefactoring(()) From 9f454a0a3d0d7a8e35a4362f412f1c58ac60584c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=A9rez=20Hidalgo?= Date: Sat, 23 Sep 2017 14:06:45 +0200 Subject: [PATCH 3/9] Avoid adding implemented methods. --- .../implementations/ImplementMethods.scala | 50 ++++++++++++++++--- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala index 1fa0eea3..fa061f80 100644 --- a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala +++ b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala @@ -10,30 +10,66 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde case class PreparationResult(targetTemplate: Template, methodsToImplement: Seq[DefDef]) + /* Helper class to box methods so they can be + compared in terms of their signature. + */ + implicit class OverloadedMethod(val method: DefDef) { + + private val key = { + import method.{name, tparams, vparamss} + + val vparamsTypes = for { + paramList <- vparamss + param <- paramList + } yield param.tpt.tpe + + (name, tparams, vparamsTypes) + + } + + override def hashCode(): RunId = key.hashCode() + + override def equals(obj: scala.Any): Boolean = obj match { + case that: OverloadedMethod => key == that.key + case _ => false + } + } + override def prepare(s: Selection): Either[PreparationError, PreparationResult] = { + // Get a sequence of methods found in the selected mixed trait. val methodsToImplement = for { selectedTemplateDeclaration <- index.declaration(s.enclosingTree.symbol).toSeq collect { - case traitDeclaration: ClassDef => traitDeclaration + case templateDeclaration: ClassDef => templateDeclaration } unimplementedMethod <- selectedTemplateDeclaration.impl.body collect { - case methodDeclaration: DefDef if methodDeclaration.rhs.isEmpty => methodDeclaration + case methodDeclaration: DefDef if methodDeclaration.rhs.isEmpty => + methodDeclaration } } yield unimplementedMethod + // Use the selection to find the template where the methods should be implemented. val targetTemplate = s.expandToNextEnclosingTree.flatMap { _.selectedSymbolTree collect { case target: Template => target } } - if(targetTemplate.isEmpty) Left { + targetTemplate map { t => // If the selection has indeed a target template... + if(methodsToImplement.isEmpty) Left { //... as long as there are methods in the mixed trait... + PreparationError("There are not methods to implement") + } else Right { //... these are selected to be defined in the target template. + // If and only if they're not already defined there. + val implementedMethods: Set[OverloadedMethod] = { + t.body collect { + case methodDef: DefDef => new OverloadedMethod(methodDef) + } toSet + } + PreparationResult(t, methodsToImplement.filterNot(implementedMethods contains _)) + } + } getOrElse Left { PreparationError("No target class in selection") } - else if(methodsToImplement.isEmpty) Left { - PreparationError("There are not methods to implement") - } - else Right(PreparationResult(targetTemplate.get, methodsToImplement)) } override type RefactoringParameters = Unit From c083049b218b0e7431fd0e6885e5eb041b9816ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=A9rez=20Hidalgo?= Date: Sat, 23 Sep 2017 14:07:16 +0200 Subject: [PATCH 4/9] Improved tests --- .../ImplementMethodsTest.scala | 99 ++++++++++++++++--- 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala index 76a6d0f6..0db23b9a 100644 --- a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala +++ b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala @@ -7,7 +7,13 @@ import scala.tools.refactoring.tests.util.{TestHelper, TestRefactoring} class ImplementMethodsTest extends TestHelper with TestRefactoring { outer => - val fromSource = + def implementMethods(pro: FileSet) = new TestRefactoringImpl(pro) { + override val refactoring = new ImplementMethods with TestProjectIndex + val changes = performRefactoring(()) + }.changes + + @Test + def implementMethodFromFirstMixing() = new FileSet() { """ |package implementMethods | @@ -15,19 +21,95 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { | def f(x: Int): String |} | - |object Obj extends /*(*/T/*)*/ { + |trait S { + | def g(x: Int): Int + |} | - | def g(x: Int): Int = 1 + |object Obj extends /*(*/T/*)*/ with S { + | val x: Int = ??? + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait T { + | def f(x: Int): String + |} + | + |trait S { + | def g(x: Int): Int + |} + | + |object Obj extends /*(*/T/*)*/ with S { + | val x: Int = ??? + | + | def f(x: Int): String = { + | ??? + | } + |} + """.stripMargin + } applyRefactoring implementMethods + + @Test + def implementMethodFromSecondMixing() = new FileSet() { + """ + |package implementMethods + | + |trait T { + | def f(x: Int): String + |} + | + |trait S { + | def g(x: Int): Int + |} + | + |object Obj extends T with /*(*/S/*)*/ { + | val x: Int = ??? + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait T { + | def f(x: Int): String + |} | + |trait S { + | def g(x: Int): Int + |} + | + |object Obj extends T with /*(*/S/*)*/ { + | val x: Int = ??? + | + | def g(x: Int): Int = { + | ??? + | } |} """.stripMargin + } applyRefactoring implementMethods - val toSource = + @Test + def implementMethodsNotImplemented() = new FileSet() { """ |package implementMethods | |trait T { | def f(x: Int): String + | def g(x: Int): Int + |} + | + |object Obj extends /*(*/T/*)*/ { + | + | def g(x: Int): Int = 1 + | + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait T { + | def f(x: Int): String + | def g(x: Int): Int |} | |object Obj extends /*(*/T/*)*/ { @@ -40,15 +122,6 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { | |} """.stripMargin - - def implementMethods(pro: FileSet) = new TestRefactoringImpl(pro) { - override val refactoring = new ImplementMethods with TestProjectIndex - val changes = performRefactoring(()) - }.changes - - @Test - def doRefactor() = new FileSet() { - fromSource becomes toSource } applyRefactoring implementMethods } From e00c2ffb7eb57533bba445904951c4cf62447bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=A9rez=20Hidalgo?= Date: Wed, 27 Sep 2017 16:33:40 +0200 Subject: [PATCH 5/9] Add methods even when a template kind is selected. --- .../implementations/ImplementMethods.scala | 12 +++- .../ImplementMethodsTest.scala | 69 +++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala index fa061f80..b22d12b6 100644 --- a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala +++ b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala @@ -37,11 +37,17 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde override def prepare(s: Selection): Either[PreparationError, PreparationResult] = { + + // Expand the selection to the concrete type when a kind was initially selected. + val maybeSelectedTemplate = (s::s.expandToNextEnclosingTree.toList) flatMap { sel: Selection => + index.declaration(sel.enclosingTree.symbol) + } collectFirst { + case templateDeclaration: ClassDef => templateDeclaration + } + // Get a sequence of methods found in the selected mixed trait. val methodsToImplement = for { - selectedTemplateDeclaration <- index.declaration(s.enclosingTree.symbol).toSeq collect { - case templateDeclaration: ClassDef => templateDeclaration - } + selectedTemplateDeclaration <- maybeSelectedTemplate.toList unimplementedMethod <- selectedTemplateDeclaration.impl.body collect { case methodDeclaration: DefDef if methodDeclaration.rhs.isEmpty => methodDeclaration diff --git a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala index 0db23b9a..05a16b70 100644 --- a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala +++ b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala @@ -124,4 +124,73 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { """.stripMargin } applyRefactoring implementMethods + @Test + def implementMethodsSelectType() = new FileSet() { + """ + |package implementMethods + | + |trait T[T] { + | def f[T]: T + |} + | + |class C extends /*(*/T[Int]/*)*/ { + | + | def g(x: Int): Int = ??? + | + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait T[T] { + | def f[T]: T + |} + | + |class C extends /*(*/T[Int]/*)*/ { + | + | def g(x: Int): Int = ??? + | + | def f[T]: T = { + | ??? + | } + | + |} + """.stripMargin + } applyRefactoring implementMethods + + @Test + def implementMethodsSelectKind() = new FileSet() { + """ + |package implementMethods + | + |trait T[T] { + | def f[T]: T + |} + | + |class C extends /*(*/T/*)*/[Int] { + | + | def g(x: Int): Int = ??? + | + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait T[T] { + | def f[T]: T + |} + | + |class C extends /*(*/T/*)*/[Int] { + | + | def g(x: Int): Int = ??? + | + | def f[T]: T = { + | ??? + | } + | + |} + """.stripMargin + } applyRefactoring implementMethods + + } From 2cc215597a54fb2b668225fbf165d90a5293fd3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=A9rez=20Hidalgo?= Date: Wed, 27 Sep 2017 16:37:02 +0200 Subject: [PATCH 6/9] Additional tests --- .../ImplementMethodsTest.scala | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala index 05a16b70..00b819ad 100644 --- a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala +++ b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala @@ -50,6 +50,36 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { """.stripMargin } applyRefactoring implementMethods + @Test + def implementMethodFromExtendedClass() = new FileSet() { + """ + |package implementMethods + | + |abstract class C { + | def f(x: Int): String + |} + | + |object Obj extends /*(*/C/*)*/ { + | val x: Int = ??? + |} + """.stripMargin becomes + """ + |package implementMethods + | + |abstract class C { + | def f(x: Int): String + |} + | + |object Obj extends /*(*/C/*)*/ { + | val x: Int = ??? + | + | def f(x: Int): String = { + | ??? + | } + |} + """.stripMargin + } applyRefactoring implementMethods + @Test def implementMethodFromSecondMixing() = new FileSet() { """ @@ -63,7 +93,7 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { | def g(x: Int): Int |} | - |object Obj extends T with /*(*/S/*)*/ { + |class C extends T with /*(*/S/*)*/ { | val x: Int = ??? |} """.stripMargin becomes @@ -78,7 +108,7 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { | def g(x: Int): Int |} | - |object Obj extends T with /*(*/S/*)*/ { + |class C extends T with /*(*/S/*)*/ { | val x: Int = ??? | | def g(x: Int): Int = { From 2baf08935f62a404b15977634e337b2a2a567e9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=83=C2=83=C3=82=C2=A9rez=20Hidalgo?= Date: Fri, 29 Sep 2017 11:21:28 +0200 Subject: [PATCH 7/9] Implement methods not only from the selected template but also from its ancestry. --- .../implementations/ImplementMethods.scala | 39 +++++-- .../ImplementMethodsTest.scala | 106 ++++++++++++++++++ 2 files changed, 137 insertions(+), 8 deletions(-) diff --git a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala index b22d12b6..610f0d5e 100644 --- a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala +++ b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala @@ -35,6 +35,17 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde } } + private def templateAncestry(template: Template): List[Template] = + template :: { + for { + parent <- template.parents + parentImp <- index.declaration(parent.symbol).toList collect { + case ClassDef(_, _, _, impl) => impl + } + ancestor <- templateAncestry(parentImp) + } yield ancestor + } + override def prepare(s: Selection): Either[PreparationError, PreparationResult] = { @@ -42,17 +53,29 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde val maybeSelectedTemplate = (s::s.expandToNextEnclosingTree.toList) flatMap { sel: Selection => index.declaration(sel.enclosingTree.symbol) } collectFirst { - case templateDeclaration: ClassDef => templateDeclaration + case templateDeclaration: ClassDef => templateDeclaration.impl } // Get a sequence of methods found in the selected mixed trait. - val methodsToImplement = for { - selectedTemplateDeclaration <- maybeSelectedTemplate.toList - unimplementedMethod <- selectedTemplateDeclaration.impl.body collect { - case methodDeclaration: DefDef if methodDeclaration.rhs.isEmpty => - methodDeclaration - } - } yield unimplementedMethod + val methodsToImplement = { + + val rawList = for { + selectedTemplate <- maybeSelectedTemplate.toList + selectedDeclaration <- templateAncestry(selectedTemplate) + unimplementedMethod <- selectedDeclaration.body collect { + case methodDeclaration: DefDef if methodDeclaration.rhs.isEmpty => + methodDeclaration + } + } yield unimplementedMethod; + + val (uniqueMethods, _) = + rawList.foldRight((List.empty[DefDef], Set.empty[OverloadedMethod])) { + case (method, (l, visited)) if !visited.contains(method) => + (method::l, visited + method) + case (_, st) => st + } + uniqueMethods + } // Use the selection to find the template where the methods should be implemented. val targetTemplate = s.expandToNextEnclosingTree.flatMap { diff --git a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala index 00b819ad..06af3428 100644 --- a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala +++ b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala @@ -223,4 +223,110 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { } applyRefactoring implementMethods + @Test + def implementMethodFromAncestry() = new FileSet() { + """ + |package implementMethods + | + |trait R { + | def k: Unit + |} + | + |trait T { + | def f(x: Int): String + |} + | + |trait S extends T { + | def g(x: Int): Int + |} + | + |object Obj extends /*(*/S/*)*/ with R { + | val x: Int = ??? + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait R { + | def k: Unit + |} + | + |trait T { + | def f(x: Int): String + |} + | + |trait S extends T { + | def g(x: Int): Int + |} + | + |object Obj extends /*(*/S/*)*/ with R { + | val x: Int = ??? + | + | def g(x: Int): Int = { + | ??? + | } + | + | def f(x: Int): String = { + | ??? + | } + |} + """.stripMargin + + } applyRefactoring implementMethods + + @Test + def implementMethodFromCyclicAncestry() = new FileSet() { + """ + |package implementMethods + | + |trait R { + | def k: Unit + |} + | + |trait T extends R { + | def f(x: Int): String + |} + | + |trait S extends T with R { + | def g(x: Int): Int + |} + | + |object Obj extends /*(*/S/*)*/ with R { + | val x: Int = ??? + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait R { + | def k: Unit + |} + | + |trait T extends R { + | def f(x: Int): String + |} + | + |trait S extends T with R { + | def g(x: Int): Int + |} + | + |object Obj extends /*(*/S/*)*/ with R { + | val x: Int = ??? + | + | def g(x: Int): Int = { + | ??? + | } + | + | def f(x: Int): String = { + | ??? + | } + | + | def k: Unit = { + | ??? + | } + |} + """.stripMargin + + } applyRefactoring implementMethods + } From 7b0f633586460c4b0fc18af2f8e8e6c37a2b0df3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=A9rez=20Hidalgo?= Date: Mon, 16 Oct 2017 19:14:47 +0200 Subject: [PATCH 8/9] Implement not only methods but also values. --- .../implementations/ImplementMethods.scala | 61 +++++++++++-------- .../ImplementMethodsTest.scala | 56 ++++++++++++++++- 2 files changed, 89 insertions(+), 28 deletions(-) diff --git a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala index 610f0d5e..2a82e75d 100644 --- a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala +++ b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala @@ -8,29 +8,32 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde import global._ - case class PreparationResult(targetTemplate: Template, methodsToImplement: Seq[DefDef]) + case class PreparationResult(targetTemplate: Template, memberToImplement: Seq[ValOrDefDef]) - /* Helper class to box methods so they can be + /* Helper class to box members so they can be compared in terms of their signature. */ - implicit class OverloadedMethod(val method: DefDef) { + implicit class OverloadedMember(val member: ValOrDefDef) { - private val key = { - import method.{name, tparams, vparamss} + private val key = member match { + case method: DefDef => + import method.{name, tparams, vparamss} - val vparamsTypes = for { - paramList <- vparamss - param <- paramList - } yield param.tpt.tpe + val vparamsTypes = for { + paramList <- vparamss + param <- paramList + } yield param.tpt.tpe - (name, tparams, vparamsTypes) + (name, tparams, vparamsTypes) + case ValDef(_, name, _, _) => name.toString.trim } - override def hashCode(): RunId = key.hashCode() + override def hashCode(): RunId = + key.hashCode() override def equals(obj: scala.Any): Boolean = obj match { - case that: OverloadedMethod => key == that.key + case that: OverloadedMember => key == that.key case _ => false } } @@ -56,20 +59,19 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde case templateDeclaration: ClassDef => templateDeclaration.impl } - // Get a sequence of methods found in the selected mixed trait. - val methodsToImplement = { + // Get a sequence of members (methods or values) found in the selected mixed trait + val membersToImplement = { val rawList = for { selectedTemplate <- maybeSelectedTemplate.toList selectedDeclaration <- templateAncestry(selectedTemplate) - unimplementedMethod <- selectedDeclaration.body collect { - case methodDeclaration: DefDef if methodDeclaration.rhs.isEmpty => - methodDeclaration + unimplementedMember <- selectedDeclaration.body collect { + case defOrValDef: ValOrDefDef if defOrValDef.rhs.isEmpty => defOrValDef } - } yield unimplementedMethod; + } yield unimplementedMember val (uniqueMethods, _) = - rawList.foldRight((List.empty[DefDef], Set.empty[OverloadedMethod])) { + rawList.foldRight((List.empty[ValOrDefDef], Set.empty[OverloadedMember])) { case (method, (l, visited)) if !visited.contains(method) => (method::l, visited + method) case (_, st) => st @@ -77,7 +79,7 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde uniqueMethods } - // Use the selection to find the template where the methods should be implemented. + // Use the selection to find the template where the members should be implemented. val targetTemplate = s.expandToNextEnclosingTree.flatMap { _.selectedSymbolTree collect { case target: Template => target @@ -85,16 +87,16 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde } targetTemplate map { t => // If the selection has indeed a target template... - if(methodsToImplement.isEmpty) Left { //... as long as there are methods in the mixed trait... + if(membersToImplement.isEmpty) Left { //... as long as there are members in the mixed trait... PreparationError("There are not methods to implement") } else Right { //... these are selected to be defined in the target template. - // If and only if they're not already defined there. - val implementedMethods: Set[OverloadedMethod] = { + // If and only if they were not already defined there. + val implementedMembers: Set[OverloadedMember] = { t.body collect { - case methodDef: DefDef => new OverloadedMethod(methodDef) + case memberDef: ValOrDefDef => new OverloadedMember(memberDef) } toSet } - PreparationResult(t, methodsToImplement.filterNot(implementedMethods contains _)) + PreparationResult(t, membersToImplement.filterNot(implementedMembers contains _)) } } getOrElse Left { PreparationError("No target class in selection") @@ -112,8 +114,13 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde val addMethods = transform { case tpl @ Template(_, _, body) if tpl == prepared.targetTemplate => - val methodsBody = Block(Ident("???") :: Nil, EmptyTree) - val methodWithRhs = methodsToImplement.map(_ copy (rhs = methodsBody)) + val unimplementedSentence = Ident("???") + val methodWithRhs = memberToImplement collect { + case methodDef: DefDef => + methodDef copy (rhs = Block(unimplementedSentence :: Nil, EmptyTree)) + case valueDef: ValDef => + valueDef copy (rhs = unimplementedSentence) + } tpl.copy(body = body ++ methodWithRhs).replaces(tpl) } diff --git a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala index 06af3428..31dbc408 100644 --- a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala +++ b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala @@ -119,7 +119,7 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { } applyRefactoring implementMethods @Test - def implementMethodsNotImplemented() = new FileSet() { + def implementNotImplementedMethods() = new FileSet() { """ |package implementMethods | @@ -154,6 +154,60 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { """.stripMargin } applyRefactoring implementMethods + @Test + def implementNotImplementedMembers() = new FileSet() { + """ + |package implementMethods + | + |trait T { + | + | val x: Int + | val y: Double + | val a_longIdentifier: Long + | + | def f(x: Int): String + | def g(x: Int): Int + |} + | + |object Obj extends /*(*/T/*)*/ { + | + | val y: Double = 42.0 + | val a_longIdentifier: Long = 42L + | + | def g(x: Int): Int = 1 + | + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait T { + | + | val x: Int + | val y: Double + | val a_longIdentifier: Long + | + | def f(x: Int): String + | def g(x: Int): Int + |} + | + |object Obj extends /*(*/T/*)*/ { + | + | val y: Double = 42.0 + | val a_longIdentifier: Long = 42L + | + | def g(x: Int): Int = 1 + | + | val x: Int = ??? + | + | def f(x: Int): String = { + | ??? + | } + | + |} + """.stripMargin + } applyRefactoring implementMethods + @Test def implementMethodsSelectType() = new FileSet() { """ From 034a0625d9757935e33f392f7248da02cf51025c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Fco=2E=20P=C3=A9rez=20Hidalgo?= Date: Tue, 17 Oct 2017 16:42:52 +0200 Subject: [PATCH 9/9] "Implement methods" now is able to implement abstract type tags. --- .../implementations/ImplementMethods.scala | 38 +++++++----- .../ImplementMethodsTest.scala | 60 +++++++++++++++++++ 2 files changed, 83 insertions(+), 15 deletions(-) diff --git a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala index 2a82e75d..f3fa6e8d 100644 --- a/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala +++ b/src/main/scala/scala/tools/refactoring/implementations/ImplementMethods.scala @@ -8,12 +8,12 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde import global._ - case class PreparationResult(targetTemplate: Template, memberToImplement: Seq[ValOrDefDef]) + case class PreparationResult(targetTemplate: Template, memberToImplement: Seq[MemberDef]) /* Helper class to box members so they can be compared in terms of their signature. */ - implicit class OverloadedMember(val member: ValOrDefDef) { + implicit class OverloadedMember(val member: MemberDef) { private val key = member match { case method: DefDef => @@ -26,7 +26,8 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde (name, tparams, vparamsTypes) - case ValDef(_, name, _, _) => name.toString.trim + case ValDef(_, name, _, _) => 'valdef -> name.toString.trim + case TypeDef(_, name, _, _) => 'typedef -> name.toString.trim } override def hashCode(): RunId = @@ -59,7 +60,7 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde case templateDeclaration: ClassDef => templateDeclaration.impl } - // Get a sequence of members (methods or values) found in the selected mixed trait + // Get a sequence of members (types, methods or values) found in the selected mixed trait val membersToImplement = { val rawList = for { @@ -67,16 +68,18 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde selectedDeclaration <- templateAncestry(selectedTemplate) unimplementedMember <- selectedDeclaration.body collect { case defOrValDef: ValOrDefDef if defOrValDef.rhs.isEmpty => defOrValDef + case typeDef: TypeDef if !typeDef.rhs.hasExistingSymbol => + typeDef } } yield unimplementedMember - val (uniqueMethods, _) = - rawList.foldRight((List.empty[ValOrDefDef], Set.empty[OverloadedMember])) { - case (method, (l, visited)) if !visited.contains(method) => - (method::l, visited + method) + val (uniqueMembers, _) = + rawList.foldRight((List.empty[MemberDef], Set.empty[OverloadedMember])) { + case (member, (l, visited)) if !visited.contains(member) => + (member::l, visited + member) case (_, st) => st } - uniqueMethods + uniqueMembers } // Use the selection to find the template where the members should be implemented. @@ -88,12 +91,12 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde targetTemplate map { t => // If the selection has indeed a target template... if(membersToImplement.isEmpty) Left { //... as long as there are members in the mixed trait... - PreparationError("There are not methods to implement") + PreparationError("There are not members to implement") } else Right { //... these are selected to be defined in the target template. // If and only if they were not already defined there. val implementedMembers: Set[OverloadedMember] = { t.body collect { - case memberDef: ValOrDefDef => new OverloadedMember(memberDef) + case memberDef: MemberDef => new OverloadedMember(memberDef) } toSet } PreparationResult(t, membersToImplement.filterNot(implementedMembers contains _)) @@ -112,22 +115,27 @@ abstract class ImplementMethods extends MultiStageRefactoring with analysis.Inde case t: Template => t == targetTemplate } - val addMethods = transform { + val addMembers = transform { case tpl @ Template(_, _, body) if tpl == prepared.targetTemplate => val unimplementedSentence = Ident("???") - val methodWithRhs = memberToImplement collect { + val thisType = SingletonTypeTree(This(TypeName(""))) + + val membersWithRhs = memberToImplement collect { case methodDef: DefDef => methodDef copy (rhs = Block(unimplementedSentence :: Nil, EmptyTree)) case valueDef: ValDef => valueDef copy (rhs = unimplementedSentence) + case typeDef: TypeDef => + typeDef copy (rhs = thisType) } - tpl.copy(body = body ++ methodWithRhs).replaces(tpl) + + tpl.copy(body = body ++ membersWithRhs).replaces(tpl) } val transformation = topdown { matchingChildren { findTemplate &> - addMethods + addMembers } } Right(transformFile(selection.file, transformation)) diff --git a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala index 31dbc408..36911cdf 100644 --- a/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala +++ b/src/test/scala/scala/tools/refactoring/tests/implementations/ImplementMethodsTest.scala @@ -383,4 +383,64 @@ class ImplementMethodsTest extends TestHelper with TestRefactoring { } applyRefactoring implementMethods + @Test + def implementTypes() = new FileSet() { + """ + |package implementMethods + | + |trait T { + | type S + |} + | + |object Obj extends /*(*/T/*)*/ { + | val x: Int = 3 + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait T { + | type S + |} + | + |object Obj extends /*(*/T/*)*/ { + | val x: Int = 3 + | + | type S = this.type + |} + """.stripMargin + } applyRefactoring implementMethods + + @Test + def implementTypesSkippingImplemented() = new FileSet() { + """ + |package implementMethods + | + |trait T { + | type S + | type R + |} + | + |object Obj extends /*(*/T/*)*/ { + | type R = Int + | val x: Int = 3 + |} + """.stripMargin becomes + """ + |package implementMethods + | + |trait T { + | type S + | type R + |} + | + |object Obj extends /*(*/T/*)*/ { + | type R = Int + | val x: Int = 3 + | + | type S = this.type + |} + """.stripMargin + } applyRefactoring implementMethods + }