Skip to content

Correct offset for single-string interpolation #8629

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Mar 30, 2020
Merged

Correct offset for single-string interpolation #8629

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c691beb
failing positions
martijnhoekstra Mar 28, 2020
a8040df
offset correction for stringlit interpolation
martijnhoekstra Mar 29, 2020
e55aa85
adjust repl test position
martijnhoekstra Mar 29, 2020
506a9c4
incorporate review
martijnhoekstra Mar 30, 2020
c3432ff
update expect
martijnhoekstra Mar 30, 2020
31e9464
alternative position correction
martijnhoekstra Mar 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/ast/Desugar.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1665,7 +1665,7 @@ object desugar {
case t => t
}
// This is a deliberate departure from scalac, where StringContext is not rooted (See #4732)
Apply(Select(Apply(scalaDot(nme.StringContext), strs), id), elems)
Apply(Select(Apply(scalaDot(nme.StringContext).withSpan(tree.span), strs), id), elems)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also try this variant to see if the semanticdb test keep on working without changes.

Suggested change
Apply(Select(Apply(scalaDot(nme.StringContext).withSpan(tree.span), strs), id), elems)
Apply(Select(Apply(scalaDot(nme.StringContext), strs), id).withSpan(tree.span), elems)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Giving that a try, you get 31e9464

No more truncated string segments, but I have no idea whether that was a problem in the first place or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better. Maybe @bishabosha can tell us which one is better. If not I would go with this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its better now that the actual method call has a span the length of the interpolator prefix, rather than empty but i guess its not matched exactly to the prefix, but there are other desugaring anomalies like extension methods.
So this is good.

case PostfixOp(t, op) =>
if ((ctx.mode is Mode.Type) && !isBackquoted(op) && op.name == tpnme.raw.STAR) {
val seqType = if (ctx.compilationUnit.isJava) defn.ArrayType else defn.SeqType
Expand Down
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1245,12 +1245,12 @@ object Parsers {
}
})

if (in.token == STRINGPART)
nextSegment(in.offset + (if (isTripleQuoted) 3 else 1))
var offsetCorrection = if isTripleQuoted then 3 else 1
while (in.token == STRINGPART)
nextSegment(in.offset)
nextSegment(in.offset + offsetCorrection)
offsetCorrection = 0
if (in.token == STRINGLIT)
segmentBuf += literal(inPattern = inPattern, negOffset = in.offset, inStringInterpolation = true)
segmentBuf += literal(inPattern = inPattern, negOffset = in.offset + offsetCorrection, inStringInterpolation = true)

InterpolatedString(interpolator, segmentBuf.toList)
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/test-resources/repl/i6676
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ scala> xml"
| ';' expected, but eof found
scala> xml""
1 | xml""
| ^
| value xml is not a member of StringContext
| ^^^^^
| value xml is not a member of StringContext
scala> xml"""
1 | xml"""
| ^
Expand Down
6 changes: 3 additions & 3 deletions tests/semanticdb/expect/Givens.expect.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ package b
object Givens/*<-a::b::Givens.*/:

extension on [A](any: A):
def /*<-a::b::Givens.extension_sayHello_A.*//*<-a::b::Givens.extension_sayHello_A.sayHello().[A]*//*<-a::b::Givens.extension_sayHello_A.sayHello().(any)*//*->a::b::Givens.extension_sayHello_A.sayHello().[A]*/sayHello/*<-a::b::Givens.extension_sayHello_A.sayHello().*/ = s"/*->scala::StringContext.apply().*/Hello, I am $any/*->a::b::Givens.extension_sayHello_A.sayHello().(any)*//*->scala::StringContext#s().*/"
def /*<-a::b::Givens.extension_sayHello_A.*//*<-a::b::Givens.extension_sayHello_A.sayHello().[A]*//*<-a::b::Givens.extension_sayHello_A.sayHello().(any)*//*->a::b::Givens.extension_sayHello_A.sayHello().[A]*/sayHello/*<-a::b::Givens.extension_sayHello_A.sayHello().*/ = s"Hello, I am $any"/*->scala::StringContext.*//*->a::b::Givens.extension_sayHello_A.sayHello().(any)*//*->scala::StringContext#s().*//*->scala::StringContext.apply().*/

extension on [B](any: B):
def sa/*<-a::b::Givens.extension_sayGoodbye_B.*//*<-a::b::Givens.extension_sayGoodbye_B.sayGoodbye().[B]*//*<-a::b::Givens.extension_sayGoodbye_B.saySoLong().[B]*//*<-a::b::Givens.extension_sayGoodbye_B.sayGoodbye().(any)*//*<-a::b::Givens.extension_sayGoodbye_B.saySoLong().(any)*//*->a::b::Givens.extension_sayGoodbye_B.sayGoodbye().[B]*//*->a::b::Givens.extension_sayGoodbye_B.saySoLong().[B]*/yGoodbye/*<-a::b::Givens.extension_sayGoodbye_B.sayGoodbye().*/ = s"/*->scala::StringContext.apply().*/Goodbye, from $any/*->a::b::Givens.extension_sayGoodbye_B.sayGoodbye().(any)*//*->scala::StringContext#s().*/"
def saySoLong/*<-a::b::Givens.extension_sayGoodbye_B.saySoLong().*/ = s"/*->scala::StringContext.apply().*/So Long, from $any/*->a::b::Givens.extension_sayGoodbye_B.saySoLong().(any)*//*->scala::StringContext#s().*/"
def sa/*<-a::b::Givens.extension_sayGoodbye_B.*//*<-a::b::Givens.extension_sayGoodbye_B.sayGoodbye().[B]*//*<-a::b::Givens.extension_sayGoodbye_B.saySoLong().[B]*//*<-a::b::Givens.extension_sayGoodbye_B.sayGoodbye().(any)*//*<-a::b::Givens.extension_sayGoodbye_B.saySoLong().(any)*//*->a::b::Givens.extension_sayGoodbye_B.sayGoodbye().[B]*//*->a::b::Givens.extension_sayGoodbye_B.saySoLong().[B]*/yGoodbye/*<-a::b::Givens.extension_sayGoodbye_B.sayGoodbye().*/ = s"Goodbye, from $any"/*->scala::StringContext.*//*->a::b::Givens.extension_sayGoodbye_B.sayGoodbye().(any)*//*->scala::StringContext#s().*//*->scala::StringContext.apply().*/
def saySoLong/*<-a::b::Givens.extension_sayGoodbye_B.saySoLong().*/ = s"So Long, from $any"/*->scala::StringContext.*//*->a::b::Givens.extension_sayGoodbye_B.saySoLong().(any)*//*->scala::StringContext#s().*//*->scala::StringContext.apply().*/

val hello1/*<-a::b::Givens.hello1.*/ = /*->a::b::Givens.extension_sayHello_A.sayHello().*/1.sayHello
val goodbye1/*<-a::b::Givens.goodbye1.*/ = /*->a::b::Givens.extension_sayGoodbye_B.sayGoodbye().*/1.sayGoodbye
Expand Down
6 changes: 3 additions & 3 deletions tests/semanticdb/expect/ImplicitConversion.expect.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ class ImplicitConversion/*<-example::ImplicitConversion#*/ {
val x/*<-example::ImplicitConversion#x.*/: Int/*->scala::Int#*/ = /*->example::ImplicitConversion#string2Number().*/message/*->example::ImplicitConversion#message.*/

// interpolators
s"/*->scala::StringContext.apply().*/Hello $message/*->example::ImplicitConversion#message.*/ $number/*->example::ImplicitConversion#number.*//*->scala::StringContext#s().*/"
/*->scala::Predef.augmentString().*/s"""/*->scala::StringContext.apply().*/Hello
s"Hello $message/*->example::ImplicitConversion#message.*/ $number"/*->scala::StringContext.*//*->example::ImplicitConversion#number.*//*->scala::StringContext#s().*//*->scala::StringContext.apply().*/
/*->scala::Predef.augmentString().*/s"""Hello
|$message/*->example::ImplicitConversion#message.*/
|$number/*->example::ImplicitConversion#number.*//*->scala::StringContext#s().*/""".stripMargin/*->scala::collection::StringOps#stripMargin(+1).*/
|$number"""/*->scala::StringContext.*//*->example::ImplicitConversion#number.*//*->scala::StringContext#s().*//*->scala::StringContext.apply().*/.stripMargin/*->scala::collection::StringOps#stripMargin(+1).*/

val a/*<-example::ImplicitConversion#a.*/: Int/*->scala::Int#*/ = /*->scala::Char.char2int().*/char/*->example::ImplicitConversion#char.*/
val b/*<-example::ImplicitConversion#b.*/: Long/*->scala::Long#*/ = /*->scala::Char.char2long().*/char/*->example::ImplicitConversion#char.*/
Expand Down
29 changes: 17 additions & 12 deletions tests/semanticdb/metac.expect
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ Uri => Givens.scala
Text => empty
Language => Scala
Symbols => 31 entries
Occurrences => 72 entries
Occurrences => 75 entries

Symbols:
a/b/Givens. => final object Givens
Expand Down Expand Up @@ -1313,9 +1313,10 @@ Occurrences:
[5:19..5:22): any <- a/b/Givens.extension_sayHello_A.sayHello().(any)
[5:24..5:25): A -> a/b/Givens.extension_sayHello_A.sayHello().[A]
[6:8..6:16): sayHello <- a/b/Givens.extension_sayHello_A.sayHello().
[6:21..6:21): -> scala/StringContext.apply().
[6:25..6:38): o, I am $any" -> scala/StringContext.
[6:34..6:37): any -> a/b/Givens.extension_sayHello_A.sayHello().(any)
[6:37..6:37): -> scala/StringContext#s().
[6:38..6:38): -> scala/StringContext#s().
[6:38..6:38): -> scala/StringContext.apply().
[8:16..9:10): <- a/b/Givens.extension_sayGoodbye_B.
[8:16..8:17): B <- a/b/Givens.extension_sayGoodbye_B.sayGoodbye().[B]
[8:16..8:17): B <- a/b/Givens.extension_sayGoodbye_B.saySoLong().[B]
Expand All @@ -1324,13 +1325,15 @@ Occurrences:
[8:24..8:25): B -> a/b/Givens.extension_sayGoodbye_B.sayGoodbye().[B]
[8:24..8:25): B -> a/b/Givens.extension_sayGoodbye_B.saySoLong().[B]
[9:8..9:18): sayGoodbye <- a/b/Givens.extension_sayGoodbye_B.sayGoodbye().
[9:23..9:23): -> scala/StringContext.apply().
[9:29..9:42): e, from $any" -> scala/StringContext.
[9:38..9:41): any -> a/b/Givens.extension_sayGoodbye_B.sayGoodbye().(any)
[9:41..9:41): -> scala/StringContext#s().
[9:42..9:42): -> scala/StringContext#s().
[9:42..9:42): -> scala/StringContext.apply().
[10:8..10:17): saySoLong <- a/b/Givens.extension_sayGoodbye_B.saySoLong().
[10:22..10:22): -> scala/StringContext.apply().
[10:28..10:41): g, from $any" -> scala/StringContext.
[10:37..10:40): any -> a/b/Givens.extension_sayGoodbye_B.saySoLong().(any)
[10:40..10:40): -> scala/StringContext#s().
[10:41..10:41): -> scala/StringContext#s().
[10:41..10:41): -> scala/StringContext.apply().
[12:6..12:12): hello1 <- a/b/Givens.hello1.
[12:15..12:15): -> a/b/Givens.extension_sayHello_A.sayHello().
[13:6..13:14): goodbye1 <- a/b/Givens.goodbye1.
Expand Down Expand Up @@ -1387,7 +1390,7 @@ Uri => ImplicitConversion.scala
Text => empty
Language => Scala
Symbols => 23 entries
Occurrences => 62 entries
Occurrences => 64 entries

Symbols:
example/ImplicitConversion# => class ImplicitConversion
Expand Down Expand Up @@ -1442,15 +1445,17 @@ Occurrences:
[20:9..20:12): Int -> scala/Int#
[20:15..20:15): -> example/ImplicitConversion#string2Number().
[20:15..20:22): message -> example/ImplicitConversion#message.
[23:4..23:4): -> scala/StringContext.apply().
[23:11..23:18): message -> example/ImplicitConversion#message.
[23:14..23:27): sage $number" -> scala/StringContext.
[23:20..23:26): number -> example/ImplicitConversion#number.
[23:26..23:26): -> scala/StringContext#s().
[23:27..23:27): -> scala/StringContext#s().
[23:27..23:27): -> scala/StringContext.apply().
[24:2..24:2): -> scala/Predef.augmentString().
[24:6..24:6): -> scala/StringContext.apply().
[25:7..25:14): message -> example/ImplicitConversion#message.
[26:3..26:16): |$number""" -> scala/StringContext.
[26:7..26:13): number -> example/ImplicitConversion#number.
[26:13..26:13): -> scala/StringContext#s().
[26:16..26:16): -> scala/StringContext#s().
[26:16..26:16): -> scala/StringContext.apply().
[26:17..26:28): stripMargin -> scala/collection/StringOps#stripMargin(+1).
[28:6..28:7): a <- example/ImplicitConversion#a.
[28:9..28:12): Int -> scala/Int#
Expand Down