Scala Programming Language
  1. Scala Programming Language
  2. SI-2337

Transition from pattern match based on extractor with a typed parameter in unapply() skips next case

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: Scala 2.10.0
    • Component/s: Pattern Matcher
    • Labels:
      None
    • Environment:

      pattern extractor transition

      Description

      Not sure if this is a known limitation, an existing bug, or a new bug in 2.8.

      I have a value 'element' of runtime type:

      class ScTypeParameterImpl extends 
        (trait ScTypeParam extends 
          (trait ScPolymorphicElement extends ScNamedElement)))
      
      trait ScNamedElement extends ... {
         // abstract defs
         // implementations
      }
      

      It was fed into the pattern match. I expected it to match x: !ScNamedElement. Instead, it matched the default case. I added an explicit case for 'x: !ScTypeParam' as a workaround.

          val name = element match {
            case RealPsiClass(c) => c.getQualifiedName
            case x: PsiMethod => PsiFormatUtil.formatMethod(x, PsiSubstitutor.EMPTY,
              PsiFormatUtilBase.SHOW_NAME | PsiFormatUtilBase.SHOW_PARAMETERS,
              PsiFormatUtilBase.SHOW_TYPE) + " of " + getDescriptiveName(x.getContainingClass)
            case x: PsiVariable => x.getName
            case x: PsiFile => x.getName
      
            case x: ScNamedElement => x.getName // todo This line should match, but doesn't.
            case x: ScTypeParam => (x: ScNamedElement).getName // todo Should even compile! Previous pattern should match!
      
            case _ => element.getText
          }
          Option(name) getOrElse "anonymous"
        }
      

      Taking a closer look with the debugger, it appears, at least from a Java perspective, that the class literal !ScNamedElement.class refers to the class containing the method definitions from trait !ScNamedElement, rather than its abstract interface.

      Debugger:

      element.getClass() = class org.jetbrains.plugins.scala.lang.psi.impl.statements.params.ScTypeParamImpl
      
      element.getClass().getInterfaces()r1.getInterfaces()r1.getInterfaces()r2 = {java.lang.Class@10972}"interface org.jetbrains.plugins.scala.lang.psi.api.toplevel.ScNamedElement"
      
      ScNamedElement.class = {java.lang.Class@10978}"class org.jetbrains.plugins.scala.lang.psi.api.toplevel.ScNamedElement$$class"
      
      ScNamedElement.class.isAssignableFrom(element.getClass()) == false
      
      Class.forName("org.jetbrains.plugins.scala.lang.psi.api.toplevel.ScNamedElement").isAssignableFrom(element.getClass()) = true
      

      My brief attempt to isolate this to a small testcase were unsuccessful. If this isn't enough to pinpoint the problem, and it really is a bug, I'll spend some more time at this.

      Tested against r18650

      thanks,

      -jason

      1. 2337.check
        0.0 kB
        Jira Admin
      2. 2337.scala
        0.3 kB
        Jira Admin

        Issue Links

          Activity

          Hide
          Paul Phillips added a comment -

          See SI-3479 for another failing example.

          Show
          Paul Phillips added a comment - See SI-3479 for another failing example.
          Hide
          Andrey added a comment -

          Another failing example (http://stackoverflow.com/questions/3915866/number-number-matches-float-int-but-does-not-match-int-float)

          import junit.framework._
          import Assert._
          
          class BugTest extends TestCase {
          
            def compare(first: Any, second: Any): Int = {
                (first, second) match {
                  case (k: Int, o: Int) => k compare o
                  //why the next case matches (Float, Int) but does not match (Int, Float) ???
                  case (k: Number, o: Number) => k.doubleValue() compare o.doubleValue()
                  case _ => throw new Exception("Unsupported compare " + first + "; " + second)
              }
            }
          
            def testCompare() {
              assertEquals("Both Int", -1, compare(0, 1))
              assertEquals("Both Float", 1, compare(1.0, 0.0))
              assertEquals("Float then Int", 0, compare(10.0, 10))
              assertEquals("Int then Float", 0, compare(10, 10.0))//this fails with an exception
            }
          }
          
          Show
          Andrey added a comment - Another failing example ( http://stackoverflow.com/questions/3915866/number-number-matches-float-int-but-does-not-match-int-float ) import junit.framework._ import Assert._ class BugTest extends TestCase { def compare(first: Any, second: Any): Int = { (first, second) match { case (k: Int, o: Int) => k compare o //why the next case matches (Float, Int) but does not match (Int, Float) ??? case (k: Number, o: Number) => k.doubleValue() compare o.doubleValue() case _ => throw new Exception("Unsupported compare " + first + "; " + second) } } def testCompare() { assertEquals("Both Int", -1, compare(0, 1)) assertEquals("Both Float", 1, compare(1.0, 0.0)) assertEquals("Float then Int", 0, compare(10.0, 10)) assertEquals("Int then Float", 0, compare(10, 10.0))//this fails with an exception } }
          Hide
          Alex Black added a comment -

          I believe this is another failing example:

          object App {
          object Extractor {
          def unapply(s: List[String]): Option[Boolean] =

          { if ( s == List("1", "2", "3") ) Some(true) else None }

          }

          def matcher1: PartialFunction[List[String], String] =

          { case Extractor(_) => "GOOD" case x @ List("foo") => "FOO" }

          def matcher2: PartialFunction[List[String], String] =

          { case x @ List("foo") => "FOO" case Extractor(_) => "GOOD" }

          def main(args: Array[String]): Unit =

          { val x = List("1", "2", "3") println(matcher1.isDefinedAt(x)) println(matcher2.isDefinedAt(x)) }

          }

          I expected the output to be:

          true
          true

          But, the output is: (on Scala 2.8.0 final)

          true
          false

          Show
          Alex Black added a comment - I believe this is another failing example: object App { object Extractor { def unapply(s: List [String] ): Option [Boolean] = { if ( s == List("1", "2", "3") ) Some(true) else None } } def matcher1: PartialFunction[List [String] , String] = { case Extractor(_) => "GOOD" case x @ List("foo") => "FOO" } def matcher2: PartialFunction[List [String] , String] = { case x @ List("foo") => "FOO" case Extractor(_) => "GOOD" } def main(args: Array [String] ): Unit = { val x = List("1", "2", "3") println(matcher1.isDefinedAt(x)) println(matcher2.isDefinedAt(x)) } } I expected the output to be: true true But, the output is: (on Scala 2.8.0 final) true false
          Hide
          Commit Message Bot added a comment -

          (moors in r25966) smarter bridges to unapplies

          wraps the call to a bridged synthetic unapply(Seq) in a defensive if-test:

          if (x.isInstanceOf[expectedType]) real.unapply(x.asInstanceOf[expectedType]) else None

          NOTE: the test is WRONG, but it has to be due to #1697/#2337 – once those are fixed, this one should generate the expected output

          Show
          Commit Message Bot added a comment - (moors in r25966 ) smarter bridges to unapplies wraps the call to a bridged synthetic unapply(Seq) in a defensive if-test: if (x.isInstanceOf [expectedType] ) real.unapply(x.asInstanceOf [expectedType] ) else None NOTE: the test is WRONG, but it has to be due to #1697/#2337 – once those are fixed, this one should generate the expected output
          Hide
          Paul Phillips added a comment -

          virtpatmat saves day, test in 8bc8b83f0b .

          Show
          Paul Phillips added a comment - virtpatmat saves day, test in 8bc8b83f0b .

            People

            • Assignee:
              Adriaan Moors
              Reporter:
              Jason Zaugg
              TracCC:
              Alex Black, Johannes Rudolph, Paul Phillips, Seth Tisue
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development