From 213491f834258df94d20958faa52072ea8114dbb Mon Sep 17 00:00:00 2001 From: Ondrej Lhotak Date: Fri, 6 Jun 2025 12:37:02 -0400 Subject: [PATCH 1/7] warn about unnecessary uses of .nn --- .../tools/dotc/core/NullOpsDecorator.scala | 12 ++++++++++++ .../src/dotty/tools/dotc/core/StdNames.scala | 1 + compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Typer.scala | 17 ++++++++++++++++- tests/explicit-nulls/warn/unnecessary-nn.scala | 13 +++++++++++++ 5 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 tests/explicit-nulls/warn/unnecessary-nn.scala diff --git a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala index 291498dbc558..a3ea59782cfb 100644 --- a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala +++ b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala @@ -50,6 +50,18 @@ object NullOpsDecorator: val stripped = self.stripNull() stripped ne self } + + def admitsNull(using Context): Boolean = { + val widened = self.widenDealias + widened.isNullType || widened.isAny || (widened 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 + ) + } end extension import ast.tpd.* diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index ac1f4f448722..252c25468b9a 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -564,6 +564,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/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index b06bd5c00a28..cfd1298635c3 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -373,7 +373,7 @@ 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: TypeProxy => tp.underlying.isNotNull diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 30d3add2529f..049c493b3316 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1088,7 +1088,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) } @@ -1098,6 +1110,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/tests/explicit-nulls/warn/unnecessary-nn.scala b/tests/explicit-nulls/warn/unnecessary-nn.scala new file mode 100644 index 000000000000..51578e70a75d --- /dev/null +++ b/tests/explicit-nulls/warn/unnecessary-nn.scala @@ -0,0 +1,13 @@ +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 \ No newline at end of file From 795163591c0ec740b0929c24c0b7167a5ead3b75 Mon Sep 17 00:00:00 2001 From: Ondrej Lhotak Date: Fri, 6 Jun 2025 12:48:59 -0400 Subject: [PATCH 2/7] remove some unnecessary uses of .nn --- .../tools/dotc/core/OrderingConstraint.scala | 2 +- .../dotty/tools/dotc/core/SymDenotations.scala | 2 +- .../dotty/tools/dotc/printing/Formatting.scala | 2 +- .../src/dotty/tools/dotc/transform/Erasure.scala | 2 +- .../tools/dotc/transform/GenericSignatures.scala | 2 +- .../dotty/tools/dotc/transform/LazyVals.scala | 16 ++++++++-------- .../src/dotty/tools/dotc/typer/ProtoTypes.scala | 6 +++--- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 150c39aa8e13..0154c70e7949 100644 --- 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/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 6f7f77de70a5..8c8579672dcc 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -2581,7 +2581,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/printing/Formatting.scala b/compiler/src/dotty/tools/dotc/printing/Formatting.scala index 70d305c2e372..daf4e70ad25a 100644 --- 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 index 25239aee59cf..3503c707aed9 100644 --- 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 index 26ede05ba607..b0c6672733e2 100644 --- 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 index 2fd777f715d9..76bc09e07540 100644 --- 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,23 +617,23 @@ 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)) } @@ -641,7 +641,7 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { 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 index 0c5382d8849d..d0baa3373c2a 100644 --- 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 From cf9883fcecfb60606a8729548ce7d11f8dec6556 Mon Sep 17 00:00:00 2001 From: Ondrej Lhotak Date: Fri, 6 Jun 2025 19:42:58 -0400 Subject: [PATCH 3/7] remove some more unnecessary uses of .nn --- .../src/main/dotty/tools/pc/CompilerSearchVisitor.scala | 2 +- .../src/main/dotty/tools/pc/SignatureHelpProvider.scala | 2 +- .../src/main/dotty/tools/pc/completions/CompletionPos.scala | 2 +- .../main/dotty/tools/pc/completions/CompletionProvider.scala | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/CompilerSearchVisitor.scala b/presentation-compiler/src/main/dotty/tools/pc/CompilerSearchVisitor.scala index f2e17415138a..aed8bf418490 100644 --- 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 index 423ca5d8db89..4fab29159bfb 100644 --- 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 index 40f1ccd2e797..ed181695922e 100644 --- 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 index f1645f76cf97..4c11eb93540a 100644 --- 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 From d705c1bde010fe6f4613d1dad4dd9d7e4ea511ac Mon Sep 17 00:00:00 2001 From: Ondrej Lhotak Date: Fri, 6 Jun 2025 20:45:04 -0400 Subject: [PATCH 4/7] fix TypeBounds case in isNotNull --- compiler/src/dotty/tools/dotc/core/Types.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index cfd1298635c3..246fbee91e63 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -375,7 +375,7 @@ object Types extends TypeUtils { case tp: FlexibleType => false 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 From 31afb35b28532c78895897c336cfebdac057acd9 Mon Sep 17 00:00:00 2001 From: Ondrej Lhotak Date: Fri, 6 Jun 2025 20:48:09 -0400 Subject: [PATCH 5/7] don't widen in admitsNull --- compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala index a3ea59782cfb..99aadcec11c3 100644 --- a/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala +++ b/compiler/src/dotty/tools/dotc/core/NullOpsDecorator.scala @@ -52,8 +52,7 @@ object NullOpsDecorator: } def admitsNull(using Context): Boolean = { - val widened = self.widenDealias - widened.isNullType || widened.isAny || (widened match + self.isNullType || self.isAny || (self match case OrType(l, r) => r.admitsNull || l.admitsNull case AndType(l, r) => r.admitsNull && l.admitsNull case TypeBounds(lo, hi) => lo.admitsNull From 56ac3a6b764fb0719edbc1bdefd6849963937dc0 Mon Sep 17 00:00:00 2001 From: Ondrej Lhotak Date: Sat, 7 Jun 2025 00:34:58 -0400 Subject: [PATCH 6/7] remove one more unnecessary .nn --- .../test/dotty/tools/pc/utils/TestInlayHints.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presentation-compiler/test/dotty/tools/pc/utils/TestInlayHints.scala b/presentation-compiler/test/dotty/tools/pc/utils/TestInlayHints.scala index 5fb38ad88e19..c0064b94355a 100644 --- 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 += "*/" From b96d7e3d36485b95f8262cfe9b833734a7021de8 Mon Sep 17 00:00:00 2001 From: Ondrej Lhotak Date: Sat, 7 Jun 2025 07:29:19 -0400 Subject: [PATCH 7/7] add test with flow-sensitive reasoning about nulls --- tests/explicit-nulls/warn/unnecessary-nn.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/explicit-nulls/warn/unnecessary-nn.scala b/tests/explicit-nulls/warn/unnecessary-nn.scala index 51578e70a75d..59dfa7ba6283 100644 --- a/tests/explicit-nulls/warn/unnecessary-nn.scala +++ b/tests/explicit-nulls/warn/unnecessary-nn.scala @@ -10,4 +10,9 @@ 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 \ No newline at end of file +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 \ No newline at end of file