From a52d12cefdbe50e7d177b6fd4cdbaabb728b05ce Mon Sep 17 00:00:00 2001
From: Som Snytt <som.snytt@gmail.com>
Date: Fri, 26 Jan 2024 22:12:56 -0800
Subject: [PATCH] Lint cloneable modules

---
 .../scala/tools/nsc/settings/Warnings.scala   |  2 ++
 .../tools/nsc/typechecker/RefChecks.scala     | 28 +++++++++----------
 test/files/neg/cloneable.check                |  6 ++++
 test/files/neg/cloneable.scala                |  8 ++++++
 4 files changed, 29 insertions(+), 15 deletions(-)
 create mode 100644 test/files/neg/cloneable.check
 create mode 100644 test/files/neg/cloneable.scala

diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala
index f05532ce3865..f54c139abe27 100644
--- a/src/compiler/scala/tools/nsc/settings/Warnings.scala
+++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala
@@ -218,6 +218,7 @@ trait Warnings {
     val IntDivToFloat          = LintWarning("int-div-to-float",          "Warn when an integer division is converted (widened) to floating point: `(someInt / 2): Double`.")
     val NamedBooleans          = LintWarning("named-booleans",            "Boolean literals should be named args unless param is @deprecatedName.")
     val PatternShadow          = LintWarning("pattern-shadow",            "Pattern variable id is also a term in scope.")
+    val CloneableObject        = LintWarning("cloneable",                 "Modules (objects) should not be Cloneable.")
 
     def allLintWarnings = values.toSeq.asInstanceOf[Seq[LintWarning]]
   }
@@ -256,6 +257,7 @@ trait Warnings {
   def lintIntDivToFloat          = lint.contains(IntDivToFloat)
   def lintNamedBooleans          = lint.contains(NamedBooleans)
   def warnPatternShadow          = lint.contains(PatternShadow)
+  def warnCloneableObject        = lint.contains(CloneableObject)
 
   // The Xlint warning group.
   val lint = MultiChoiceSetting(
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index a003bd3846b0..c2356f690203 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -886,28 +886,26 @@ abstract class RefChecks extends Transform {
      */
     private def validateBaseTypes(clazz: Symbol): Unit = {
       val seenParents = mutable.HashSet[Type]()
-      val seenTypes = new Array[List[Type]](clazz.info.baseTypeSeq.length)
-      for (i <- 0 until seenTypes.length)
-        seenTypes(i) = Nil
+      val seenTypes = Array.fill[List[Type]](clazz.info.baseTypeSeq.length)(Nil)
+      val warnCloneable = settings.warnCloneableObject && clazz.isModuleClass
 
       /* validate all base types of a class in reverse linear order. */
       def register(tp: Type): Unit = {
-//        if (clazz.fullName.endsWith("Collection.Projection"))
-//            println("validate base type "+tp)
         val baseClass = tp.typeSymbol
         if (baseClass.isClass) {
           if (!baseClass.isTrait && !baseClass.isJavaDefined && !currentRun.compiles(baseClass) && !separatelyCompiledScalaSuperclass.contains(baseClass))
             separatelyCompiledScalaSuperclass.update(baseClass, ())
           val index = clazz.info.baseTypeIndex(baseClass)
           if (index >= 0) {
-            if (seenTypes(index) forall (tp1 => !(tp1 <:< tp)))
-              seenTypes(index) =
-                tp :: (seenTypes(index) filter (tp1 => !(tp <:< tp1)))
+            if (!seenTypes(index).exists(_ <:< tp))
+              seenTypes(index) = tp :: seenTypes(index).filterNot(tp <:< _)
           }
         }
-        val remaining = tp.parents filterNot seenParents
+        if (warnCloneable && baseClass.eq(JavaCloneableClass))
+          reporter.warning(clazz.pos, s"$clazz should not extend Cloneable.")
+        val remaining = tp.parents.filterNot(seenParents)
         seenParents ++= remaining
-        remaining foreach register
+        remaining.foreach(register)
       }
       register(clazz.tpe)
       for (i <- 0 until seenTypes.length) {
@@ -915,12 +913,12 @@ abstract class RefChecks extends Transform {
         seenTypes(i) match {
           case Nil =>
             devWarning(s"base $baseClass not found in basetypes of $clazz. This might indicate incorrect caching of TypeRef#parents.")
-          case _ :: Nil =>
-            ;// OK
+          case _ :: Nil => // OK
           case tp1 :: tp2 :: _ =>
-            reporter.error(clazz.pos, "illegal inheritance;\n " + clazz +
-                       " inherits different type instances of " + baseClass +
-                       ":\n" + tp1 + " and " + tp2)
+            reporter.error(clazz.pos,
+              sm"""|illegal inheritance;
+                   | $clazz inherits different type instances of $baseClass:
+                   |$tp1 and $tp2""")
             explainTypes(tp1, tp2)
             explainTypes(tp2, tp1)
         }
diff --git a/test/files/neg/cloneable.check b/test/files/neg/cloneable.check
new file mode 100644
index 000000000000..751a6b945622
--- /dev/null
+++ b/test/files/neg/cloneable.check
@@ -0,0 +1,6 @@
+cloneable.scala:6: warning: object X should not extend Cloneable.
+object X extends Base
+       ^
+error: No warnings can be incurred under -Werror.
+1 warning
+1 error
diff --git a/test/files/neg/cloneable.scala b/test/files/neg/cloneable.scala
new file mode 100644
index 000000000000..72f06d5558c8
--- /dev/null
+++ b/test/files/neg/cloneable.scala
@@ -0,0 +1,8 @@
+
+//> using options -Werror -Xlint:cloneable
+
+class Base extends Cloneable
+
+object X extends Base
+
+class Y extends Base