From e516b0902da832558a74325d60f0126a967cb347 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 29 May 2023 17:00:57 +0200 Subject: [PATCH 1/5] improvement: show args completions for all matching overloaded methods 1. added fallback for arg completions for overloaded methods in Scala 3 (previously no arg completion would be visible) 2. in Scala 2, choose the matching ones from overloaded methods and collect matching arg completions from all of those --- .../pc/completions/ArgCompletions.scala | 108 ++++--- .../pc/completions/NamedArgCompletions.scala | 223 ++++++++++++--- .../scala/tests/pc/CompletionArgSuite.scala | 269 +++++++++++++++++- 3 files changed, 513 insertions(+), 87 deletions(-) diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala index a17222497e4..a0c83a39c4d 100644 --- a/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala +++ b/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala @@ -21,49 +21,85 @@ trait ArgCompletions { this: MetalsGlobal => val funPos = apply.fun.pos val method: Tree = typedTreeAt(funPos) val methodSym = method.symbol - lazy val baseParams: List[Symbol] = - if (methodSym.isModule) - getParamsFromObject() - else if (methodSym.isClass) methodSym.constrParamAccessors + lazy val baseParamss: List[List[Symbol]] = { + if (methodSym.isModule) getParamsFromObject(methodSym) + else if ( + methodSym.isMethod && methodSym.decodedName == "apply" && methodSym.owner.isModuleClass + ) getParamsFromObject(methodSym.owner) + else if (methodSym.isClass) List(methodSym.constrParamAccessors) else if (method.tpe == null) method match { case Ident(name) => metalsScopeMembers(funPos) - .collectFirst { + .map { case m: Member - if m.symNameDropLocal == name && m.sym != NoSymbol && m.sym.paramss.nonEmpty => + if m.symNameDropLocal == name && m.sym != NoSymbol && m.sym.paramss.nonEmpty && checkIfArgsMatch( + m.sym.paramss.head + ) => m.sym.paramss.head + case _ => Nil } - .getOrElse(Nil) case _ => Nil } else { - method.tpe.paramss.headOption - .getOrElse(methodSym.paramss.flatten) + method.tpe match { + case OverloadedType(_, alts) => + alts.flatMap(_.info.paramss.headOption) + case _ => + List( + method.tpe.paramss.headOption + .getOrElse(methodSym.paramss.flatten) + ) + } } + } - lazy val isNamed: Set[Name] = apply.args.iterator - .filterNot(_ == ident) - .zip(baseParams.iterator) - .map { - case (AssignOrNamedArg(Ident(name), _), _) => - name - case (_, param) => - param.name + def checkIfArgsMatch(possibleMatch: List[Symbol]): Boolean = { + apply.args.length <= possibleMatch.length && + !apply.args.zipWithIndex.exists { + case (Ident(name), _) if name.decodedName.endsWith(CURSOR) => false + case (AssignOrNamedArg(Ident(name), rhs), _) => + !possibleMatch.exists { param => + name.decodedName == param.name.decodedName && + (rhs.tpe == null || rhs.tpe <:< param.tpe) + } + case (rhs, i) => + rhs.tpe != null && !(rhs.tpe <:< possibleMatch(i).tpe) + } + } + lazy val baseParamssWithisNamed: List[(List[Symbol], Set[Name])] = + baseParamss.map { baseParams => + val isNamed = + apply.args.iterator + .filterNot(_ == ident) + .zip(baseParams.iterator) + .map { + case (AssignOrNamedArg(Ident(name), _), _) => + name + case (_, param) => + param.name + } + .toSet + (baseParams, isNamed) } - .toSet val prefix: String = ident.name.toString.stripSuffix(CURSOR) - lazy val allParams: List[Symbol] = { - baseParams.iterator.filterNot { param => - isNamed(param.name) || - param.isSynthetic - }.toList + lazy val allParams: List[Symbol] = baseParamssWithisNamed.flatMap { + case (baseParams, isNamed) => + baseParams.iterator.filterNot { param => + isNamed(param.name) || + param.isSynthetic + }.toList } lazy val params: List[Symbol] = - allParams.filter(param => param.name.startsWith(prefix)) + allParams + .filter(param => param.name.startsWith(prefix)) + .foldLeft(List.empty[Symbol])((acc, curr) => + if (acc.exists(el => el.name == curr.name && el.tpe == curr.tpe)) acc + else curr :: acc + ) + .reverse lazy val isParamName: Set[String] = params.iterator .map(_.name) - .filterNot(isNamed) .map(_.toString().trim()) .toSet @@ -129,7 +165,7 @@ trait ArgCompletions { this: MetalsGlobal => val isExplicitlyCalled = suffix.startsWith(prefix) val hasParamsToFill = allParams.count(!_.hasDefault) > 1 if ( - (shouldShow || isExplicitlyCalled) && hasParamsToFill && clientSupportsSnippets + baseParamss.length == 1 && (shouldShow || isExplicitlyCalled) && hasParamsToFill && clientSupportsSnippets ) { val editText = allParams.zipWithIndex .collect { @@ -170,20 +206,18 @@ trait ArgCompletions { this: MetalsGlobal => } } - private def getParamsFromObject(): List[Symbol] = { - methodSym.info.members - .collect { - case m - if m.decodedName == "apply" && - m.paramss.flatten.length >= apply.args.length => - m.paramss.flatten - } - .lastOption // for case classes, apply in companion object is after the default one - .getOrElse(Nil) + private def getParamsFromObject(objectSym: Symbol): List[List[Symbol]] = { + objectSym.info.members.collect { + case m + if m.decodedName == "apply" && m.paramss.nonEmpty && checkIfArgsMatch( + m.paramss.head + ) => + m.paramss.head + }.toList } override def contribute: List[Member] = { - params.map(param => + params.distinct.map(param => new NamedArgMember(param) ) ::: findPossibleDefaults() ::: fillAllFields() } diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala index 6133ecf9081..59a4b5b0433 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala @@ -1,17 +1,29 @@ package scala.meta.internal.pc.completions +import scala.util.Try + import scala.meta.internal.mtags.MtagsEnrichments.* import scala.meta.internal.pc.IndexedContext import dotty.tools.dotc.ast.Trees.ValDef import dotty.tools.dotc.ast.tpd.* import dotty.tools.dotc.core.Constants.Constant +import dotty.tools.dotc.core.ContextOps.localContext import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.core.Definitions import dotty.tools.dotc.core.Flags +import dotty.tools.dotc.core.Flags.Method import dotty.tools.dotc.core.NameKinds.DefaultGetterName import dotty.tools.dotc.core.Names.Name +import dotty.tools.dotc.core.Symbols import dotty.tools.dotc.core.Symbols.Symbol +import dotty.tools.dotc.core.Types.AndType +import dotty.tools.dotc.core.Types.AppliedType +import dotty.tools.dotc.core.Types.OrType +import dotty.tools.dotc.core.Types.TermRef import dotty.tools.dotc.core.Types.Type +import dotty.tools.dotc.core.Types.TypeBounds +import dotty.tools.dotc.core.Types.WildcardType import dotty.tools.dotc.util.SourcePosition object NamedArgCompletions: @@ -61,6 +73,7 @@ object NamedArgCompletions: apply.fun match case Select(New(_), _) => false case Select(_, name) if name.decoded == "apply" => false + case Select(This(_), _) => false // is a select statement without a dot `qual.name` case sel @ Select(qual, _) if !sel.symbol.is(Flags.Synthetic) => !(qual.span.end until sel.nameSpan.start) @@ -73,7 +86,7 @@ object NamedArgCompletions: apply: Apply, indexedContext: IndexedContext, clientSupportsSnippets: Boolean, - )(using Context): List[CompletionValue] = + )(using context: Context): List[CompletionValue] = def isUselessLiteral(arg: Tree): Boolean = arg match case Literal(Constant(())) => true // unitLiteral @@ -99,52 +112,120 @@ object NamedArgCompletions: end collectArgss val method = apply.fun - val methodSym = method.symbol - // paramSymss contains both type params and value params - val vparamss = - methodSym.paramSymss.filter(params => params.forall(p => p.isTerm)) val argss = collectArgss(apply) - // get params and args we are interested in - // e.g. - // in the following case, the interesting args and params are - // - params: [apple, banana] - // - args: [apple, b] - // ``` - // def curry(x; Int)(apple: String, banana: String) = ??? - // curry(1)(apple = "test", b@@) - // ``` - val (baseParams, baseArgs) = - vparamss.zip(argss).lastOption.getOrElse((Nil, Nil)) - - val args = ident - .map(i => baseArgs.filterNot(_ == i)) - .getOrElse(baseArgs) - .filterNot(isUselessLiteral) - - val isNamed: Set[Name] = args.iterator - .zip(baseParams.iterator) - // filter out synthesized args and default arg getters - .filterNot { - case (arg, _) if arg.symbol.denot.is(Flags.Synthetic) => true - case (Ident(name), _) => name.is(DefaultGetterName) // default args - case (Select(Ident(_), name), _) => - name.is(DefaultGetterName) // default args for apply method - case _ => false - } - .map { - case (NamedArg(name, _), _) => name - case (_, param) => param.name - } - .toSet - val allParams: List[Symbol] = + // fallback for when multiple overloaded methods match the supplied args + def fallbackFindMatchingMethods() = + def maybeNameAndIndexedContext( + method: Tree + ): Option[(Name, IndexedContext)] = + method match + case Ident(name) => Some((name, indexedContext)) + case Select(This(_), name) => Some((name, indexedContext)) + case Select(from, name) => + val symbol = from.symbol + val ownerSymbol = + if symbol.is(Method) && symbol.owner.isClass then + Some(symbol.owner) + else Try(symbol.info.classSymbol).toOption + ownerSymbol.map(sym => + (name, IndexedContext(context.localContext(from, sym))) + ) + case Apply(fun, _) => maybeNameAndIndexedContext(fun) + case _ => None + val matchingMethods = + for + (name, indxContext) <- maybeNameAndIndexedContext(method) + potentialMatches <- indxContext.findSymbol(name) + yield potentialMatches.collect { + case m + if m.is(Flags.Method) && + m.vparamss.length >= argss.length && + m.isAccessibleFrom(apply.symbol.info) && + m.vparamss + .zip(argss) + .reverse + .zipWithIndex + .forall { case (pair, index) => + FuzzyArgMatcher(m.tparams) + .doMatch(allArgsProvided = index != 0) + .tupled(pair) + } => + m + } + matchingMethods.getOrElse(Nil) + end fallbackFindMatchingMethods + + val matchingMethods: List[Symbols.Symbol] = + if method.symbol.paramSymss.nonEmpty + then + val allArgsAreSupplied = + val vparamss = method.symbol.vparamss + vparamss.length == argss.length && vparamss + .zip(argss) + .lastOption + .exists { case (baseParams, baseArgs) => + baseArgs.length == baseParams.length + } + // ``` + // m(arg : Int) + // m(arg : Int, anotherArg : Int) + // m(a@@) + // ``` + // complier will choose the first `m`, so we need to manually look for the other one + if allArgsAreSupplied then + val foundPotential = fallbackFindMatchingMethods() + if foundPotential.contains(method.symbol) then foundPotential + else method.symbol :: foundPotential + else List(method.symbol) + else fallbackFindMatchingMethods() + end if + end matchingMethods + + val allParams = matchingMethods.flatMap { methodSym => + val vparamss = methodSym.vparamss + + // get params and args we are interested in + // e.g. + // in the following case, the interesting args and params are + // - params: [apple, banana] + // - args: [apple, b] + // ``` + // def curry(x: Int)(apple: String, banana: String) = ??? + // curry(1)(apple = "test", b@@) + // ``` + val (baseParams, baseArgs) = + vparamss.zip(argss).lastOption.getOrElse((Nil, Nil)) + + val args = ident + .map(i => baseArgs.filterNot(_ == i)) + .getOrElse(baseArgs) + .filterNot(isUselessLiteral) + + val isNamed: Set[Name] = args.iterator + .zip(baseParams.iterator) + // filter out synthesized args and default arg getters + .filterNot { + case (arg, _) if arg.symbol.denot.is(Flags.Synthetic) => true + case (Ident(name), _) => name.is(DefaultGetterName) // default args + case (Select(Ident(_), name), _) => + name.is(DefaultGetterName) // default args for apply method + case _ => false + } + .map { + case (NamedArg(name, _), _) => name + case (_, param) => param.name + } + .toSet + baseParams.filterNot(param => isNamed(param.name) || param.denot.is( Flags.Synthetic ) // filter out synthesized param, like evidence ) + } val prefix = ident @@ -152,7 +233,9 @@ object NamedArgCompletions: .getOrElse("") .replace(Cursor.value, "") val params: List[Symbol] = - allParams.filter(param => param.name.startsWith(prefix)) + allParams + .filter(param => param.name.startsWith(prefix)) + .distinctBy(sym => (sym.name, sym.info)) val completionSymbols = indexedContext.scopeSymbols def matchingTypesInScope(paramType: Type): List[String] = @@ -174,11 +257,11 @@ object NamedArgCompletions: def fillAllFields(): List[CompletionValue] = val suffix = "autofill" - val shouldShow = + def shouldShow = allParams.exists(param => param.name.startsWith(prefix)) - val isExplicitlyCalled = suffix.startsWith(prefix) - val hasParamsToFill = allParams.count(!_.is(Flags.HasDefault)) > 1 - if (shouldShow || isExplicitlyCalled) && hasParamsToFill && clientSupportsSnippets + def isExplicitlyCalled = suffix.startsWith(prefix) + def hasParamsToFill = allParams.count(!_.is(Flags.HasDefault)) > 1 + if clientSupportsSnippets && matchingMethods.length == 1 && (shouldShow || isExplicitlyCalled) && hasParamsToFill then val editText = allParams.zipWithIndex .collect { @@ -216,4 +299,58 @@ object NamedArgCompletions: ) ::: findPossibleDefaults() ::: fillAllFields() end contribute + extension (method: Symbols.Symbol) + def vparamss(using Context) = method.filteredParamss(_.isTerm) + def tparams(using Context) = method.filteredParamss(_.isType).flatten + def filteredParamss(f: Symbols.Symbol => Boolean)(using Context) = + method.paramSymss.filter(params => params.forall(f)) end NamedArgCompletions + +class FuzzyArgMatcher(tparams: List[Symbols.Symbol])(using Context): + + /** + * A heuristic for checking if the passed arguments match the method's arguments' types. + * For non-polymorphic methods we use the subtype relation (`<:<`) + * and for polymorphic methods we use a heuristic. + * We check the args types not the result type. + */ + def doMatch( + allArgsProvided: Boolean + )(expectedArgs: List[Symbols.Symbol], actualArgs: List[Tree]) = + (expectedArgs.length == actualArgs.length || + (!allArgsProvided && expectedArgs.length >= actualArgs.length)) && + actualArgs.zipWithIndex.forall { + case (Ident(name), _) if name.endsWith(Cursor.value) => true + case (NamedArg(name, arg), _) => + expectedArgs.exists { expected => + expected.name == name && (!arg.hasType || arg.typeOpt.unfold + .fuzzyArg_<:<(expected.info)) + } + case (arg, i) => + !arg.hasType || arg.typeOpt.unfold.fuzzyArg_<:<(expectedArgs(i).info) + } + + extension (arg: Type) + def fuzzyArg_<:<(expected: Type) = + if tparams.isEmpty then arg <:< expected + else arg <:< substituteTypeParams(expected) + def unfold = + arg match + case arg: TermRef => arg.underlying + case e => e + + private val substituteTypeParams: Type => Type = + case e if tparams.exists(_ == e.typeSymbol) => + val matchingParam = tparams.find(_ == e.typeSymbol).get + matchingParam.info match + case b @ TypeBounds(_, _) => WildcardType(b) + case _ => WildcardType + case o @ OrType(e1, e2) => + OrType(substituteTypeParams(e1), substituteTypeParams(e2), o.isSoft) + case AndType(e1, e2) => + AndType(substituteTypeParams(e1), substituteTypeParams(e2)) + case AppliedType(et, eparams) => + AppliedType(et, eparams.map(substituteTypeParams)) + case t => t + +end FuzzyArgMatcher diff --git a/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala index e66944a6ea4..0832f6eb76a 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala @@ -163,16 +163,10 @@ class CompletionArgSuite extends BaseCompletionSuite { | Option[Int](@@) |} |""".stripMargin, - """|x = : Int + """|x = : A |Main arg7 |""".stripMargin, topLines = Option(2), - compat = Map( - "3" -> - """|x = : A - |Main arg7 - |""".stripMargin - ), ) check( @@ -508,6 +502,7 @@ class CompletionArgSuite extends BaseCompletionSuite { |""".stripMargin, """|x: Int |""".stripMargin, + topLines = Some(1), ) check( @@ -619,6 +614,7 @@ class CompletionArgSuite extends BaseCompletionSuite { |} |""".stripMargin, """|foo = : Int + |fooBar = : Int |""".stripMargin, ) @@ -641,6 +637,7 @@ class CompletionArgSuite extends BaseCompletionSuite { """|case class A(val foo: Int, val fooBar: Int) |object A { | def apply(foo: Int): A = new A(foo, 3) + | def m = 3 |} | |object Main { @@ -651,8 +648,19 @@ class CompletionArgSuite extends BaseCompletionSuite { |} |""".stripMargin, """|foo = : Int + |fooBar = : Int |foo = a : Int + |fooBar = a : Int |""".stripMargin, + topLines = Some(4), + compat = Map( + "3" -> + """|foo = : Int + |foo = a : Int + |fooBar = : Int + |fooBar = a : Int + |""".stripMargin + ), ) check( @@ -722,4 +730,251 @@ class CompletionArgSuite extends BaseCompletionSuite { ), topLines = Some(4), ) + check( + "overloaded", + """|object Main { + | def m(inn : Int) = ??? + | def m(idd : Option[Int]) = ??? + | def k = m(i@@) + |} + |""".stripMargin, + """|idd = : Option[Int] + |inn = : Int + |""".stripMargin, + topLines = Some(2), + ) + + check( + "overloaded-with-param", + """|object Main { + | def m(idd : String, abb: Int): Int = ??? + | def m(inn : Int, uuu: Option[Int]): Int = ??? + | def m(inn : Int, aaa: Int): Int = ??? + | def k: Int = m(1, a@@) + |} + |""".stripMargin, + """|aaa = : Int + |assert(assertion: Boolean): Unit + |""".stripMargin, + topLines = Some(2), + ) + + check( + "overloaded-with-named-param", + """|object Main { + | def m(idd : String, abb: Int): Int = ??? + | def m(inn : Int, uuu: Option[Int]): Int = ??? + | def m(inn : Int, aaa: Int): Int = ??? + | def k: Int = m(inn = 1, a@@) + |} + |""".stripMargin, + """|aaa = : Int + |assert(assertion: Boolean): Unit + |""".stripMargin, + topLines = Some(2), + ) + + check( + "overloaded-generic", + """|object Main { + | val h = 3 + | val l : List[Int] = List(1,2,3) + | def m[T](inn : List[T], yy: Int, aaa: Int, abb: Option[Int]): Int = ??? + | def m[T](inn : List[T], yy: Int, aaa: Int, abb: Int): Int = ??? + | def k: Int = m(yy = 3, inn = l, a@@) + |} + |""".stripMargin, + """|aaa = : Int + |abb = : Int + |abb = : Option[Int] + |aaa = h : Int + |abb = h : Int + |""".stripMargin, + compat = Map( + "3" -> + """|aaa = : Int + |aaa = h : Int + |abb = : Option[Int] + |abb = : Int + |abb = h : Int + |""".stripMargin + ), + topLines = Some(5), + ) + + check( + "overloaded-methods", + """|class A() { + | def m(anInt : Int): Int = ??? + | def m(aString : String): String = ??? + |} + |object O { + | def m(aaa: Int): Int = ??? + | val k = new A().m(a@@) + |} + |""".stripMargin, + """|aString = : String + |anInt = : Int + |""".stripMargin, + topLines = Some(2), + ) + + // type of `myInstance` is resolved to `null` for Scala 2 + check( + "overloaded-methods2".tag(IgnoreScala2), + """|class A() { + | def m(anInt : Int): Int = ??? + | def m(aString : String): String = ??? + | private def m(aBoolean: Boolean): Boolean = ??? + |} + |object O { + | def m(aaa: Int): Int = ??? + | val myInstance = new A() + | val k = myInstance.m(a@@) + |} + |""".stripMargin, + """|aString = : String + |anInt = : Int + |""".stripMargin, + topLines = Some(2), + ) + + check( + "overloaded-select", + """|package a.b { + | object A { + | def m(anInt : Int): Int = ??? + | def m(aString : String): String = ??? + | } + |} + |object O { + | def m(aaa: Int): Int = ??? + | val k = a.b.A.m(a@@) + |} + |""".stripMargin, + """|aString = : String + |anInt = : Int + |""".stripMargin, + topLines = Some(2), + ) + + check( + "overloaded-in-a-class", + """|trait Planet + |case class Venus() extends Planet + |class Main[T <: Planet](t : T) { + | def m(inn: Planet, abb: Option[Int]): Int = ??? + | def m(inn: Planet, aaa: Int): Int = ??? + | def k = m(t, a@@) + |} + |""".stripMargin, + """|aaa = : Int + |abb = : Option[Int] + |""".stripMargin, + topLines = Some(2), + ) + + check( + "overloaded-function-param".tag(IgnoreScala2), + """|def m[T](i: Int)(inn: T => Int, abb: Option[Int]): Int = ??? + |def m[T](i: Int)(inn: T => Int, aaa: Int): Int = ??? + |def m[T](i: Int)(inn: T => String, acc: List[Int]): Int = ??? + |def k = m(1)(inn = identity[Int], a@@) + |""".stripMargin, + """|aaa = : Int + |abb = : Option[Int] + |assert(assertion: Boolean): Unit + |""".stripMargin, + topLines = Some(3), + ) + + check( + "overloaded-function-param2".tag(IgnoreScala2), + """|def m[T](i: Int)(inn: T => Int, abb: Option[Int]): Int = ??? + |def m[T](i: Int)(inn: T => Int, aaa: Int): Int = ??? + |def m[T](i: String)(inn: T => Int, acc: List[Int]): Int = ??? + |def k = m(1)(inn = identity[Int], a@@) + |""".stripMargin, + """|aaa = : Int + |abb = : Option[Int] + |assert(assertion: Boolean): Unit + |""".stripMargin, + topLines = Some(3), + ) + + // doesn't filter properly for Scala 2, since the filtering heuristic for matching methods + // isn't accurate enough (polymorphic args types are resolved to `null`) + check( + "overloaded-function-param3".tag(IgnoreScala2), + """|object M { + | def m[T](inn: Int => T, abb: Option[Int]): Int = ??? + | def m[T](inn: String => T, aaa: Int): Int = ??? + | def k = m(identity[Int], a@@) + |} + |""".stripMargin, + """|abb = : Option[Int] + |""".stripMargin, + topLines = Some(1), + ) + + check( + "overloaded-bounds".tag(IgnoreScala2), + """|trait Planet + |case class Moon() + |object Main { + | def m[M](inn: M, abb: Option[Int]): T = ??? + | def m[M](inn: M, acc: List[Int]): T = ??? + | def m[M <: Planet](inn: M, aaa: Int): T = ??? + | def k: T = m(Moon(), a@@) + | + |""".stripMargin, + """|abb = : Option[Int] + |acc = : List[Int] + |assert(assertion: Boolean): Unit + |""".stripMargin, + topLines = Some(3), + ) + + check( + "overloaded-or-type".tag(IgnoreScala2), + """|object Main: + | val h : Int = 3 + | def m[T](inn: String | T, abb: Option[Int]): Int = ??? + | def m(inn: Int, aaa: Int): Int = ??? + | def k: Int = m(3, a@@) + |""".stripMargin, + """|aaa = : Int + |aaa = h : Int + |abb = : Option[Int] + |""".stripMargin, + topLines = Some(3), + ) + + check( + "overloaded-applied-type".tag(IgnoreScala2), + """|trait MyCollection[+T] + |class IntCollection() extends MyCollection[Int] + |def m[T](inn: MyCollection[T], abb: Option[Int]): Int = ??? + |def m[T](inn: MyCollection[T], aaa: Int): Int = ??? + |def k = m(IntCollection(), a@@) + |""".stripMargin, + """|aaa = : Int + |abb = : Option[Int] + |""".stripMargin, + topLines = Some(2), + ) + + check( + "overloaded-extension-methods".ignore, + """|extension(i: Int) + | def m(inn : Int, aaa : Int): Int = ??? + | def m(inn : Int, abb : Option[Int]): Int = ??? + | + |val k = 1.m(3, a@@) + |""".stripMargin, + """|aaa = : Int + |abb = : Option[Int] + |""".stripMargin, + topLines = Some(2), + ) } From 8ed55eed212c42c19ee538d1e8d58b3ddd0f8f45 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 12 Jun 2023 14:44:52 +0100 Subject: [PATCH 2/5] `ArgCompletions` refactor --- .../pc/completions/ArgCompletions.scala | 90 ++++++++++--------- 1 file changed, 46 insertions(+), 44 deletions(-) diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala index a0c83a39c4d..29ff7765dd8 100644 --- a/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala +++ b/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala @@ -15,41 +15,42 @@ trait ArgCompletions { this: MetalsGlobal => pos: Position, text: String, completions: CompletionResult - ) extends CompletionPosition { + ) extends CompletionPosition { self => val editRange: l.Range = pos.withStart(ident.pos.start).withEnd(pos.start).toLsp val funPos = apply.fun.pos val method: Tree = typedTreeAt(funPos) val methodSym = method.symbol - lazy val baseParamss: List[List[Symbol]] = { + lazy val methodsParams: List[MethodParams] = { if (methodSym.isModule) getParamsFromObject(methodSym) else if ( methodSym.isMethod && methodSym.decodedName == "apply" && methodSym.owner.isModuleClass ) getParamsFromObject(methodSym.owner) - else if (methodSym.isClass) List(methodSym.constrParamAccessors) + else if (methodSym.isClass) + List(MethodParams(methodSym.constrParamAccessors)) else if (method.tpe == null) method match { case Ident(name) => metalsScopeMembers(funPos) - .map { + .flatMap { case m: Member - if m.symNameDropLocal == name && m.sym != NoSymbol && m.sym.paramss.nonEmpty && checkIfArgsMatch( - m.sym.paramss.head - ) => - m.sym.paramss.head - case _ => Nil + if m.symNameDropLocal == name && m.sym != NoSymbol => + for { + params <- m.sym.paramss.headOption + if checkIfArgsMatch(params) + } yield MethodParams(params) + case _ => None } case _ => Nil } else { method.tpe match { case OverloadedType(_, alts) => - alts.flatMap(_.info.paramss.headOption) + alts.flatMap(_.info.paramss.headOption).map(MethodParams(_)) case _ => - List( - method.tpe.paramss.headOption - .getOrElse(methodSym.paramss.flatten) - ) + val params = + method.tpe.paramss.headOption.getOrElse(methodSym.paramss.flatten) + List(MethodParams(params)) } } } @@ -67,29 +68,9 @@ trait ArgCompletions { this: MetalsGlobal => rhs.tpe != null && !(rhs.tpe <:< possibleMatch(i).tpe) } } - lazy val baseParamssWithisNamed: List[(List[Symbol], Set[Name])] = - baseParamss.map { baseParams => - val isNamed = - apply.args.iterator - .filterNot(_ == ident) - .zip(baseParams.iterator) - .map { - case (AssignOrNamedArg(Ident(name), _), _) => - name - case (_, param) => - param.name - } - .toSet - (baseParams, isNamed) - } + val prefix: String = ident.name.toString.stripSuffix(CURSOR) - lazy val allParams: List[Symbol] = baseParamssWithisNamed.flatMap { - case (baseParams, isNamed) => - baseParams.iterator.filterNot { param => - isNamed(param.name) || - param.isSynthetic - }.toList - } + lazy val allParams: List[Symbol] = methodsParams.flatMap(_.allParams) lazy val params: List[Symbol] = allParams .filter(param => param.name.startsWith(prefix)) @@ -165,7 +146,7 @@ trait ArgCompletions { this: MetalsGlobal => val isExplicitlyCalled = suffix.startsWith(prefix) val hasParamsToFill = allParams.count(!_.hasDefault) > 1 if ( - baseParamss.length == 1 && (shouldShow || isExplicitlyCalled) && hasParamsToFill && clientSupportsSnippets + methodsParams.length == 1 && (shouldShow || isExplicitlyCalled) && hasParamsToFill && clientSupportsSnippets ) { val editText = allParams.zipWithIndex .collect { @@ -206,13 +187,14 @@ trait ArgCompletions { this: MetalsGlobal => } } - private def getParamsFromObject(objectSym: Symbol): List[List[Symbol]] = { - objectSym.info.members.collect { - case m - if m.decodedName == "apply" && m.paramss.nonEmpty && checkIfArgsMatch( - m.paramss.head - ) => - m.paramss.head + private def getParamsFromObject(objectSym: Symbol): List[MethodParams] = { + objectSym.info.members.flatMap { + case m if m.decodedName == "apply" => + for { + params <- m.paramss.headOption + if (checkIfArgsMatch(params)) + } yield MethodParams(params) + case _ => None }.toList } @@ -221,5 +203,25 @@ trait ArgCompletions { this: MetalsGlobal => new NamedArgMember(param) ) ::: findPossibleDefaults() ::: fillAllFields() } + + case class MethodParams(params: List[Symbol], isNamed: Set[Name]) { + def allParams: List[Symbol] = + params.filterNot(param => isNamed(param.name) || param.isSynthetic) + } + + object MethodParams { + def apply(baseParams: List[Symbol]): MethodParams = { + val isNamed = + self.apply.args.iterator + .filterNot(_ == ident) + .zip(baseParams.iterator) + .map { + case (AssignOrNamedArg(Ident(name), _), _) => name + case (_, param) => param.name + } + .toSet + MethodParams(baseParams, isNamed) + } + } } } From 4cefa4b30d037276a6598f473cd7d6ccfa1e618b Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Tue, 13 Jun 2023 14:07:30 +0100 Subject: [PATCH 3/5] `FuzzyArgMatcher.substituteTypeParams` refactor --- .../pc/completions/NamedArgCompletions.scala | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala index 59a4b5b0433..57a55444c57 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala @@ -339,18 +339,19 @@ class FuzzyArgMatcher(tparams: List[Symbols.Symbol])(using Context): case arg: TermRef => arg.underlying case e => e - private val substituteTypeParams: Type => Type = - case e if tparams.exists(_ == e.typeSymbol) => - val matchingParam = tparams.find(_ == e.typeSymbol).get - matchingParam.info match - case b @ TypeBounds(_, _) => WildcardType(b) - case _ => WildcardType - case o @ OrType(e1, e2) => - OrType(substituteTypeParams(e1), substituteTypeParams(e2), o.isSoft) - case AndType(e1, e2) => - AndType(substituteTypeParams(e1), substituteTypeParams(e2)) - case AppliedType(et, eparams) => - AppliedType(et, eparams.map(substituteTypeParams)) - case t => t + private def substituteTypeParams(t: Type): Type = + t match + case e if tparams.exists(_ == e.typeSymbol) => + val matchingParam = tparams.find(_ == e.typeSymbol).get + matchingParam.info match + case b @ TypeBounds(_, _) => WildcardType(b) + case _ => WildcardType + case o @ OrType(e1, e2) => + OrType(substituteTypeParams(e1), substituteTypeParams(e2), o.isSoft) + case AndType(e1, e2) => + AndType(substituteTypeParams(e1), substituteTypeParams(e2)) + case AppliedType(et, eparams) => + AppliedType(et, eparams.map(substituteTypeParams)) + case _ => t end FuzzyArgMatcher From fb7e86f8e8bcf991aa628b0a96e8cc5d4993c802 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Wed, 5 Jul 2023 13:31:04 +0200 Subject: [PATCH 4/5] review refactor --- .../pc/completions/ArgCompletions.scala | 7 ++- .../pc/completions/NamedArgCompletions.scala | 3 +- .../scala/tests/pc/CompletionArgSuite.scala | 59 ++++++++++++------- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala index 29ff7765dd8..598b4aa9770 100644 --- a/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala +++ b/mtags/src/main/scala-2/scala/meta/internal/pc/completions/ArgCompletions.scala @@ -24,7 +24,7 @@ trait ArgCompletions { this: MetalsGlobal => lazy val methodsParams: List[MethodParams] = { if (methodSym.isModule) getParamsFromObject(methodSym) else if ( - methodSym.isMethod && methodSym.decodedName == "apply" && methodSym.owner.isModuleClass + methodSym.isMethod && methodSym.name == nme.apply && methodSym.owner.isModuleClass ) getParamsFromObject(methodSym.owner) else if (methodSym.isClass) List(MethodParams(methodSym.constrParamAccessors)) @@ -58,6 +58,9 @@ trait ArgCompletions { this: MetalsGlobal => def checkIfArgsMatch(possibleMatch: List[Symbol]): Boolean = { apply.args.length <= possibleMatch.length && !apply.args.zipWithIndex.exists { + // the identifier we want to generate completions for + // v + // m(arg1 = 3, wri@@) case (Ident(name), _) if name.decodedName.endsWith(CURSOR) => false case (AssignOrNamedArg(Ident(name), rhs), _) => !possibleMatch.exists { param => @@ -189,7 +192,7 @@ trait ArgCompletions { this: MetalsGlobal => private def getParamsFromObject(objectSym: Symbol): List[MethodParams] = { objectSym.info.members.flatMap { - case m if m.decodedName == "apply" => + case m if m.name == nme.apply => for { params <- m.paramss.headOption if (checkIfArgsMatch(params)) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala index 57a55444c57..321163babfc 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/NamedArgCompletions.scala @@ -142,7 +142,8 @@ object NamedArgCompletions: case m if m.is(Flags.Method) && m.vparamss.length >= argss.length && - m.isAccessibleFrom(apply.symbol.info) && + Try(m.isAccessibleFrom(apply.symbol.info)).toOption + .getOrElse(false) && m.vparamss .zip(argss) .reverse diff --git a/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala index 0832f6eb76a..116d46c31bb 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala @@ -501,8 +501,18 @@ class CompletionArgSuite extends BaseCompletionSuite { |} |""".stripMargin, """|x: Int + |x = : Byte + |x = : Char + |x = : Double + |x = : Float + |x = : Int + |x = : Long + |x = : Short + |x = x : Int |""".stripMargin, - topLines = Some(1), + compat = Map( + "3" -> "x: Int" + ), ) check( @@ -874,6 +884,7 @@ class CompletionArgSuite extends BaseCompletionSuite { topLines = Some(2), ) + // In Scala 2 first argument list needs to disambiguate between overloaded methods. check( "overloaded-function-param".tag(IgnoreScala2), """|def m[T](i: Int)(inn: T => Int, abb: Option[Int]): Int = ??? @@ -904,17 +915,23 @@ class CompletionArgSuite extends BaseCompletionSuite { // doesn't filter properly for Scala 2, since the filtering heuristic for matching methods // isn't accurate enough (polymorphic args types are resolved to `null`) + // issue: https://github.com/scalameta/metals/issues/5406 check( - "overloaded-function-param3".tag(IgnoreScala2), - """|object M { - | def m[T](inn: Int => T, abb: Option[Int]): Int = ??? - | def m[T](inn: String => T, aaa: Int): Int = ??? - | def k = m(identity[Int], a@@) + "overloaded-applied-type".tag(IgnoreScala2), + """|trait MyCollection[+T] + |case class IntCollection() extends MyCollection[Int] + |object Main { + | def m[T](inn: MyCollection[T], abb: Option[Int]): Int = ??? + | def m[T](inn: MyCollection[T], aaa: Int): Int = ??? + | def m[T](inn: List[T], acc: Int): Int = ??? + | def k = m(IntCollection(), a@@) |} |""".stripMargin, - """|abb = : Option[Int] + """|aaa = : Int + |abb = : Option[Int] + |assert(assertion: Boolean): Unit |""".stripMargin, - topLines = Some(1), + topLines = Some(3), ) check( @@ -922,11 +939,11 @@ class CompletionArgSuite extends BaseCompletionSuite { """|trait Planet |case class Moon() |object Main { - | def m[M](inn: M, abb: Option[Int]): T = ??? - | def m[M](inn: M, acc: List[Int]): T = ??? - | def m[M <: Planet](inn: M, aaa: Int): T = ??? - | def k: T = m(Moon(), a@@) - | + | def m[M](inn: M, abb: Option[Int]): M = ??? + | def m[M](inn: M, acc: List[Int]): M = ??? + | def m[M <: Planet](inn: M, aaa: Int): M = ??? + | def k = m(Moon(), a@@) + |} |""".stripMargin, """|abb = : Option[Int] |acc = : List[Int] @@ -951,19 +968,17 @@ class CompletionArgSuite extends BaseCompletionSuite { ) check( - "overloaded-applied-type".tag(IgnoreScala2), - """|trait MyCollection[+T] - |class IntCollection() extends MyCollection[Int] - |def m[T](inn: MyCollection[T], abb: Option[Int]): Int = ??? - |def m[T](inn: MyCollection[T], aaa: Int): Int = ??? - |def k = m(IntCollection(), a@@) + "overloaded-function-param3".tag(IgnoreScala2), + """|def m[T](inn: Int => T, abb: Option[Int]): Int = ??? + |def m[T](inn: String => T, aaa: Int): Int = ??? + |def k = m(identity[Int], a@@) |""".stripMargin, - """|aaa = : Int - |abb = : Option[Int] + """|abb = : Option[Int] |""".stripMargin, - topLines = Some(2), + topLines = Some(1), ) + // issue: https://github.com/scalameta/metals/issues/5407 check( "overloaded-extension-methods".ignore, """|extension(i: Int) From 033517b17ef79bf3bfe831fbbab7d8444847b91c Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Thu, 20 Jul 2023 10:56:38 +0200 Subject: [PATCH 5/5] bugfix: don't show arg completions for infix --- .../internal/pc/completions/Completions.scala | 16 +++++++++++++++- .../test/scala/tests/pc/CompletionArgSuite.scala | 11 ----------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/completions/Completions.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/completions/Completions.scala index 67a2e5210bf..ceba2fc348f 100644 --- a/mtags/src/main/scala-2/scala/meta/internal/pc/completions/Completions.scala +++ b/mtags/src/main/scala-2/scala/meta/internal/pc/completions/Completions.scala @@ -455,6 +455,18 @@ trait Completions { this: MetalsGlobal => ident: Ident, apply: Apply ): CompletionPosition = { + def isInfix(apply: Apply, text: String) = + apply.fun match { + case Select(New(_), _) => false + case Select(_, name) if name.decoded == "apply" => false + case Select(This(_), _) => false + // is a select statement without a dot `qual.name` + case Select(qual, _) => { + val pos = qual.pos.end + pos < text.length() && text(pos) != '.' + } + case _ => false + } if (hasLeadingBrace(ident, text)) { if (isCasePrefix(ident.name)) { val moveToNewLine = ident.pos.line == apply.pos.line @@ -473,7 +485,9 @@ trait Completions { this: MetalsGlobal => NoneCompletion } } else { - ArgCompletion(ident, apply, pos, text, completions) + if (!isInfix(apply, text)) { + ArgCompletion(ident, apply, pos, text, completions) + } else NoneCompletion } } diff --git a/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala index 116d46c31bb..86cb7897e85 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionArgSuite.scala @@ -501,18 +501,7 @@ class CompletionArgSuite extends BaseCompletionSuite { |} |""".stripMargin, """|x: Int - |x = : Byte - |x = : Char - |x = : Double - |x = : Float - |x = : Int - |x = : Long - |x = : Short - |x = x : Int |""".stripMargin, - compat = Map( - "3" -> "x: Int" - ), ) check(