From f1e553ecfbea25bcc7e13294f6104d599336f460 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 29 Aug 2015 21:42:29 +1000 Subject: SI-9029 Fix regression in extractor patterns The unified treatment of classical and named-based pattern matching does not correctly handle the generalization of "tuple capture". By "tuple capture", I mean: ``` scala> object Extractor { def unapply(a: Any): Option[(Int, String)] = Some((1, "2")) } defined object Extractor scala> "" match { case Extractor(x: Int, y: String) => } scala> "" match { case Extractor(xy : (Int, String)) => } warning: there was one deprecation warning; re-run with -deprecation for details scala> :warnings :9: warning: object Extractor expects 2 patterns to hold (Int, String) but crushing into 2-tuple to fit single pattern (SI-6675) "" match { case Extractor(xy : (Int, String)) => } ^ ``` Name based pattern matching, new in Scala 2.11, allows one to deconstruct the elements that structurally resembles `ProductN`: ``` scala> class P2(val _1: Int, val _2: String) defined class P2 scala> object Extractor { def unapply(a: Any): Option[P2] = Some(new P2(1, "2")) } defined object Extractor scala> "" match { case Extractor(x: Int, y: String) => } ``` However, attempting to extract the `P2` in its entirety leads to an internal error: ``` scala> "" match { case Extractor(p2: P2) => } :10: warning: fruitless type test: a value of type (Int, String) cannot also be a P2 "" match { case Extractor(p2: P2) => } ^ :10: error: error during expansion of this match (this is a scalac bug). The underlying error was: type mismatch; found : P2 required: (Int, String) "" match { case Extractor(p2: P2) => } ^ ``` Note that this match was legal and warning free in 2.10. This commit avoids the hard-coded assumption that the "tuple capture" results in a `TupleN`, and instead keeps track of the product-ish type from which we extracted the element types. I have also opted not to limit the deprecation warning to `TupleN` extractors. --- test/files/run/t9029.flags | 1 + test/files/run/t9029.scala | 15 +++++++++++++++ test/files/run/t9029b.check | 1 + test/files/run/t9029b.scala | 31 +++++++++++++++++++++++++++++++ test/files/run/t9029c.scala | 21 +++++++++++++++++++++ 5 files changed, 69 insertions(+) create mode 100644 test/files/run/t9029.flags create mode 100644 test/files/run/t9029.scala create mode 100644 test/files/run/t9029b.check create mode 100644 test/files/run/t9029b.scala create mode 100644 test/files/run/t9029c.scala (limited to 'test') diff --git a/test/files/run/t9029.flags b/test/files/run/t9029.flags new file mode 100644 index 0000000000..dcc59ebe32 --- /dev/null +++ b/test/files/run/t9029.flags @@ -0,0 +1 @@ +-deprecation diff --git a/test/files/run/t9029.scala b/test/files/run/t9029.scala new file mode 100644 index 0000000000..c01033b76e --- /dev/null +++ b/test/files/run/t9029.scala @@ -0,0 +1,15 @@ +class Y(val _2: Int, val _1: String) + +object X { def unapply(u: Unit): Option[Y] = Some(new Y(42, "!")) } + +object Test { + def test1 = { + val X(y) = () + val yy: Y = y + assert(yy._1 == "!") + assert(yy._2 == 42) + } + def main(args: Array[String]): Unit = { + test1 + } +} diff --git a/test/files/run/t9029b.check b/test/files/run/t9029b.check new file mode 100644 index 0000000000..aeb2d5e239 --- /dev/null +++ b/test/files/run/t9029b.check @@ -0,0 +1 @@ +Some(1) diff --git a/test/files/run/t9029b.scala b/test/files/run/t9029b.scala new file mode 100644 index 0000000000..764d0771ec --- /dev/null +++ b/test/files/run/t9029b.scala @@ -0,0 +1,31 @@ +class Foo(val x: Bar) { + def isEmpty = false + def get = x +} + +object Foo { + def unapply(x: Foo) = x +} + +class Bar(val x: Option[Int], val y: Option[Int]) { + def isEmpty = false + def get = this + def _1 = x + def _2 = y +} + +object Bar { + def unapply(x: Bar) = x +} + +object Test { + def nameBased: Unit = { + val x: AnyRef = new Foo(new Bar(Some(1), Some(2))) + x match { + case Foo(Bar(x1, x2)) => println(x1) + } + } + def main(args: Array[String]): Unit = { + nameBased + } +} diff --git a/test/files/run/t9029c.scala b/test/files/run/t9029c.scala new file mode 100644 index 0000000000..ccb51e23ae --- /dev/null +++ b/test/files/run/t9029c.scala @@ -0,0 +1,21 @@ +object Extractor { + def unapply(a: Any): Option[Product2[Int, String]] = Some(new P2(1, "2")) +} +class P2[A, B](val _1: A, val _2: B) extends Product2[A, B] { + def canEqual(other: Any) = true + def isP2 = true +} + +object Test { + def main(args: Array[String]): Unit = { + "" match { + case Extractor(p) => + val pp: Product2[Int, String] = p + } + "" match { + case Extractor(x, y) => + val xx: Int = x + val yy: String = y + } + } +} -- cgit v1.2.3 From daa421189d9191f05b5006418580eb6c0e0b1ec7 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 31 Aug 2015 13:44:54 +1000 Subject: SI-8989 Better error message for invalid extractor pattern --- .../transform/patmat/ScalacPatternExpanders.scala | 11 ++++++++--- test/files/neg/t4425b.check | 22 +++++++++++++++++----- test/files/neg/t8127a.check | 4 ++++ test/files/neg/t8127a.scala | 12 ++++++++++++ test/files/neg/t8989.check | 4 ++++ test/files/neg/t8989.scala | 14 ++++++++++++++ 6 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 test/files/neg/t8127a.check create mode 100644 test/files/neg/t8127a.scala create mode 100644 test/files/neg/t8989.check create mode 100644 test/files/neg/t8989.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala b/src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala index 55d4366a46..5678c53e78 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala @@ -74,11 +74,16 @@ trait ScalacPatternExpanders { * Unfortunately the MethodType does not carry the information of whether * it was unapplySeq, so we have to funnel that information in separately. */ - def unapplyMethodTypes(whole: Type, result: Type, isSeq: Boolean): Extractor = { + def unapplyMethodTypes(context: Context, whole: Type, result: Type, isSeq: Boolean): Extractor = { if (result =:= BooleanTpe) newExtractor(whole, Nil, NoRepeated) else { val getResult = typeOfMemberNamedGet(result) + def noGetError() = { + val name = "unapply" + (if (isSeq) "Seq" else "") + context.error(context.tree.pos, s"The result type of an $name method must contain a member `get` to be used as an extractor pattern, no such member exists in ${result}") + } val expanded = getResult match { + case global.NoType => noGetError(); Nil case rawGet if !hasSelectors(rawGet) => rawGet :: Nil case rawGet => typesOfSelectors(rawGet) } @@ -131,8 +136,8 @@ trait ScalacPatternExpanders { val isUnapply = sel.symbol.name == nme.unapply val extractor = sel.symbol.name match { - case nme.unapply => unapplyMethodTypes(firstParamType(fn.tpe), sel.tpe, isSeq = false) - case nme.unapplySeq => unapplyMethodTypes(firstParamType(fn.tpe), sel.tpe, isSeq = true) + case nme.unapply => unapplyMethodTypes(context, firstParamType(fn.tpe), sel.tpe, isSeq = false) + case nme.unapplySeq => unapplyMethodTypes(context, firstParamType(fn.tpe), sel.tpe, isSeq = true) case _ => applyMethodTypes(fn.tpe) } diff --git a/test/files/neg/t4425b.check b/test/files/neg/t4425b.check index 8418b4fd12..a204467586 100644 --- a/test/files/neg/t4425b.check +++ b/test/files/neg/t4425b.check @@ -22,16 +22,28 @@ t4425b.scala:10: error: object X is not a case class, nor does it have an unappl Note: def unapply(x: String)(y: String): Nothing exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list println((X: Any) match { case X(_, _) => "ok" ; case _ => "fail" }) ^ -t4425b.scala:18: error: too many patterns for object X: expected 1, found 2 +t4425b.scala:18: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing + println( "" match { case _ X _ => "ok" ; case _ => "fail" }) + ^ +t4425b.scala:18: error: too many patterns for object X offering Boolean: expected 0, found 2 println( "" match { case _ X _ => "ok" ; case _ => "fail" }) ^ -t4425b.scala:19: error: too many patterns for object X: expected 1, found 2 +t4425b.scala:19: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing + println((X: Any) match { case _ X _ => "ok" ; case _ => "fail" }) + ^ +t4425b.scala:19: error: too many patterns for object X offering Boolean: expected 0, found 2 println((X: Any) match { case _ X _ => "ok" ; case _ => "fail" }) ^ -t4425b.scala:22: error: too many patterns for object X: expected 1, found 2 +t4425b.scala:20: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing + println( "" match { case X(_) => "ok" ; case _ => "fail" }) + ^ +t4425b.scala:21: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing + println((X: Any) match { case X(_) => "ok" ; case _ => "fail" }) + ^ +t4425b.scala:22: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing println( "" match { case X(_, _) => "ok" ; case _ => "fail" }) ^ -t4425b.scala:23: error: too many patterns for object X: expected 1, found 2 +t4425b.scala:23: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing println((X: Any) match { case X(_, _) => "ok" ; case _ => "fail" }) ^ t4425b.scala:31: error: too many patterns for object X offering Nothing: expected 1, found 2 @@ -46,4 +58,4 @@ t4425b.scala:35: error: too many patterns for object X offering Nothing: expecte t4425b.scala:36: error: too many patterns for object X offering Nothing: expected 1, found 2 println((X: Any) match { case X(_, _) => "ok" ; case _ => "fail" }) ^ -14 errors found +18 errors found diff --git a/test/files/neg/t8127a.check b/test/files/neg/t8127a.check new file mode 100644 index 0000000000..5a30574861 --- /dev/null +++ b/test/files/neg/t8127a.check @@ -0,0 +1,4 @@ +t8127a.scala:7: error: The result type of an unapplySeq method must contain a member `get` to be used as an extractor pattern, no such member exists in Seq[_$1] + case H(v) => + ^ +one error found diff --git a/test/files/neg/t8127a.scala b/test/files/neg/t8127a.scala new file mode 100644 index 0000000000..c05facdac1 --- /dev/null +++ b/test/files/neg/t8127a.scala @@ -0,0 +1,12 @@ +object H { + def unapplySeq(m: Any): Seq[_] = List("") +} + +object Test { + def unapply(m: Any) = m match { + case H(v) => + case _ => + } + // now: too many patterns for object H offering Boolean: expected 0, found 1 + // was: result type Seq[_$2] of unapplySeq defined in method unapplySeq in object H does not conform to Option[_] +} diff --git a/test/files/neg/t8989.check b/test/files/neg/t8989.check new file mode 100644 index 0000000000..4e89b862bd --- /dev/null +++ b/test/files/neg/t8989.check @@ -0,0 +1,4 @@ +t8989.scala:11: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in A + val f = p match {case d(1) => true; case _ => false} + ^ +one error found diff --git a/test/files/neg/t8989.scala b/test/files/neg/t8989.scala new file mode 100644 index 0000000000..8ed6a901cd --- /dev/null +++ b/test/files/neg/t8989.scala @@ -0,0 +1,14 @@ +class A extends Product1[Int] { + def _1 = 1 + def isEmpty = false // used by scalac + def isDefined = !isEmpty // used by dotty + def canEqual(a: Any) = true +} + +object d{ + def unapply(a: Any) = new A + val p: Any = ??? + val f = p match {case d(1) => true; case _ => false} +} + + -- cgit v1.2.3 From 54445becc8dc035898c39b965e54bbaed80d0b98 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 31 Aug 2015 13:55:00 +1000 Subject: SI-7850 Additional tests for name based patmat Found these in an old review branch of mine. --- test/files/run/t7850c.scala | 11 +++++++++++ test/files/run/t7850d.scala | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 test/files/run/t7850c.scala create mode 100644 test/files/run/t7850d.scala (limited to 'test') diff --git a/test/files/run/t7850c.scala b/test/files/run/t7850c.scala new file mode 100644 index 0000000000..25b9c0028d --- /dev/null +++ b/test/files/run/t7850c.scala @@ -0,0 +1,11 @@ +// Testing that isEmpty and get are viewed with `memberType` from `Casey1`. +trait T[A, B >: Null] { def isEmpty: A = false.asInstanceOf[A]; def get: B = null} +class Casey1() extends T[Boolean, String] +object Casey1 { def unapply(a: Casey1) = a } + +object Test { + def main(args: Array[String]) { + val c @ Casey1(x) = new Casey1() + assert(x == c.get) + } +} diff --git a/test/files/run/t7850d.scala b/test/files/run/t7850d.scala new file mode 100644 index 0000000000..ccc98f1bcc --- /dev/null +++ b/test/files/run/t7850d.scala @@ -0,0 +1,17 @@ +// Testing that the ad-hoc overload resolution of isEmpty/get discards +// parameter-accepting variants +trait T[A, B >: Null] { def isEmpty: A = false.asInstanceOf[A]; def get: B = null} +class Casey1(val a: Int) { + def isEmpty: Boolean = false + def isEmpty(x: Int): Boolean = ??? + def get: Int = a + def get(x: Int): String = ??? +} +object Casey1 { def unapply(a: Casey1) = a } + +object Test { + def main(args: Array[String]) { + val c @ Casey1(x) = new Casey1(0) + assert(x == c.get) + } +} -- cgit v1.2.3 From 2c9e506bc32248f9ae4929790a0cb7484a53a66e Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 24 Sep 2015 13:22:29 +1000 Subject: Support completion in erroneous string interpolation. In the code: ``` s"${fooo Date: Thu, 24 Sep 2015 14:27:58 +1000 Subject: Improve presentation compilation of annotations A trio of problems were hampering autocompletion of annotations. First, given that that annotation is written before the annotated member, it is very common to end parse incomplete code that has a floating annotation without an anotatee. The parser was discarding the annotations (ie, the modifiers) and emitting an `EmptyTree`. Second, the presetation compiler was only looking for annotations in the Modifiers of a member def, but after typechecking annotations are moved into the symbol. Third, if an annotation failed to typecheck, it was being discarded in place of `ErroneousAnnotation`. This commit: - modifies the parser to uses a dummy class- or type-def tree, instead of EmptyTree, which can carry the annotations. - updates the locator to look in the symbol annotations of the modifiers contains no annotations. - uses a separate instance of `ErroneousAnnotation` for each erroneous annotation, and stores the original tree in its `original` tree. --- src/compiler/scala/tools/nsc/ast/parser/Parsers.scala | 10 ++++++++-- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 1 + src/reflect/scala/reflect/internal/AnnotationInfos.scala | 2 +- src/reflect/scala/reflect/internal/Positions.scala | 9 ++++++++- src/reflect/scala/reflect/runtime/JavaUniverseForce.scala | 1 - test/junit/scala/tools/nsc/interpreter/CompletionTest.scala | 11 +++++++++++ 6 files changed, 29 insertions(+), 5 deletions(-) (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 70abdf6bc0..4494a8ac8d 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -2680,7 +2680,10 @@ self => case t if t == SUPERTYPE || t == SUBTYPE || t == COMMA || t == RBRACE || isStatSep(t) => TypeDef(mods | Flags.DEFERRED, name, tparams, typeBounds()) case _ => - syntaxErrorOrIncompleteAnd("`=', `>:', or `<:' expected", skipIt = true)(EmptyTree) + syntaxErrorOrIncompleteAnd("`=', `>:', or `<:' expected", skipIt = true)( + // assume a dummy type def so as to have somewhere to stash the annotations + TypeDef(mods, tpnme.ERROR, Nil, EmptyTree) + ) } } } @@ -2713,7 +2716,10 @@ self => case CASEOBJECT => objectDef(pos, (mods | Flags.CASE) withPosition (Flags.CASE, tokenRange(in.prev /*scanner skips on 'case' to 'object', thus take prev*/))) case _ => - syntaxErrorOrIncompleteAnd("expected start of definition", skipIt = true)(EmptyTree) + syntaxErrorOrIncompleteAnd("expected start of definition", skipIt = true)( + // assume a class definition so as to have somewhere to stash the annotations + atPos(pos)(gen.mkClassDef(mods, tpnme.ERROR, Nil, Template(Nil, noSelfType, Nil))) + ) } } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 8113cd9b96..6b73a538df 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -3558,6 +3558,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper def typedAnnotation(ann: Tree, mode: Mode = EXPRmode): AnnotationInfo = { var hasError: Boolean = false val pending = ListBuffer[AbsTypeError]() + def ErroneousAnnotation = new ErroneousAnnotation().setOriginal(ann) def finish(res: AnnotationInfo): AnnotationInfo = { if (hasError) { diff --git a/src/reflect/scala/reflect/internal/AnnotationInfos.scala b/src/reflect/scala/reflect/internal/AnnotationInfos.scala index 6863cdfd82..b923541b56 100644 --- a/src/reflect/scala/reflect/internal/AnnotationInfos.scala +++ b/src/reflect/scala/reflect/internal/AnnotationInfos.scala @@ -404,7 +404,7 @@ trait AnnotationInfos extends api.Annotations { self: SymbolTable => object UnmappableAnnotation extends CompleteAnnotationInfo(NoType, Nil, Nil) - object ErroneousAnnotation extends CompleteAnnotationInfo(ErrorType, Nil, Nil) + class ErroneousAnnotation() extends CompleteAnnotationInfo(ErrorType, Nil, Nil) /** Extracts symbol of thrown exception from AnnotationInfo. * diff --git a/src/reflect/scala/reflect/internal/Positions.scala b/src/reflect/scala/reflect/internal/Positions.scala index 4d0e31b037..15d68bcdfe 100644 --- a/src/reflect/scala/reflect/internal/Positions.scala +++ b/src/reflect/scala/reflect/internal/Positions.scala @@ -252,7 +252,14 @@ trait Positions extends api.Positions { self: SymbolTable => super.traverse(t) } else t match { case mdef: MemberDef => - traverseTrees(mdef.mods.annotations) + val annTrees = mdef.mods.annotations match { + case Nil if mdef.symbol != null => + // After typechecking, annotations are mvoed from the modifiers + // to the annotation on the symbol of the anotatee. + mdef.symbol.annotations.map(_.original) + case anns => anns + } + traverseTrees(annTrees) case _ => } } diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index 5477bdd6d4..7725e4a2f0 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -106,7 +106,6 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => this.AnnotationInfo this.Annotation this.UnmappableAnnotation - this.ErroneousAnnotation this.ThrownException this.typeNames this.tpnme diff --git a/test/junit/scala/tools/nsc/interpreter/CompletionTest.scala b/test/junit/scala/tools/nsc/interpreter/CompletionTest.scala index d00a9bf69a..78ebb7cf9c 100644 --- a/test/junit/scala/tools/nsc/interpreter/CompletionTest.scala +++ b/test/junit/scala/tools/nsc/interpreter/CompletionTest.scala @@ -51,6 +51,17 @@ class CompletionTest { assertEquals(List("prefix_aaa", "prefix_nnn", "prefix_zzz"), completer.complete( """class C { def prefix_nnn = 0; def prefix_zzz = 0; def prefix_aaa = 0; prefix_""").candidates) } + @Test + def annotations(): Unit = { + val intp = newIMain() + val completer = new PresentationCompilerCompleter(intp) + checkExact(completer, "def foo[@specialize", " A]")("specialized") + checkExact(completer, "def foo[@specialize")("specialized") + checkExact(completer, """@deprecatedN""", """ class Foo""")("deprecatedName") + checkExact(completer, """@deprecateN""")("deprecatedName") + checkExact(completer, """{@deprecateN""")("deprecatedName") + } + @Test def incompleteStringInterpolation(): Unit = { val intp = newIMain() -- cgit v1.2.3