warn about unnecessary uses of .nn (#23327)

olhotak · web-flow · commit b04a412688ce · 2025-06-10T09:35:21.000-04:00
Issue warnings when `.nn` is called on a receiver that is already not
null or when the expected type of the result of `.nn` admits null.
diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
@@ -73,7 +73,7 @@ object OrderingConstraint {
           }
           else {
             val prev_es = entries(prev, poly)
-            if (prev_es == null || (es.nn ne prev_es.nn))
+            if (prev_es == null || (es.nn ne prev_es))
               current // can re-use existing entries array.
             else {
               es = es.nn.clone
diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala
@@ -563,6 +563,7 @@ object StdNames {
     val next: N                 = "next"
     val nmeNewTermName: N       = "newTermName"
     val nmeNewTypeName: N       = "newTypeName"
+    val nn: N                   = "nn"
     val noAutoTupling: N        = "noAutoTupling"
     val normalize: N            = "normalize"
     val notifyAll_ : N          = "notifyAll"
diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -2586,7 +2586,7 @@ object SymDenotations {
               try f.container == chosen.container catch case NonFatal(ex) => true
             if !ambiguityWarningIssued then
               for conflicting <- assocFiles.find(!sameContainer(_)) do
-                report.warning(em"""${ambiguousFilesMsg(conflicting.nn)}
+                report.warning(em"""${ambiguousFilesMsg(conflicting)}
                                    |Keeping only the definition in $chosen""")
                 ambiguityWarningIssued = true
             multi.filterWithPredicate(_.symbol.associatedFile == chosen)
diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala
@@ -373,15 +373,26 @@ object Types extends TypeUtils {
     final def isNotNull(using Context): Boolean = this match {
       case tp: ConstantType => tp.value.value != null
       case tp: FlexibleType => false
-      case tp: ClassInfo => !tp.cls.isNullableClass && tp.cls != defn.NothingClass
+      case tp: ClassInfo => !tp.cls.isNullableClass && !tp.isNothingType
       case tp: AppliedType => tp.superType.isNotNull
-      case tp: TypeBounds => tp.lo.isNotNull
+      case tp: TypeBounds => tp.hi.isNotNull
       case tp: TypeProxy => tp.underlying.isNotNull
       case AndType(tp1, tp2) => tp1.isNotNull || tp2.isNotNull
       case OrType(tp1, tp2) => tp1.isNotNull && tp2.isNotNull
       case _ => false
     }
 
+    /** Is `null` a value of this type? */
+    def admitsNull(using Context): Boolean =
+      isNullType || isAny || (this match
+        case OrType(l, r) => r.admitsNull || l.admitsNull
+        case AndType(l, r) => r.admitsNull && l.admitsNull
+        case TypeBounds(lo, hi) => lo.admitsNull
+        case FlexibleType(lo, hi) => true
+        case tp: TypeProxy => tp.underlying.admitsNull
+        case _ => false
+      )
+
     /** Is this type produced as a repair for an error? */
     final def isError(using Context): Boolean = stripTypeVar.isInstanceOf[ErrorType]
 
diff --git a/compiler/src/dotty/tools/dotc/printing/Formatting.scala b/compiler/src/dotty/tools/dotc/printing/Formatting.scala
@@ -63,7 +63,7 @@ object Formatting {
 
     class ShowImplicits4:
       given [X: Show]: Show[X | Null] with
-        def show(x: X | Null) = if x == null then "null" else CtxShow(toStr(x.nn))
+        def show(x: X | Null) = if x == null then "null" else CtxShow(toStr(x))
 
     class ShowImplicits3 extends ShowImplicits4:
       given Show[Product] = ShowAny
diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala
@@ -695,7 +695,7 @@ object Erasure {
           val owner = sym.maybeOwner
           if defn.specialErasure.contains(owner) then
             assert(sym.isConstructor, s"${sym.showLocated}")
-            defn.specialErasure(owner).nn
+            defn.specialErasure(owner)
           else if defn.isSyntheticFunctionClass(owner) then
             defn.functionTypeErasure(owner).typeSymbol
           else
diff --git a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala
@@ -262,7 +262,7 @@ object GenericSignatures {
             typeParamSig(sym.name.lastPart)
           }
           else if (defn.specialErasure.contains(sym))
-            jsig(defn.specialErasure(sym).nn.typeRef)
+            jsig(defn.specialErasure(sym).typeRef)
           else if (sym == defn.UnitClass || sym == defn.BoxedUnitModule)
             jsig(defn.BoxedUnitClass.typeRef)
           else if (sym == defn.NothingClass)
diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala
@@ -477,12 +477,12 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
         newSymbol(claz, offsetName(info.defs.size), Synthetic, defn.LongType).enteredAfter(this)
       case None =>
         newSymbol(claz, offsetName(0), Synthetic, defn.LongType).enteredAfter(this)
-    offsetSymbol.nn.addAnnotation(Annotation(defn.ScalaStaticAnnot, offsetSymbol.nn.span))
+    offsetSymbol.addAnnotation(Annotation(defn.ScalaStaticAnnot, offsetSymbol.span))
     val fieldTree = thizClass.select(lazyNme.RLazyVals.getDeclaredField).appliedTo(Literal(Constant(containerName.mangledString)))
-    val offsetTree = ValDef(offsetSymbol.nn, getOffset.appliedTo(fieldTree))
+    val offsetTree = ValDef(offsetSymbol, getOffset.appliedTo(fieldTree))
     val offsetInfo = appendOffsetDefs.getOrElseUpdate(claz, new OffsetInfo(Nil))
     offsetInfo.defs = offsetTree :: offsetInfo.defs
-    val offset = ref(offsetSymbol.nn)
+    val offset = ref(offsetSymbol)
 
     val swapOver =
         This(claz)
@@ -617,31 +617,31 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
              .symbol.asTerm
         else { // need to create a new flag
           offsetSymbol = newSymbol(claz, offsetById, Synthetic, defn.LongType).enteredAfter(this)
-          offsetSymbol.nn.addAnnotation(Annotation(defn.ScalaStaticAnnot, offsetSymbol.nn.span))
+          offsetSymbol.addAnnotation(Annotation(defn.ScalaStaticAnnot, offsetSymbol.span))
           val flagName = LazyBitMapName.fresh(id.toString.toTermName)
           val flagSymbol = newSymbol(claz, flagName, containerFlags, defn.LongType).enteredAfter(this)
           flag = ValDef(flagSymbol, Literal(Constant(0L)))
           val fieldTree = thizClass.select(lazyNme.RLazyVals.getDeclaredField).appliedTo(Literal(Constant(flagName.toString)))
-          val offsetTree = ValDef(offsetSymbol.nn, getOffsetStatic.appliedTo(fieldTree))
+          val offsetTree = ValDef(offsetSymbol, getOffsetStatic.appliedTo(fieldTree))
           info.defs = offsetTree :: info.defs
         }
 
       case None =>
         offsetSymbol = newSymbol(claz, offsetName(0), Synthetic, defn.LongType).enteredAfter(this)
-        offsetSymbol.nn.addAnnotation(Annotation(defn.ScalaStaticAnnot, offsetSymbol.nn.span))
+        offsetSymbol.addAnnotation(Annotation(defn.ScalaStaticAnnot, offsetSymbol.span))
         val flagName = LazyBitMapName.fresh("0".toTermName)
         val flagSymbol = newSymbol(claz, flagName, containerFlags, defn.LongType).enteredAfter(this)
         flag = ValDef(flagSymbol, Literal(Constant(0L)))
         val fieldTree = thizClass.select(lazyNme.RLazyVals.getDeclaredField).appliedTo(Literal(Constant(flagName.toString)))
-        val offsetTree = ValDef(offsetSymbol.nn, getOffsetStatic.appliedTo(fieldTree))
+        val offsetTree = ValDef(offsetSymbol, getOffsetStatic.appliedTo(fieldTree))
         appendOffsetDefs += (claz -> new OffsetInfo(List(offsetTree), ord))
     }
 
     val containerName = LazyLocalName.fresh(x.name.asTermName)
     val containerSymbol = newSymbol(claz, containerName, x.symbol.flags &~ containerFlagsMask | containerFlags, tpe, coord = x.symbol.coord).enteredAfter(this)
     val containerTree = ValDef(containerSymbol, defaultValue(tpe))
 
-    val offset =  ref(offsetSymbol.nn)
+    val offset =  ref(offsetSymbol)
     val getFlag = Select(ref(defn.LazyValsModule), lazyNme.RLazyVals.get)
     val setFlag = Select(ref(defn.LazyValsModule), lazyNme.RLazyVals.setFlag)
     val wait =    Select(ref(defn.LazyValsModule), lazyNme.RLazyVals.wait4Notification)
diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
@@ -466,13 +466,13 @@ object ProtoTypes {
             targ = typerFn(arg)
             // TODO: investigate why flow typing is not working on `targ`
             if ctx.reporter.hasUnreportedErrors then
-              if hasInnerErrors(targ.nn, argType) then
+              if hasInnerErrors(targ, argType) then
                 state.errorArgs += arg
             else
-              state.typedArg = state.typedArg.updated(arg, targ.nn)
+              state.typedArg = state.typedArg.updated(arg, targ)
               state.errorArgs -= arg
         }
-      targ.nn
+      targ
     }
 
     /** The typed arguments. This takes any arguments already typed using
diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -1079,7 +1079,19 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         errorTree(tree, em"cannot convert to type selection") // will never be printed due to fallback
     }
 
-    if (tree.qualifier.isType) {
+    def warnUnnecessaryNN(tree: Tree): Unit = {
+      if ctx.explicitNulls then {
+        val symbol = tree.symbol
+        if symbol.exists && symbol.owner == defn.ScalaPredefModuleClass && symbol.name == nme.nn then
+          tree match
+          case Apply(_, args) =>
+            if(args.head.tpe.isNotNull) then report.warning("Unnecessary .nn: qualifier is already not null", tree)
+            if pt.admitsNull then report.warning("Unnecessary .nn: expected type admits null", tree)
+          case _ =>
+      }
+    }
+
+    val tree1 = if (tree.qualifier.isType) {
       val qual1 = typedType(tree.qualifier, shallowSelectionProto(tree.name, pt, this, tree.nameSpan))
       assignType(cpy.Select(tree)(qual1, tree.name), qual1)
     }
@@ -1089,6 +1101,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
       tryAlternatively(typeSelectOnTerm)(tryJavaSelectOnType)
     else
       typeSelectOnTerm
+
+    warnUnnecessaryNN(tree1)
+    tree1
   }
 
   def typedThis(tree: untpd.This)(using Context): Tree = {
diff --git a/presentation-compiler/src/main/dotty/tools/pc/CompilerSearchVisitor.scala b/presentation-compiler/src/main/dotty/tools/pc/CompilerSearchVisitor.scala
@@ -19,7 +19,7 @@ class CompilerSearchVisitor(
 )(using ctx: Context, reports: ReportContext)
     extends SymbolSearchVisitor:
 
-  val logger: Logger = Logger.getLogger(classOf[CompilerSearchVisitor].getName().nn).nn
+  val logger: Logger = Logger.getLogger(classOf[CompilerSearchVisitor].getName()).nn
 
   private def isAccessibleImplicitClass(sym: Symbol) =
     val owner = sym.maybeOwner
diff --git a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala
@@ -84,7 +84,7 @@ object SignatureHelpProvider:
             case Some(paramDoc) =>
               val newName =
                 if isJavaSymbol && head.name.startsWith("x$") then
-                  paramDoc.nn.displayName()
+                  paramDoc.displayName()
                 else head.name
               head.copy(name = newName.nn, doc = Some(paramDoc.docstring.nn)) :: rest
             case _ => head :: rest
diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionPos.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionPos.scala
@@ -62,7 +62,7 @@ object CompletionPos:
     CompletionPos(
       start,
       identEnd,
-      query.nn,
+      query,
       sourcePos,
       offsetParams.uri.nn,
       wasCursorApplied,
diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala
@@ -174,7 +174,7 @@ class CompletionProvider(
    */
   private def applyCompletionCursor(params: OffsetParams): (Boolean, String) =
     val text = params.text().nn
-    val offset = params.offset().nn
+    val offset = params.offset()
     val query = Completion.naiveCompletionPrefix(text, offset)
 
     if offset > 0 && text.charAt(offset - 1).isUnicodeIdentifierPart
diff --git a/presentation-compiler/test/dotty/tools/pc/utils/TestInlayHints.scala b/presentation-compiler/test/dotty/tools/pc/utils/TestInlayHints.scala
@@ -36,7 +36,7 @@ object TestInlayHints {
       InlayHints.fromData(inlayHint.getData().asInstanceOf[JsonElement])._2
     buffer += "/*"
     labels.zip(data).foreach { case (label, data) =>
-      buffer += label.nn
+      buffer += label
       buffer ++= readData(data)
     }
     buffer += "*/"
diff --git a/tests/explicit-nulls/warn/flow-match.check b/tests/explicit-nulls/warn/flow-match.check
@@ -11,6 +11,6 @@
    |         ^^^^
    |         Unreachable case
 -- [E030] Match case Unreachable Warning: tests/explicit-nulls/warn/flow-match.scala:14:9 ------------------------------
-14 |    case s4 => s4.nn // warn
+14 |    case s4 => s4 // warn
    |         ^^
    |         Unreachable case
diff --git a/tests/explicit-nulls/warn/flow-match.scala b/tests/explicit-nulls/warn/flow-match.scala
@@ -2,7 +2,7 @@
 
 object MatchTest2 {
   def f6(s: String | Null): String = s match {
-    case s2 => s2.nn
+    case s2 => "string"
     case null => "other" // warn
     case s3 => s3 // warn
   }
@@ -11,6 +11,6 @@ object MatchTest2 {
     case null => "other"
     case null => "other" // warn
     case s3: String => s3
-    case s4 => s4.nn // warn
+    case s4 => s4 // warn
   }
 }
diff --git a/tests/explicit-nulls/warn/unnecessary-nn.scala b/tests/explicit-nulls/warn/unnecessary-nn.scala
@@ -0,0 +1,21 @@
+def f1(s: String): String = s.nn // warn
+def f2(s: String|Null): String|Null = s.nn // warn
+def f3(s: String|Null): Any = s.nn // warn
+def f4(s: String|Null): String = s.nn
+
+def f5[T >: String](s: String|Null): T = s.nn
+def f6[T >: String|Null](s: String|Null): T = s.nn // warn
+
+def f5a[T <: String](s: T): String = s.nn // warn
+
+// flexible types
+def f7(s: String|Null) = "".concat(s.nn) // warn
+def f8(s: String): String = s.trim().nn // OK because the .nn could be useful as a dynamic null check
+
+
+def f9(s: String|Null): String =
+  if(s == null) "default"
+  else s.nn // warn
+
+def f10(x: String|Null): ((String|Null) @deprecated) = x.nn // warn
+def f10b(x: String|Null): (String|Null) = x.nn // warn

Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ object OrderingConstraint {`
`73`	`73`	`}`
`74`	`74`	`else {`
`75`	`75`	`val prev_es = entries(prev, poly)`
`76`		`- if (prev_es == null \|\| (es.nn ne prev_es.nn))`
	`76`	`+ if (prev_es == null \|\| (es.nn ne prev_es))`
`77`	`77`	`current // can re-use existing entries array.`
`78`	`78`	`else {`
`79`	`79`	`es = es.nn.clone`
Original file line number	Diff line number	Diff line change
`@@ -262,7 +262,7 @@ object GenericSignatures {`
`262`	`262`	`typeParamSig(sym.name.lastPart)`
`263`	`263`	`}`
`264`	`264`	`else if (defn.specialErasure.contains(sym))`
`265`		`- jsig(defn.specialErasure(sym).nn.typeRef)`
	`265`	`+ jsig(defn.specialErasure(sym).typeRef)`
`266`	`266`	`else if (sym == defn.UnitClass \|\| sym == defn.BoxedUnitModule)`
`267`	`267`	`jsig(defn.BoxedUnitClass.typeRef)`
`268`	`268`	`else if (sym == defn.NothingClass)`