From 9c981ba11806c2e782bbb9662225ba739e664af9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Marks?= <pmarks@virtuslab.com>
Date: Tue, 22 Jun 2021 20:04:36 +0200
Subject: [PATCH] Restricts `isInstanceOf[Null]` checks

Fixes #4004
`isInstanceOf[Nothing]` checks are always prohibited
`isInstanceOf[Null]` checks are probihibited unless they can be proven at compiletime, then thay are simplified to `true`
---
 .../src/dotty/tools/dotc/core/Definitions.scala     |  2 ++
 .../dotty/tools/dotc/transform/TypeTestsCasts.scala |  6 ++++--
 tests/neg/i4004.scala                               | 13 +++++++++++++
 tests/run/i4004.check                               |  2 ++
 tests/run/i4004.scala                               |  6 ++++++
 5 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 tests/neg/i4004.scala
 create mode 100644 tests/run/i4004.check
 create mode 100644 tests/run/i4004.scala

diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala
index 547ceb292055..8d71a1718c81 100644
--- a/compiler/src/dotty/tools/dotc/core/Definitions.scala
+++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala
@@ -1196,6 +1196,8 @@ class Definitions {
 
   @tu lazy val topClasses: Set[Symbol] = Set(AnyClass, MatchableClass, ObjectClass, AnyValClass)
 
+  @tu lazy val untestableClasses: Set[Symbol] = Set(NothingClass, NullClass)
+
   @tu lazy val AbstractFunctionType: Array[TypeRef] = mkArityArray("scala.runtime.AbstractFunction", MaxImplementedFunctionArity, 0)
   val AbstractFunctionClassPerRun: PerRun[Array[Symbol]] = new PerRun(AbstractFunctionType.map(_.symbol.asClass))
   def AbstractFunctionClass(n: Int)(using Context): Symbol = AbstractFunctionClassPerRun()(using ctx)(n)
diff --git a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
index 5d03d5381eed..452142c1eda9 100644
--- a/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
+++ b/compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala
@@ -236,14 +236,16 @@ object TypeTestsCasts {
 
             val foundEffectiveClass = effectiveClass(expr.tpe.widen)
 
-            if foundEffectiveClass.isPrimitiveValueClass && !testCls.isPrimitiveValueClass then
+            val isUntestable = defn.untestableClasses.contains(testCls)
+            val isIllegalPrimitiveTest = foundEffectiveClass.isPrimitiveValueClass && !testCls.isPrimitiveValueClass
+            if isIllegalPrimitiveTest || isUntestable then
               report.error(i"cannot test if value of $exprType is a reference of $testCls", tree.srcPos)
               false
             else foundClasses.exists(check)
           end checkSensical
 
           if (expr.tpe <:< testType)
-            if (expr.tpe.isNotNull) {
+            if (expr.tpe.isNotNull || expr.tpe.widen.isRef(defn.NullClass)) {
               if (!inMatch) report.warning(TypeTestAlwaysSucceeds(expr.tpe, testType), tree.srcPos)
               constant(expr, Literal(Constant(true)))
             }
diff --git a/tests/neg/i4004.scala b/tests/neg/i4004.scala
new file mode 100644
index 000000000000..9833038823be
--- /dev/null
+++ b/tests/neg/i4004.scala
@@ -0,0 +1,13 @@
+@main def Test =
+  "a".isInstanceOf[Null] // error
+  null.isInstanceOf[Null]
+  "a".isInstanceOf[Nothing] // error
+
+  "a" match
+  case _: Null => () // error
+  case _ => ()
+
+  null match
+  case _: Null => ()
+  case _ => ()
+
diff --git a/tests/run/i4004.check b/tests/run/i4004.check
new file mode 100644
index 000000000000..3324d4040ec0
--- /dev/null
+++ b/tests/run/i4004.check
@@ -0,0 +1,2 @@
+true
+null
\ No newline at end of file
diff --git a/tests/run/i4004.scala b/tests/run/i4004.scala
new file mode 100644
index 000000000000..77f38f7e1e33
--- /dev/null
+++ b/tests/run/i4004.scala
@@ -0,0 +1,6 @@
+@main def Test =
+  println(null.isInstanceOf[Null])
+
+  null match
+    case _: Null => println("null")
+    case _ => println("not null")
\ No newline at end of file