diff --git a/docs/features/code-actions.md b/docs/features/code-actions.md index a6aca9559c8..5095141d1a4 100644 --- a/docs/features/code-actions.md +++ b/docs/features/code-actions.md @@ -55,6 +55,12 @@ It converts a chain of `map`, `flatMap`, `filter` and `filterNot` methods into a ![To For Comprehension](./gifs/FlatMapToForComprehension.gif) +## filter then map to collect + +It converts a chain of `filter` and `map` methods into a `collect` method. + +![To Collect](./gifs/FilterMapToCollect.gif) + ## Implement Abstract Members of the Parent Type Upon inheriting from a type, you also have to implement its abstract members. But manually looking them all up and copying their signature is time consuming, isn't it? You can just use this code action instead. diff --git a/docs/features/gifs/FilterMapToCollect.gif b/docs/features/gifs/FilterMapToCollect.gif new file mode 100644 index 00000000000..24601f2b7ec Binary files /dev/null and b/docs/features/gifs/FilterMapToCollect.gif differ diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala index 5ed5356e5e3..97d1c6f3f5d 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala @@ -43,6 +43,7 @@ final class CodeActionProvider( new InlineValueCodeAction(trees, compilers, languageClient), new ConvertToNamedArguments(trees, compilers), new FlatMapToForComprehensionCodeAction(trees, buffers), + new FilterMapToCollectCodeAction(trees), new MillifyDependencyCodeAction(buffers), new MillifyScalaCliDependencyCodeAction(buffers), new ConvertCommentCodeAction(buffers), diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/FilterMapToCollectCodeAction.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/FilterMapToCollectCodeAction.scala new file mode 100644 index 00000000000..7d3c0d9f39d --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/FilterMapToCollectCodeAction.scala @@ -0,0 +1,192 @@ +package scala.meta.internal.metals.codeactions + +import scala.concurrent.ExecutionContext +import scala.concurrent.Future + +import scala.meta._ +import scala.meta.internal.metals.JsonParser._ +import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.codeactions.CodeAction +import scala.meta.internal.metals.codeactions.CodeActionBuilder +import scala.meta.internal.parsing.Trees +import scala.meta.pc.CancelToken + +import org.eclipse.lsp4j.CodeActionParams +import org.eclipse.{lsp4j => l} + +class FilterMapToCollectCodeAction(trees: Trees) extends CodeAction { + override def kind: String = l.CodeActionKind.RefactorRewrite + + private case class FilterMapCollectParams( + param: l.TextDocumentIdentifier, + pos: l.Position, + ) + + override def resolveCodeAction(codeAction: l.CodeAction, token: CancelToken)( + implicit ec: ExecutionContext + ): Option[Future[l.CodeAction]] = { + println(codeAction.getData.toJson) + val edits = for { + data <- codeAction.getData.toJson.as[FilterMapCollectParams].toOption + params = data.param + uri = params.getUri() + path = uri.toAbsolutePath + } yield trees + .findLastEnclosingAt[Term.Apply](path, data.pos) + .flatMap(findFilterMapChain) + .map(toTextEdit(_)) + .map(edit => List(uri -> List(edit))) + .getOrElse(Nil) + + edits match { + case None | (Some(Nil)) => None + case Some(xs) => { + val workspaceEdit = new l.WorkspaceEdit( + xs.map { case (uri, edits) => uri -> edits.asJava }.toMap.asJava + ) + codeAction.setEdit(workspaceEdit) + Some(Future.successful(codeAction)) + } + } + } + + override def contribute(params: CodeActionParams, token: CancelToken)(implicit + ec: ExecutionContext + ): Future[Seq[l.CodeAction]] = Future { + val uri = params.getTextDocument().getUri() + + val path = uri.toAbsolutePath + val start = params.getRange.getStart + + trees + .findLastEnclosingAt[Term.Apply](path, start) + .flatMap(findFilterMapChain) + .map(_ => { + val data = + FilterMapCollectParams( + params.getTextDocument(), + start, + ) + CodeActionBuilder.build( + title = FilterMapToCollectCodeAction.title, + kind = this.kind, + data = Some(data.toJsonObject), + ) + }) + .toSeq + } + + private def toTextEdit(chain: FilterMapChain) = { + val param = chain.filterFn.params.head + val paramName = Term.Name(param.name.value) + val paramPatWithType = param.decltpe match { + case Some(tpe) => Pat.Typed(Pat.Var(paramName), tpe) + case None => Pat.Var(paramName) + } + + val collectCall = Term.Apply( + fun = Term.Select(chain.qual, Term.Name("collect")), + argClause = Term.ArgClause( + values = List( + Term.PartialFunction( + cases = List( + Case( + pat = paramPatWithType, + cond = Some(chain.filterFn.renameParam(paramName)), + body = chain.mapFn.renameParam(paramName), + ) + ) + ) + ) + ), + ) + val indented = collectCall.syntax.linesIterator.zipWithIndex + .map { + case (line, 0) => line + case (line, _) => " " + line + } + .mkString("\n") + + new l.TextEdit(chain.pos.toLsp, indented) + } + + private implicit class FunctionOps(fn: Term.Function) { + def renameParam(to: Term.Name): Term = { + val fnParamName = fn.params.head.name.value + fn.body + .transform { case Term.Name(name) if name == fnParamName => to } + .asInstanceOf[Term] + } + } + + private def findFilterMapChain(tree: Term.Apply): Option[FilterMapChain] = { + val x = Term.Name("x") + def extractFunction(arg: Tree): Option[Term.Function] = arg match { + case fn: Term.Function => Some(fn) + case Term.Block(List(fn: Term.Function)) => extractFunction(fn) + case ref: Term.Name => { + Some( + Term.Function( + UnaryParameterList(x), + Term.Apply(ref, Term.ArgClause(List(x))), + ) + ) + } + case _ => None + } + + def findChain(tree: Term.Apply): Option[FilterMapChain] = + tree match { + case MapFunctionApply(FilterFunctionApply(base, filterArg), mapArg) => + for { + filterFn <- extractFunction(filterArg) + mapFn <- extractFunction(mapArg) + } yield FilterMapChain(tree.pos, base, filterFn, mapFn) + case _ => None + } + + findChain(tree).orElse { + // If we're inside the chain, look at our parent + tree.parent.flatMap { + // We're in a method call or function, look at parent apply + case Term.Select(_, Term.Name("map" | "filter")) | Term.Function(_) => + tree.parent + .flatMap(_.parent) + .collectFirst { case parent: Term.Apply => parent } + .flatMap(findChain) + case _ => None + } + } + } + + private object UnaryParameterList { + def unapply(tree: Tree): Option[Name] = tree match { + case Term.Param(_, name, _, _) => Some(name) + case _ => None + } + def apply(name: Name): List[Term.Param] = List( + Term.Param(Nil, name, None, None) + ) + } + + private case class FunctionApply(val name: String) { + def unapply(tree: Tree): Option[(Term, Term)] = tree match { + case Term.Apply(Term.Select(base, Term.Name(`name`)), List(args)) => + Some((base, args)) + case _ => None + } + } + private val FilterFunctionApply = new FunctionApply("filter") + private val MapFunctionApply = new FunctionApply("map") + + private case class FilterMapChain( + pos: Position, + qual: Term, + filterFn: Term.Function, + mapFn: Term.Function, + ) +} + +object FilterMapToCollectCodeAction { + val title = "Convert to collect" +} diff --git a/tests/unit/src/test/scala/tests/codeactions/FilterMapToCollectCodeActionSuite.scala b/tests/unit/src/test/scala/tests/codeactions/FilterMapToCollectCodeActionSuite.scala new file mode 100644 index 00000000000..bd386d002a3 --- /dev/null +++ b/tests/unit/src/test/scala/tests/codeactions/FilterMapToCollectCodeActionSuite.scala @@ -0,0 +1,277 @@ +package tests.codeactions + +import scala.meta.internal.metals.codeactions.FilterMapToCollectCodeAction +import scala.meta.internal.metals.codeactions.FlatMapToForComprehensionCodeAction +import scala.meta.internal.metals.codeactions.RewriteBracesParensCodeAction + +class FilterMapToCollectCodeActionSuite + extends BaseCodeActionLspSuite("filterMapToCollect") { + val simpleOutput: String = + """|object Main { + | val numbers = List(1, 2, 3, 4) + | numbers.collect { + | case x if x > 2 => + | x * 2 + | } + |} + |""".stripMargin + + check( + "cursor-on-filter", + """|object Main { + | val numbers = List(1, 2, 3, 4) + | numbers.fil<<>>ter(x => x > 2).map(x => x * 2) + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + simpleOutput, + selectedActionIndex = 2, + ) + + check( + "cursor-on-filter-newline", + """|object Main { + | val numbers = List(1, 2, 3, 4) + | numbers.fil<<>>ter{ + | x => x > 2 + | }.map{ + | x => x * 2 + | } + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toParens("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + simpleOutput, + selectedActionIndex = 2, + ) + + check( + "cursor-on-map", + """|object Main { + | val numbers = List(1, 2, 3, 4) + | numbers.filter(x => x > 2).m<<>>ap(x => x * 2) + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("map")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + simpleOutput, + selectedActionIndex = 2, + ) + + check( + "higher-order-function", + """|object Main { + | val numbers = List(1, 2, 3, 4) + | def check = (x: Int) => x > 2 + | def double = (x: Int) => x * 2 + | numbers.fil<<>>ter(check).map(double) + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + """|object Main { + | val numbers = List(1, 2, 3, 4) + | def check = (x: Int) => x > 2 + | def double = (x: Int) => x * 2 + | numbers.collect { + | case x if check(x) => + | double(x) + | } + |} + |""".stripMargin, + selectedActionIndex = 2, + ) + + check( + "higher-order-function-rename-filter-argument", + """|object Main { + | val numbers = List(1, 2, 3, 4) + | def double = (x: Int) => x * 2 + | numbers.fil<<>>ter(x => x > 0).map(double) + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + """|object Main { + | val numbers = List(1, 2, 3, 4) + | def double = (x: Int) => x * 2 + | numbers.collect { + | case x if x > 0 => + | double(x) + | } + |} + |""".stripMargin, + selectedActionIndex = 2, + ) + + check( + "higher-order-function-rename-map-argument", + """|object Main { + | val numbers = List(1, 2, 3, 4) + | def check = (x: Int) => x > 2 + | numbers.fil<<>>ter(check).map(y => y * 2) + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + """|object Main { + | val numbers = List(1, 2, 3, 4) + | def check = (x: Int) => x > 2 + | numbers.collect { + | case x if check(x) => + | x * 2 + | } + |} + |""".stripMargin, + selectedActionIndex = 2, + ) + + check( + "multiple-map-calls", + """|object Main { + | val numbers = List(1, 2, 3, 4) + | numbers.fil<<>>ter(x => x > 2).map(x => x * 2).map(x => x * 2) + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + """|object Main { + | val numbers = List(1, 2, 3, 4) + | numbers.collect { + | case x if x > 2 => + | x * 2 + | }.map(x => x * 2) + |} + |""".stripMargin, + selectedActionIndex = 2, + ) + + check( + "complex-predicate", + """|object Main { + | val users = List(("Alice", 25), ("Bob", 30)) + | users.fil<<>>ter(u => u._2 >= 18 && u._1.startsWith("A")).map(u => u._1) + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + """|object Main { + | val users = List(("Alice", 25), ("Bob", 30)) + | users.collect { + | case u if u._2 >= 18 && u._1.startsWith("A") => + | u._1 + | } + |} + |""".stripMargin, + selectedActionIndex = 2, + ) + + check( + "multiline-predicate", + """|object Main { + | val users = List(("Alice", 25), ("Bob", 30)) + | users + | .fil<<>>ter(u => + | u._2 >= 18 && + | u._1.startsWith("A") + | ) + | .map { u => + | // format user name into multline card + | val card = Seq( + | s"Name: ${u._1}", + | s"Age: ${u._2}", + | ) + | card.mkString("\n") + | } + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + """|object Main { + | val users = List(("Alice", 25), ("Bob", 30)) + | users.collect { + | case u if u._2 >= 18 && u._1.startsWith("A") => + | val card = Seq(s"Name: ${ + | u._1 + | }", s"Age: ${ + | u._2 + | }") + | card.mkString("\n") + | } + |} + |""".stripMargin, + selectedActionIndex = 2, + ) + + check( + "multiline-predicate-block", + """|object Main { + | val users = List(("Alice", 25), ("Bob", 30)) + | users + | .fil<<>>ter { u => + | val nameLength = u._1.length + | + | nameLength > 3 + | } + | .map { u => u._2 } + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toParens("map")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + """|object Main { + | val users = List(("Alice", 25), ("Bob", 30)) + | users.collect { + | case u if { + | val nameLength = u._1.length + | nameLength > 3 + | } => + | u._2 + | } + |} + |""".stripMargin, + selectedActionIndex = 2, + ) + + check( + "with-type-annotations", + """|object Main { + | val numbers = List(1, 2, 3, 4) + | numbers.fil<<>>ter((x: Int) => x > 2).map((x: Int) => x * 2) + |} + |""".stripMargin, + s"""|${RewriteBracesParensCodeAction.toBraces("filter")} + |${FlatMapToForComprehensionCodeAction.flatMapToForComprehension} + |${FilterMapToCollectCodeAction.title} + |""".stripMargin, + """|object Main { + | val numbers = List(1, 2, 3, 4) + | numbers.collect { + | case x: Int if x > 2 => + | x * 2 + | } + |} + |""".stripMargin, + selectedActionIndex = 2, + ) +}