From 30a88be8181038fa9b5b18fac795e2b965d61d73 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 28 Feb 2022 18:16:15 +0100 Subject: [PATCH] check if package names will be encoded in desugar produce a warning if they are - this is because encoding a package name is mostly undefined behaviour that the scala 3 compiler magically recovers from - but could be problematic. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 36 +++++++++++++++---- .../src/dotty/tools/dotc/util/Chars.scala | 3 ++ .../fatal-warnings/symbolic-packages.check | 16 +++++++++ .../fatal-warnings/symbolic-packages.scala | 11 ++++++ .../fatal-warnings/stats-in-empty-pkg.scala | 4 +++ 5 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/symbolic-packages.check create mode 100644 tests/neg-custom-args/fatal-warnings/symbolic-packages.scala create mode 100644 tests/pos-special/fatal-warnings/stats-in-empty-pkg.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 3b9e12e28ad1..55e9110afce0 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -8,7 +8,7 @@ import Symbols._, StdNames._, Trees._, ContextOps._ import Decorators._, transform.SymUtils._ import NameKinds.{UniqueName, EvidenceParamName, DefaultGetterName} import typer.{Namer, Checking} -import util.{Property, SourceFile, SourcePosition} +import util.{Property, SourceFile, SourcePosition, Chars} import config.Feature.{sourceVersion, migrateTo3, enabled} import config.SourceVersion._ import collection.mutable.ListBuffer @@ -521,9 +521,9 @@ object desugar { val enumCompanionRef = TermRefTree() val enumImport = Import(enumCompanionRef, enumCases.flatMap(caseIds).map( - enumCase => + enumCase => ImportSelector(enumCase.withSpan(enumCase.span.startPos)) - ) + ) ) (enumImport :: enumStats, enumCases, enumCompanionRef) } @@ -834,7 +834,8 @@ object desugar { val impl = mdef.impl val mods = mdef.mods val moduleName = normalizeName(mdef, impl).asTermName - if (mods.is(Package)) + if mods.is(Package) then + checkPackageName(mdef) PackageDef(Ident(moduleName), cpy.ModuleDef(mdef)(nme.PACKAGE, impl).withMods(mods &~ Package) :: Nil) else @@ -950,6 +951,26 @@ object desugar { else tree } + def checkPackageName(mdef: ModuleDef | PackageDef)(using Context): Unit = + + def check(name: Name, errSpan: Span): Unit = name match + case name: SimpleName if !errSpan.isSynthetic && name.exists(Chars.willBeEncoded) => + report.warning(em"The package name `$name` will be encoded on the classpath, and can lead to undefined behaviour.", mdef.source.atSpan(errSpan)) + case _ => + + def loop(part: RefTree): Unit = part match + case part @ Ident(name) => check(name, part.span) + case part @ Select(qual: RefTree, name) => + check(name, part.nameSpan) + loop(qual) + case _ => + + mdef match + case pdef: PackageDef => loop(pdef.pid) + case mdef: ModuleDef if mdef.mods.is(Package) => check(mdef.name, mdef.nameSpan) + case _ => + end checkPackageName + /** The normalized name of `mdef`. This means * 1. Check that the name does not redefine a Scala core class. * If it does redefine, issue an error and return a mangled name instead @@ -1134,7 +1155,7 @@ object desugar { val matchExpr = if (tupleOptimizable) rhs else - val caseDef = CaseDef(pat, EmptyTree, makeTuple(ids)) + val caseDef = CaseDef(pat, EmptyTree, makeTuple(ids)) Match(makeSelector(rhs, MatchCheck.IrrefutablePatDef), caseDef :: Nil) vars match { case Nil if !mods.is(Lazy) => @@ -1155,11 +1176,11 @@ object desugar { val restDefs = for (((named, tpt), n) <- vars.zipWithIndex if named.name != nme.WILDCARD) yield - if mods.is(Lazy) then + if mods.is(Lazy) then DefDef(named.name.asTermName, Nil, tpt, selector(n)) .withMods(mods &~ Lazy) .withSpan(named.span) - else + else valDef( ValDef(named.name.asTermName, tpt, selector(n)) .withMods(mods) @@ -1321,6 +1342,7 @@ object desugar { * (i.e. objects having the same name as a wrapped type) */ def packageDef(pdef: PackageDef)(using Context): PackageDef = { + checkPackageName(pdef) val wrappedTypeNames = pdef.stats.collect { case stat: TypeDef if isTopLevelDef(stat) => stat.name } diff --git a/compiler/src/dotty/tools/dotc/util/Chars.scala b/compiler/src/dotty/tools/dotc/util/Chars.scala index 4c54dc73459e..471b68d6247e 100644 --- a/compiler/src/dotty/tools/dotc/util/Chars.scala +++ b/compiler/src/dotty/tools/dotc/util/Chars.scala @@ -93,4 +93,7 @@ object Chars { '|' | '/' | '\\' => true case c => isSpecial(c) } + + /** Would the character be encoded by `NameTransformer.encode`? */ + def willBeEncoded(c : Char) : Boolean = !JCharacter.isJavaIdentifierPart(c) } diff --git a/tests/neg-custom-args/fatal-warnings/symbolic-packages.check b/tests/neg-custom-args/fatal-warnings/symbolic-packages.check new file mode 100644 index 000000000000..f5abe6ed0a36 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/symbolic-packages.check @@ -0,0 +1,16 @@ +-- Error: tests/neg-custom-args/fatal-warnings/symbolic-packages.scala:1:8 --------------------------------------------- +1 |package `with spaces` { // error + | ^^^^^^^^^^^^^ + | The package name `with spaces` will be encoded on the classpath, and can lead to undefined behaviour. +-- Error: tests/neg-custom-args/fatal-warnings/symbolic-packages.scala:5:10 -------------------------------------------- +5 |package +.* { // error // error + | ^ + | The package name `*` will be encoded on the classpath, and can lead to undefined behaviour. +-- Error: tests/neg-custom-args/fatal-warnings/symbolic-packages.scala:5:8 --------------------------------------------- +5 |package +.* { // error // error + | ^ + | The package name `+` will be encoded on the classpath, and can lead to undefined behaviour. +-- Error: tests/neg-custom-args/fatal-warnings/symbolic-packages.scala:9:16 -------------------------------------------- +9 |package object `mixed_*` { // error + | ^^^^^^^ + | The package name `mixed_*` will be encoded on the classpath, and can lead to undefined behaviour. diff --git a/tests/neg-custom-args/fatal-warnings/symbolic-packages.scala b/tests/neg-custom-args/fatal-warnings/symbolic-packages.scala new file mode 100644 index 000000000000..4e8ec2b15a0e --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/symbolic-packages.scala @@ -0,0 +1,11 @@ +package `with spaces` { // error + class Foo +} + +package +.* { // error // error + class Bar +} + +package object `mixed_*` { // error + class Baz +} diff --git a/tests/pos-special/fatal-warnings/stats-in-empty-pkg.scala b/tests/pos-special/fatal-warnings/stats-in-empty-pkg.scala new file mode 100644 index 000000000000..e5ea0b7566f8 --- /dev/null +++ b/tests/pos-special/fatal-warnings/stats-in-empty-pkg.scala @@ -0,0 +1,4 @@ +def foo = 23 +val bar = foo +var baz = bar +type Qux = Int