Improve logic when to emit pattern type error

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -161,7 +161,7 @@ private sealed trait WarningSettings:
   val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Werror", "Fail the compilation if there are any warnings.", aliases = List("-Xfatal-warnings"))
   val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.")
   val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
-
+  val WimplausiblePatterns = BooleanSetting("-Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.")
   val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
     name = "-Wunused",
     helpArg = "warning",

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -2009,20 +2009,21 @@ class Definitions {
     vcls
   }
 
+  def boxedClass(cls: Symbol): ClassSymbol =
+    if cls eq ByteClass then         BoxedByteClass
+    else if cls eq ShortClass then   BoxedShortClass
+    else if cls eq CharClass then    BoxedCharClass
+    else if cls eq IntClass then     BoxedIntClass
+    else if cls eq LongClass then    BoxedLongClass
+    else if cls eq FloatClass then   BoxedFloatClass
+    else if cls eq DoubleClass then  BoxedDoubleClass
+    else if cls eq UnitClass then    BoxedUnitClass
+    else if cls eq BooleanClass then BoxedBooleanClass
+    else sys.error(s"Not a primitive value type: $cls")
+
   /** The type of the boxed class corresponding to primitive value type `tp`. */
-  def boxedType(tp: Type)(using Context): TypeRef = {
-    val cls = tp.classSymbol
-    if (cls eq ByteClass)         BoxedByteClass
-    else if (cls eq ShortClass)   BoxedShortClass
-    else if (cls eq CharClass)    BoxedCharClass
-    else if (cls eq IntClass)     BoxedIntClass
-    else if (cls eq LongClass)    BoxedLongClass
-    else if (cls eq FloatClass)   BoxedFloatClass
-    else if (cls eq DoubleClass)  BoxedDoubleClass
-    else if (cls eq UnitClass)    BoxedUnitClass
-    else if (cls eq BooleanClass) BoxedBooleanClass
-    else sys.error(s"Not a primitive value type: $tp")
-  }.typeRef
+  def boxedType(tp: Type)(using Context): TypeRef =
+    boxedClass(tp.classSymbol).typeRef
 
   def unboxedType(tp: Type)(using Context): TypeRef = {
     val cls = tp.classSymbol

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -2020,8 +2020,10 @@ object SymDenotations {
      *  @return The result may contain false positives, but never false negatives.
      */
     final def mayHaveCommonChild(that: ClassSymbol)(using Context): Boolean =
-      !this.is(Final) && !that.is(Final) && (this.is(Trait) || that.is(Trait)) ||
-        this.derivesFrom(that) || that.derivesFrom(this.symbol)
+         this.is(Trait) && !that.isEffectivelyFinal
+      || that.is(Trait) && !this.isEffectivelyFinal
+      || this.derivesFrom(that)
+      || that.derivesFrom(this.symbol)
 
     final override def typeParamCreationFlags: FlagSet = ClassTypeParamCreationFlags
 

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -199,6 +199,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case ClosureCannotHaveInternalParameterDependenciesID // errorNumber: 183
   case MatchTypeNoCasesID // errorNumber: 184
   case UnimportedAndImportedID // errorNumber: 185
+  case ImplausiblePatternWarningID // erorNumber: 186
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -2938,3 +2938,11 @@ class ClosureCannotHaveInternalParameterDependencies(mt: Type)(using Context)
       i"""cannot turn method type $mt into closure
          |because it has internal parameter dependencies"""
     def explain(using Context) = ""
+
+class ImplausiblePatternWarning(pat: tpd.Tree, selType: Type)(using Context)
+  extends TypeMsg(ImplausiblePatternWarningID):
+    def msg(using Context) =
+      i"""|Implausible pattern:
+          |$pat  could match selector of type  $selType
+          |only if there is an `equals` method identifying elements of the two types."""
+    def explain(using Context) = ""

compiler/src/dotty/tools/dotc/typer/Linter.scala

@@ -0,0 +1,126 @@
+package dotty.tools
+package dotc
+package typer
+
+import core.*
+import Types.*, Contexts.*, Symbols.*, Flags.*, Constants.*
+import reporting.*
+import Decorators.i
+
+/** A module for linter checks done at typer */
+object Linter:
+  import ast.tpd.*
+
+  /** If -Wnonunit-statement is set, warn about statements in blocks that are non-unit expressions.
+   *  @return  true if a warning was issued, false otherwise
+   */
+  def warnOnInterestingResultInStatement(t: Tree)(using Context): Boolean =
+
+    def isUninterestingSymbol(sym: Symbol): Boolean =
+      sym == NoSymbol ||
+      sym.isConstructor ||
+      sym.is(Package) ||
+      sym.isPackageObject ||
+      sym == defn.BoxedUnitClass ||
+      sym == defn.AnyClass ||
+      sym == defn.AnyRefAlias ||
+      sym == defn.AnyValClass
+
+    def isUninterestingType(tpe: Type): Boolean =
+      tpe == NoType ||
+      tpe.typeSymbol == defn.UnitClass ||
+      defn.isBottomClass(tpe.typeSymbol) ||
+      tpe =:= defn.UnitType ||
+      tpe.typeSymbol == defn.BoxedUnitClass ||
+      tpe =:= defn.AnyValType ||
+      tpe =:= defn.AnyType ||
+      tpe =:= defn.AnyRefType
+
+    def isJavaApplication(t: Tree): Boolean = t match
+      case Apply(f, _) => f.symbol.is(JavaDefined) && !defn.ObjectClass.isSubClass(f.symbol.owner)
+      case _ => false
+
+    def checkInterestingShapes(t: Tree): Boolean = t match
+      case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) || checkInterestingShapes(elsepart)
+      case Block(_, res) => checkInterestingShapes(res)
+      case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body))
+      case _ => checksForInterestingResult(t)
+
+    def checksForInterestingResult(t: Tree): Boolean =
+         !t.isDef                               // ignore defs
+      && !isUninterestingSymbol(t.symbol)       // ctors, package, Unit, Any
+      && !isUninterestingType(t.tpe)            // bottom types, Unit, Any
+      && !isThisTypeResult(t)                   // buf += x
+      && !isSuperConstrCall(t)                  // just a thing
+      && !isJavaApplication(t)                  // Java methods are inherently side-effecting
+      // && !treeInfo.hasExplicitUnit(t)           // suppressed by explicit expr: Unit // TODO Should explicit `: Unit` be added as warning suppression?
+
+    if ctx.settings.WNonUnitStatement.value && !ctx.isAfterTyper && checkInterestingShapes(t) then
+      val where = t match
+        case Block(_, res) => res
+        case If(_, thenpart, Literal(Constant(()))) =>
+          thenpart match {
+            case Block(_, res) => res
+            case _ => thenpart
+          }
+        case _ => t
+      report.warning(UnusedNonUnitValue(where.tpe), t.srcPos)
+      true
+    else false
+  end warnOnInterestingResultInStatement
+
+  /** If -Wimplausible-patterns is set, warn about pattern values that can match the scrutinee
+   *  type only if there would be some user-defined equality method that equates values of the
+   *  two types.
+   */
+  def warnOnImplausiblePattern(pat: Tree, selType: Type)(using Context): Unit =
+      // approximate type params with bounds
+    def approx = new ApproximatingTypeMap {
+      var alreadyExpanding: List[TypeRef] = Nil
+      def apply(tp: Type) = tp.dealias match
+        case tp: TypeRef if !tp.symbol.isClass =>
+          if alreadyExpanding contains tp then tp else
+            val saved = alreadyExpanding
+            alreadyExpanding ::= tp
+            val res = expandBounds(tp.info.bounds)
+            alreadyExpanding = saved
+            res
+        case _ =>
+          mapOver(tp)
+    }
+
+    // Is it possible that a value of `clsA` is equal to a value of `clsB`?
+    //  This ignores user-defined equals methods, but makes an exception
+    //  for numeric classes.
+    def canOverlap(clsA: ClassSymbol, clsB: ClassSymbol): Boolean =
+      clsA.mayHaveCommonChild(clsB)
+      || clsA.isNumericValueClass // this is quite coarse, but matches to what was done before
+      || clsB.isNumericValueClass
+
+    // Can type `a` possiblly have a common instance with type `b`?
+    def canEqual(a: Type, b: Type): Boolean = trace(i"canEqual $a $b"):
+      b match
+        case _: TypeRef | _: AppliedType if b.typeSymbol.isClass =>
+          a match
+            case a: TermRef if a.symbol.isOneOf(Module | Enum) =>
+                  (a frozen_<:< b) // fast track
+              || (a frozen_<:< approx(b))
+            case _: TypeRef | _: AppliedType if a.typeSymbol.isClass =>
+              if a.isNullType then !b.isNotNull
+              else canOverlap(a.typeSymbol.asClass, b.typeSymbol.asClass)
+            case a: TypeProxy =>
+              canEqual(a.superType, b)
+            case a: AndOrType =>
+              canEqual(a.tp1, b) || canEqual(a.tp2, b)
+        case b: TypeProxy =>
+          canEqual(a, b.superType)
+        case b: AndOrType =>
+          // we lose precision with and/or types, but it's hard to do better and
+          // still compute `canEqual(A & B, B & A) = true`.
+          canEqual(a, b.tp1) || canEqual(a, b.tp2)
+
+    if ctx.settings.WimplausiblePatterns.value && !canEqual(pat.tpe, selType) then
+      report.warning(ImplausiblePatternWarning(pat, selType), pat.srcPos)
+  end warnOnImplausiblePattern
+
+end Linter

compiler/src/dotty/tools/dotc/typer/Synthesizer.scala

@@ -166,7 +166,7 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context):
 
       def cmpWithBoxed(cls1: ClassSymbol, cls2: ClassSymbol) =
         cls2 == defn.NothingClass
-        || cls2 == defn.boxedType(cls1.typeRef).symbol
+        || cls2 == defn.boxedClass(cls1)
         || cls1.isNumericValueClass && cls2.derivesFrom(defn.BoxedNumberClass)
 
       if cls1.isPrimitiveValueClass then

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -3303,7 +3303,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         traverse(xtree :: rest)
       case stat :: rest =>
         val stat1 = typed(stat)(using ctx.exprContext(stat, exprOwner))
-        if !checkInterestingResultInStatement(stat1) then checkStatementPurity(stat1)(stat, exprOwner)
+        if !Linter.warnOnInterestingResultInStatement(stat1) then checkStatementPurity(stat1)(stat, exprOwner)
         buf += stat1
         traverse(rest)(using stat1.nullableContext)
       case nil =>
@@ -4361,109 +4361,18 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
     tree match
       case _: RefTree | _: Literal
       if !isVarPattern(tree) && !(pt <:< tree.tpe) =>
-        withMode(Mode.GadtConstraintInference) {
+        withMode(Mode.GadtConstraintInference):
           TypeComparer.constrainPatternType(tree.tpe, pt)
-        }
-
-        // approximate type params with bounds
-        def approx = new ApproximatingTypeMap {
-          var alreadyExpanding: List[TypeRef] = Nil
-          def apply(tp: Type) = tp.dealias match
-            case tp: TypeRef if !tp.symbol.isClass =>
-              if alreadyExpanding contains tp then tp else
-                val saved = alreadyExpanding
-                alreadyExpanding ::= tp
-                val res = expandBounds(tp.info.bounds)
-                alreadyExpanding = saved
-                res
-            case _ =>
-              mapOver(tp)
-        }
 
-        // Is it certain that a value of `tree.tpe` is never a subtype of `pt`?
-        // It is true if either
-        // - the class of `tree.tpe` and class of `pt` cannot have common subclass, or
-        // - `tree` is an object or enum value, which cannot possibly be a subtype of `pt`
-        val isDefiniteNotSubtype = {
-          val clsA = tree.tpe.widenDealias.classSymbol
-          val clsB = pt.dealias.classSymbol
-          clsA.exists && clsB.exists
-            && clsA != defn.NullClass
-            && (!clsA.isNumericValueClass && !clsB.isNumericValueClass) // approximation for numeric conversion and boxing
-            && !clsA.asClass.mayHaveCommonChild(clsB.asClass)
-          || tree.symbol.isOneOf(Module | Enum)
-             && !(tree.tpe frozen_<:< pt) // fast track
-             && !(tree.tpe frozen_<:< approx(pt))
-        }
+        Linter.warnOnImplausiblePattern(tree, pt)
 
-        if isDefiniteNotSubtype then
-          // We could check whether `equals` is overridden.
-          // Reasons for not doing so:
-          // - it complicates the protocol
-          // - such code patterns usually implies hidden errors in the code
-          // - it's safe/sound to reject the code
-          report.error(TypeMismatch(tree.tpe, pt, Some(tree), "\npattern type is incompatible with expected type"), tree.srcPos)
-        else
-          val cmp =
-            untpd.Apply(
-              untpd.Select(untpd.TypedSplice(tree), nme.EQ),
-              untpd.TypedSplice(dummyTreeOfType(pt)))
-          typedExpr(cmp, defn.BooleanType)
+        val cmp =
+          untpd.Apply(
+            untpd.Select(untpd.TypedSplice(tree), nme.EQ),
+            untpd.TypedSplice(dummyTreeOfType(pt)))
+        typedExpr(cmp, defn.BooleanType)
       case _ =>
 
-  private def checkInterestingResultInStatement(t: Tree)(using Context): Boolean = {
-    def isUninterestingSymbol(sym: Symbol): Boolean =
-      sym == NoSymbol ||
-      sym.isConstructor ||
-      sym.is(Package) ||
-      sym.isPackageObject ||
-      sym == defn.BoxedUnitClass ||
-      sym == defn.AnyClass ||
-      sym == defn.AnyRefAlias ||
-      sym == defn.AnyValClass
-    def isUninterestingType(tpe: Type): Boolean =
-      tpe == NoType ||
-      tpe.typeSymbol == defn.UnitClass ||
-      defn.isBottomClass(tpe.typeSymbol) ||
-      tpe =:= defn.UnitType ||
-      tpe.typeSymbol == defn.BoxedUnitClass ||
-      tpe =:= defn.AnyValType ||
-      tpe =:= defn.AnyType ||
-      tpe =:= defn.AnyRefType
-    def isJavaApplication(t: Tree): Boolean = t match {
-      case Apply(f, _) => f.symbol.is(JavaDefined) && !defn.ObjectClass.isSubClass(f.symbol.owner)
-      case _ => false
-    }
-    def checkInterestingShapes(t: Tree): Boolean = t match {
-      case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) || checkInterestingShapes(elsepart)
-      case Block(_, res) => checkInterestingShapes(res)
-      case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body))
-      case _ => checksForInterestingResult(t)
-    }
-    def checksForInterestingResult(t: Tree): Boolean = (
-         !t.isDef                               // ignore defs
-      && !isUninterestingSymbol(t.symbol)       // ctors, package, Unit, Any
-      && !isUninterestingType(t.tpe)            // bottom types, Unit, Any
-      && !isThisTypeResult(t)                   // buf += x
-      && !isSuperConstrCall(t)                  // just a thing
-      && !isJavaApplication(t)                  // Java methods are inherently side-effecting
-      // && !treeInfo.hasExplicitUnit(t)           // suppressed by explicit expr: Unit // TODO Should explicit `: Unit` be added as warning suppression?
-    )
-    if ctx.settings.WNonUnitStatement.value && !ctx.isAfterTyper && checkInterestingShapes(t) then
-      val where = t match {
-        case Block(_, res) => res
-        case If(_, thenpart, Literal(Constant(()))) =>
-          thenpart match {
-            case Block(_, res) => res
-            case _ => thenpart
-          }
-        case _ => t
-      }
-      report.warning(UnusedNonUnitValue(where.tpe), t.srcPos)
-      true
-    else false
-  }
-
   private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol)(using Context): Unit =
     if !tree.tpe.isErroneous
       && !ctx.isAfterTyper

tests/neg/gadt-contradictory-pattern.scala → tests/neg-custom-args/fatal-warnings/gadt-contradictory-pattern.scala

@@ -1,3 +1,4 @@
+// scalac: -Wimplausible-patterns
 object Test {
   sealed abstract class Foo[T]
   case object Bar1 extends Foo[Int]

tests/neg/i5077.scala → tests/neg-custom-args/fatal-warnings/i5077.scala

@@ -1,3 +1,4 @@
+// scalac: -Wimplausible-patterns
 trait Is[A]
 case object IsInt extends Is[Int]
 case object IsString extends Is[String]

tests/neg/i9166.scala → tests/neg-custom-args/fatal-warnings/i9166.scala

@@ -1,3 +1,4 @@
+// scalac: -Wimplausible-patterns
 object UnitTest extends App {
   def foo(m: Unit) = m match {
     case runtime.BoxedUnit.UNIT => println("ok") // error

tests/neg-custom-args/fatal-warnings/i9740.check

@@ -0,0 +1,12 @@
+-- [E186] Type Error: tests/neg-custom-args/fatal-warnings/i9740.scala:10:9 --------------------------------------------
+10 |    case RecoveryCompleted => println("Recovery completed")            // error
+   |         ^^^^^^^^^^^^^^^^^
+   |         Implausible pattern:
+   |         RecoveryCompleted  could match selector of type  object TypedRecoveryCompleted
+   |         only if there is an `equals` method identifying elements of the two types.
+-- [E186] Type Error: tests/neg-custom-args/fatal-warnings/i9740.scala:15:9 --------------------------------------------
+15 |    case RecoveryCompleted =>             // error
+   |         ^^^^^^^^^^^^^^^^^
+   |         Implausible pattern:
+   |         RecoveryCompleted  could match selector of type  TypedRecoveryCompleted
+   |         only if there is an `equals` method identifying elements of the two types.

tests/neg/i9740.scala → tests/neg-custom-args/fatal-warnings/i9740.scala

@@ -1,3 +1,4 @@
+// scalac: -Wimplausible-patterns
 abstract class RecoveryCompleted
 object RecoveryCompleted extends RecoveryCompleted
 

tests/neg/i9740b.scala → tests/neg-custom-args/fatal-warnings/i9740b.scala

@@ -1,3 +1,4 @@
+// scalac: -Wimplausible-patterns
 enum Recovery:
   case RecoveryCompleted
 

tests/neg/i9740c.scala → tests/neg-custom-args/fatal-warnings/i9740c.scala

@@ -1,3 +1,4 @@
+// scalac: -Wimplausible-patterns
 sealed trait Exp[T]
 case class IntExp(x: Int) extends Exp[Int]
 case class StrExp(x: String) extends Exp[String]

tests/neg/i9740d.scala → tests/neg-custom-args/fatal-warnings/i9740d.scala

@@ -1,3 +1,5 @@
+// scalac: -Wimplausible-patterns
+
 sealed trait Exp[T]
 case class IntExp(x: Int) extends Exp[Int]
 case class StrExp(x: String) extends Exp[String]

tests/neg/i5004.scala

@@ -1,6 +1,6 @@
 object i0 {
 1 match {
 def this(): Int  // error
-  def this()  // error
+  def this() // error
 }
 }