Issue Illegal Identifier for names with dollars

Author: som-snytt

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -1286,9 +1286,7 @@ object Parsers {
       if (isIdent) {
         val name = in.name
         if name == nme.CONSTRUCTOR || name == nme.STATIC_CONSTRUCTOR then
-          report.error(
-            em"""Illegal backquoted identifier: `<init>` and `<clinit>` are forbidden""",
-            in.sourcePos())
+          report.error(IllegalIdentifier(name), in.sourcePos())
         in.nextToken()
         name
       }
@@ -1297,6 +1295,19 @@ object Parsers {
         nme.ERROR
       }
 
+    /** An ident that is checked to be clean for a definition. */
+    def defName(): TermName =
+      val checkable = in.token != BACKQUOTED_IDENT
+      val start = in.sourcePos()
+      ident().tap: name =>
+        if checkable && name.toSimpleName.contains('$') then
+          report.error(IllegalIdentifier(name), start)
+
+    extension (nameTree: NameTree) def checkName: nameTree.type =
+      if !isBackquoted(nameTree) && nameTree.name.toSimpleName.contains('$') then
+        report.error(IllegalIdentifier(nameTree.name), nameTree.srcPos)
+      nameTree
+
     /** Accept identifier and return Ident with its name as a term name. */
     def termIdent(): Ident =
       makeIdent(in.token, in.offset, ident())
@@ -1309,7 +1320,7 @@ object Parsers {
       val tree = Ident(name)
       if (tok == BACKQUOTED_IDENT) tree.pushAttachment(Backquoted, ())
 
-      // Make sure that even trees with parsing errors have a offset that is within the offset
+      // Make sure that even trees with parsing errors have an offset that is within the offset
       val errorOffset = offset min (in.lastOffset - 1)
       if (tree.name == nme.ERROR && tree.span == NoSpan) tree.withSpan(Span(errorOffset, errorOffset))
       else atSpan(offset)(tree)
@@ -1379,7 +1390,7 @@ object Parsers {
 
     /** QualId ::= id {`.' id}
     */
-    def qualId(): Tree = dotSelectors(termIdent())
+    def qualId(): Tree = dotSelectors(termIdent().checkName)
 
     /** SimpleLiteral     ::=  [‘-’] integerLiteral
      *                      |  [‘-’] floatingPointLiteral
@@ -3773,7 +3784,7 @@ object Parsers {
           mods |= Param
         }
         atSpan(start, nameStart) {
-          val name = ident()
+          val name = defName()
           acceptColon()
           if (in.token == ARROW && paramOwner.isClass && !mods.is(Local))
             syntaxError(VarValParametersMayNotBeCallByName(name, mods.is(Mutable)))
@@ -4103,6 +4114,7 @@ object Parsers {
         case IdPattern(id, t) :: Nil if t.isEmpty =>
           val vdef = ValDef(id.name.asTermName, tpt, rhs)
           if (isBackquoted(id)) vdef.pushAttachment(Backquoted, ())
+          else checkName(id)
           finalizeDef(vdef, mods, start)
         case _ =>
           def isAllIds = lhs.forall {
@@ -4158,7 +4170,7 @@ object Parsers {
       }
       else {
         val mods1 = addFlag(mods, Method)
-        val ident = termIdent()
+        val ident = termIdent().checkName
         var name = ident.name.asTermName
         val paramss =
           if sourceVersion.enablesClauseInterleaving then
@@ -4229,7 +4241,7 @@ object Parsers {
 
       newLinesOpt()
       atSpan(start, nameStart) {
-        val nameIdent = typeIdent()
+        val nameIdent = typeIdent().checkName
         val isCapDef = gobbleHat()
         val tname = nameIdent.name.asTypeName
         val tparams = typeParamClauseOpt(ParamOwner.Hk)
@@ -4321,7 +4333,7 @@ object Parsers {
     /** ClassDef ::= id ClassConstr TemplateOpt
      */
     def classDef(start: Offset, mods: Modifiers): TypeDef = atSpan(start, nameStart) {
-      classDefRest(start, mods, ident().toTypeName)
+      classDefRest(start, mods, defName().toTypeName)
     }
 
     def classDefRest(start: Offset, mods: Modifiers, name: TypeName): TypeDef =
@@ -4346,7 +4358,7 @@ object Parsers {
     /** ObjectDef       ::= id TemplateOpt
      */
     def objectDef(start: Offset, mods: Modifiers): ModuleDef = atSpan(start, nameStart) {
-      val name = ident()
+      val name = defName()
       val templ = templateOpt(emptyConstructor)
       finalizeDef(ModuleDef(name, templ), mods, start)
     }
@@ -4366,7 +4378,7 @@ object Parsers {
      */
     def enumDef(start: Offset, mods: Modifiers): TypeDef = atSpan(start, nameStart) {
       val mods1 = checkAccessOnly(mods, "")
-      val modulName = ident()
+      val modulName = defName()
       val clsName = modulName.toTypeName
       val constr = classConstr(ParamOwner.Class)
       val templ = template(constr, isEnum = true)
@@ -4380,10 +4392,10 @@ object Parsers {
       accept(CASE)
 
       atSpan(start, nameStart) {
-        val id = termIdent()
+        val id = termIdent().checkName
         if (in.token == COMMA) {
           in.nextToken()
-          val ids = commaSeparated(() => termIdent())
+          val ids = commaSeparated(() => termIdent().checkName)
           if ctx.settings.Whas.enumCommentDiscard then
             in.getDocComment(start).foreach: comm =>
               warning(

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -236,6 +236,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case DefaultShadowsGivenID // errorNumber: 220
   case RecurseWithDefaultID // errorNumber: 221
   case EncodedPackageNameID // errorNumber: 222
+  case IllegalIdentifierID // errorNumber: 223
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3749,3 +3749,21 @@ final class EncodedPackageName(name: Name)(using Context) extends SyntaxMsg(Enco
        |or `myfile-test.scala` can produce encoded names for the generated package objects.
        |
        |In this case, the name `$name` is encoded as `${name.encode}`."""
+
+final class IllegalIdentifier(name: Name)(using Context) extends SyntaxMsg(IllegalIdentifierID):
+  override protected def msg(using Context): String =
+    name match
+    case nme.CONSTRUCTOR | nme.STATIC_CONSTRUCTOR =>
+      "Illegal backquoted identifier: `<init>` and `<clinit>` are forbidden"
+    case _ =>
+      i"The identifier `$name` should not contain `$$`, which is reserved for internal compiler use."
+  override protected def explain(using Context): String =
+    name match
+    case nme.CONSTRUCTOR | nme.STATIC_CONSTRUCTOR =>
+      "Names can include unusual characters when enclosed in backquotes, but `<init>` and `<clinit>` are reserved."
+    case _ =>
+      i"""User identifiers may be encoded with embedded `$$`, and other compiler artifacts
+         |may rely on using `$$` with specific meanings.
+         |
+         |The prohibition against explicit `$$` may be ignored by enclosing the identifier in backquotes
+         |at the definition site."""

tests/neg/i18234.scala

@@ -0,0 +1,163 @@
+/* vals */
+val goodVal = 1
+val $startVal = 1 // error
+val mid$dleVal = 1 // error
+val endVal$ = 1 // error
+
+def testValUsage =
+  $startVal + endVal$ // ok, should only warn on declaration not on usage
+
+/* functions */
+def $funcStart() = 3 // error
+def func$Middle() = 3 // error
+def funcEnd$() = 3 // error
+
+def func1badArg(goodArg: Int, bad$Arg: Int) = 5 // error
+def func1badArgAndUsageDoesNotThrowWarning(goodArg: Int, bad$Arg: Int) = bad$Arg // error
+def func2badArgs(goodArg: Int, bad$Arg: Int, bad$arg2: String) = 5 // error // error
+def multilineFunc(
+  goodArg: Int,
+  badAr$g: Int // error
+) = 1
+
+def testFuncUsaage =
+  $funcStart() + func1badArg(1, 2) // ok, should only warn on declaration not on usage
+
+/* types */
+type GoodType = Int
+type $StartType = Int // error
+type Middle$Type = Int // error
+type EndType$ = Int // error
+
+val typedVal: Middle$Type = 2 // ok, should only warn on declaration not on usage
+def funcWithDollarTypes(foo: $StartType): Middle$Type = 2 // ok, should only warn on declaration not on usage
+
+/* enums */
+enum GoodEnum:
+  case GoodCase
+  case $BadCaseStart // error
+  case BadCase$Middle // error
+  case BadCaseEnd$ // error
+
+enum $BadEnumStart: // error
+  case GoodCase
+  case $BadCase // error
+
+enum BadEnum$Middle: // error
+  case GoodCase
+  case Bad$Case // error
+
+enum BadEnumEnd$: // error
+  case GoodCase
+  case BadCase$ // error
+
+enum E$numWithEndKeyword: // error
+  case SomeCase
+end E$numWithEndKeyword // ok
+
+def TestEnumUsage(a: $BadEnumStart): Int = // ok, should only warn on declaration not on usage
+  a match
+    case $BadEnumStart.GoodCase => 1
+    case $BadEnumStart.$BadCase => 2 // ok, should only warn on declaration not on usage
+
+/* objects */
+object $ObjectStart: // error
+  val goodVal = 1
+  val $badVal = 2 // error
+
+object Object$Middle: // error
+  val goodVal = 1
+  val bad$Val = 2 // error
+
+object ObjectEnd$: // error
+  val goodVal = 1
+  val badVal$ = 2 // error
+
+object GoodObject:
+  val goodVal = 1
+  val b$adVal = 2 // error
+
+object Ob$jectWithEndKeyword: // error
+  val someVal = 1
+end Ob$jectWithEndKeyword // ok
+
+val testObjectUsage = ObjectEnd$.badVal$ // ok, should only warn on declaration not on usage
+
+/* case classes */
+case class $InlineCaseClassStart(someField: Int) // error
+case class InlineCaseClass$Middle(someField: Int) // error
+case class InlineCaseClassEnd$(someField: Int) // error
+
+case class InlineCaseClass(goodField: Int, badFiel$d: Int) // error
+
+case class $CaseClassStart( // error
+  somefield: Int,
+  b$adfield: Int // error
+)
+
+case class CaseClass$Middle( // error
+  somefield: Int,
+  bad$Field: Int // error
+)
+
+case class CaseClassEnd$( // error
+  somefield: Int,
+  badField$: Int // error
+)
+
+// companion oject
+object CaseClassEnd$: // error
+  val food = 1
+
+val testCaseClassUsage = CaseClass$Middle(somefield = 1, bad$Field = 2) // ok, should only warn on declaration not on usage
+
+/* classes */
+class GoodClass
+class $StartClass // error
+class Middle$Class // error
+class EndClass$ // error
+
+class Cla$$( // error
+   var goodMember: Int,
+   var badM$ember: Int // error
+):
+  def goodMethod(x: Int) = badM$ember // ok, only checking if the method name does not contain a dollar sign
+  def bad$Method(y: Int) = goodMember // error
+  def methodWithBadArgNames(b$ad$arg: Int) = goodMember // error
+  def method$WithEndKeyword() = // error
+    3
+  end method$WithEndKeyword
+end Cla$$ // ok
+
+def testUsage =
+  val instatiation = new Cla$$(goodMember = 1, badM$ember = 2) // ok, should only warn on declaration not on usage
+  instatiation.bad$Method(1) // ok, should only warn on declaration not on usage
+  instatiation.methodWithBadArgNames(2) // ok, should only warn on declaration not on usage
+
+
+/* traits */
+trait GoodTrait
+trait $BadTraitStart // error
+trait BadTrait$Middle // error
+trait BadTraitEnd$ // error
+
+class TestTraitUsage extends $BadTraitStart // ok, should only warn on declaration not on usage
+
+package GoodPackage:
+  val goodVal = 1
+  val b$adVal = 2 // error
+
+package $BadPackageStart: // error
+  val goodVal = 1
+  val $badVal = 2 // error
+
+package BadPackage$Middle: // error
+  val goodVal = 1
+  val bad$Val = 2 // error
+
+package BadPackageEnd$ : // error
+  val goodVal = 1
+  val badVal$ = 2 // error
+
+class BadConstructor:
+  def `<init>`() = () // error // error

tests/pos/i18234.scala

@@ -0,0 +1,3 @@
+
+class `C$Z`:
+  override def toString = "C$Z"