-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Dollars in idents #24690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Dollars in idents #24690
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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.errorOrMigrationWarning(IllegalIdentifier(name), start, MigrationVersion.IdentifierDollars) | ||||||
|
|
||||||
| extension (nameTree: NameTree) def checkName: nameTree.type = | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sometimes this method is called as a function, sometimes as an extension. That's confusing. Given that you sometimes need it just for the side effect, consider standardizing on the function variant:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will consider it. This was my first opportunity to use an extension "both ways", which I think is a feature of extensions. |
||||||
| if !isBackquoted(nameTree) && nameTree.name.toSimpleName.contains('$') then | ||||||
| report.errorOrMigrationWarning(IllegalIdentifier(nameTree.name), nameTree.srcPos, MigrationVersion.IdentifierDollars) | ||||||
| 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 | ||||||
|
|
@@ -3414,6 +3425,9 @@ object Parsers { | |||||
| p = atSpan(startOffset(t), in.offset) { TypeApply(p, typeArgs(namedOK = false, wildOK = false)) } | ||||||
| if (in.token == LPAREN) | ||||||
| p = atSpan(startOffset(t), in.offset) { Apply(p, argumentPatterns()) } | ||||||
| p match | ||||||
| case nt: NameTree if isVarPattern(nt) => checkName(nt) | ||||||
| case _ => | ||||||
| p | ||||||
|
|
||||||
| /** Patterns ::= Pattern [`,' Pattern] | ||||||
|
|
@@ -3773,7 +3787,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 +4117,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 +4173,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 +4244,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 +4336,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 +4361,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 +4381,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 +4395,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( | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,6 @@ class Wrap { | |
| class E | ||
| object E { | ||
| final val A = new E {} | ||
| val $values = Array(A) | ||
| val `$values` = Array(A) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| //> using options -Vprint:parser | ||
|
|
||
| case class $[A](value: A) | ||
| case class `$`[A](value: A) | ||
|
|
||
| def g: Int = $ // error | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gedochao What's the warning policy for the feature freeze?
Should this be
at this point?
Or can we get away with
even though the warning would then be introduced in a patch version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my gut says the former (3.10, 3.11), but I probably could be convinced otherwise.
We haven't had a precedent for this, I guess.
let's discuss it on today's Scala Core.