Skip to content

Semanticdb usability enhancements #9768

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 5 commits into from
Sep 28, 2020
Merged

Semanticdb usability enhancements #9768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b51151b
fix #9727: use point span for primary constructor in semanticdb
bishabosha Sep 10, 2020
569925d
semanticdb: drop extension_ for extension methods
bishabosha Sep 10, 2020
1cd1054
synthethic span gets zero length nameSpan
bishabosha Sep 10, 2020
f54eecb
adjust extension method name in nameSpan
bishabosha Sep 18, 2020
bc94382
also check mods for ExtensionMethod
bishabosha Sep 18, 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
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/MainProxies.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ object MainProxies {
val mainTempl = Template(emptyConstructor, Nil, Nil, EmptyValDef, mainMeth :: Nil)
val mainCls = TypeDef(mainFun.name.toTypeName, mainTempl)
.withFlags(Final)
if (!ctx.reporter.hasErrors) result = mainCls.withSpan(mainAnnotSpan) :: Nil
if (!ctx.reporter.hasErrors) result = mainCls.withSpan(mainAnnotSpan.toSynthetic) :: Nil
}
result
}
}
}
13 changes: 8 additions & 5 deletions compiler/src/dotty/tools/dotc/ast/Trees.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,13 +337,16 @@ object Trees {
* a calling chain from `viewExists`), in that case the return position is NoSpan.
* Overridden in Bind
*/
def nameSpan: Span =
def nameSpan(using Context): Span =
if (span.exists) {
val point = span.point
if (rawMods.is(Synthetic) || name.toTermName == nme.ERROR) Span(point)
if (rawMods.is(Synthetic) || span.isSynthetic || name.toTermName == nme.ERROR) Span(point)
else {
val realName = name.stripModuleClassSuffix.lastPart
Span(point, point + realName.length, point)
var length = realName.length
if (mods.is(ExtensionMethod) || symbol.is(ExtensionMethod)) && name.isExtensionName then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the symbol.is(ExtensionMethod) check necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was told once that mods may not always be copied over beyond some phase but I'm not sure about this

length -= "extension_".length
Span(point, point + length, point)
}
}
else span
Expand All @@ -352,7 +355,7 @@ object Trees {
* This is a point position if the definition is synthetic, or a range position
* if the definition comes from source.
*/
def namePos: SourcePosition = source.atSpan(nameSpan)
def namePos(using Context): SourcePosition = source.atSpan(nameSpan)
}

/** Tree defines a new symbol and carries modifiers.
Expand Down Expand Up @@ -711,7 +714,7 @@ object Trees {
override def isType: Boolean = name.isTypeName
override def isTerm: Boolean = name.isTermName

override def nameSpan: Span =
override def nameSpan(using Context): Span =
if span.exists then Span(span.start, span.start + name.toString.length) else span
}

Expand Down
14 changes: 3 additions & 11 deletions compiler/src/dotty/tools/dotc/semanticdb/ExtractSemanticDB.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class ExtractSemanticDB extends Phase:
return
if !excludeDef(tree.symbol)
&& tree.span.hasLength then
registerDefinition(tree.symbol, tree.adjustedNameSpan, symbolKinds(tree), tree.source)
registerDefinition(tree.symbol, tree.nameSpan, symbolKinds(tree), tree.source)
val privateWithin = tree.symbol.privateWithin
if privateWithin.exists then
registerUseGuarded(None, privateWithin, spanOfSymbol(privateWithin, tree.span, tree.source), tree.source)
Expand Down Expand Up @@ -184,7 +184,7 @@ class ExtractSemanticDB extends Phase:
val ctorSym = tree.constr.symbol
if !excludeDef(ctorSym) then
traverseAnnotsOfDefinition(ctorSym)
registerDefinition(ctorSym, tree.constr.span, Set.empty, tree.source)
registerDefinition(ctorSym, tree.constr.nameSpan.startPos, Set.empty, tree.source)
ctorParams(tree.constr.vparamss, tree.body)
for parent <- tree.parentsOrDerived if parent.span.hasLength do
traverse(parent)
Expand Down Expand Up @@ -283,13 +283,6 @@ class ExtractSemanticDB extends Phase:

end PatternValDef

extension (tree: NamedDefTree):
private def adjustedNameSpan(using Context): Span =
if tree.span.exists && tree.name.isAnonymousFunctionName || tree.name.isAnonymousClassName then
Span(tree.span.point)
else
tree.nameSpan

/** Add semanticdb name of the given symbol to string builder */
private def addSymName(b: StringBuilder, sym: Symbol)(using Context): Unit =

Expand Down Expand Up @@ -530,8 +523,7 @@ class ExtractSemanticDB extends Phase:
Span(start max limit, end)

extension (span: Span):
private def hasLength: Boolean = span.start != span.end
private def zeroLength: Boolean = span.start == span.end
private def hasLength: Boolean = span.exists && !span.isZeroExtent

/**Consume head while not an import statement.
* Returns the rest of the list after the first import, or else the empty list
Expand Down
6 changes: 3 additions & 3 deletions tests/semanticdb/expect/Enums.expect.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ object Enums/*<-_empty_::Enums.*/:
case Hearts/*<-_empty_::Enums.Suits.Hearts.*/, Spades/*<-_empty_::Enums.Suits.Spades.*/, Clubs/*<-_empty_::Enums.Suits.Clubs.*/, Diamonds/*<-_empty_::Enums.Suits.Diamonds.*/

object Suits/*<-_empty_::Enums.Suits.*/:
extension (suit/*<-_empty_::Enums.Suits.extension_isRed().(suit)*/: Suits/*->_empty_::Enums.Suits#*/) def isRed: Boolean /*<-_empty_::Enums.Suits.extension_isRed().*//*->scala::Boolean#*/=
extension (suit/*<-_empty_::Enums.Suits.extension_isRed().(suit)*/: Suits/*->_empty_::Enums.Suits#*/) def isRed/*<-_empty_::Enums.Suits.extension_isRed().*/: Boolean/*->scala::Boolean#*/ =
suit/*->_empty_::Enums.Suits.extension_isRed().(suit)*/ ==/*->scala::Any#`==`().*/ Hearts/*->_empty_::Enums.Suits.Hearts.*/ ||/*->scala::Boolean#`||`().*/ suit/*->_empty_::Enums.Suits.extension_isRed().(suit)*/ ==/*->scala::Any#`==`().*/ Diamonds/*->_empty_::Enums.Suits.Diamonds.*/

extension (suit/*<-_empty_::Enums.Suits.extension_isBlack().(suit)*/: Suits/*->_empty_::Enums.Suits#*/) def isBlack: Boolean /*<-_empty_::Enums.Suits.extension_isBlack().*//*->scala::Boolean#*/= suit/*->_empty_::Enums.Suits.extension_isBlack().(suit)*/ match
extension (suit/*<-_empty_::Enums.Suits.extension_isBlack().(suit)*/: Suits/*->_empty_::Enums.Suits#*/) def isBlack/*<-_empty_::Enums.Suits.extension_isBlack().*/: Boolean/*->scala::Boolean#*/ = suit/*->_empty_::Enums.Suits.extension_isBlack().(suit)*/ match
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the name be something like extension-isBlack(). ? I think we can't name a method like that, but we can name a method extension_isBlack Or is that not an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I'm not sure what you mean

Copy link
Contributor

Choose a reason for hiding this comment

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

I am just wondering what is the string representation for extension methods (since basically everything is represented by string in semanticdb) and can it not conflict with a normal method in the class prefixed with extension_
It is highly unlikely that a method like that will be created, but I wonder if we should maybe change the string representation for extension methods.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, there is a check in desugaring that proves only extension methods can have that prefix, although its probably not been enforced in macros

Copy link
Member Author

Choose a reason for hiding this comment

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

actually it seems val/vars can also have this name 😕

Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think about using extension- to signal it's an extension? We can't use - in the name so it should be good.

Copy link
Member Author

Choose a reason for hiding this comment

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

so extension [T,U](t: T) def <*>: (T,U) = ??? would be extension-`<*>`().?

Copy link
Member Author

Choose a reason for hiding this comment

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

the other alternative is that we add another Property kind (alongside FINAL, SEALED, etc.)

case Spades/*->_empty_::Enums.Suits.Spades.*/ | Clubs/*->_empty_::Enums.Suits.Clubs.*/ => true
case _ => false

Expand Down Expand Up @@ -49,7 +49,7 @@ object Enums/*<-_empty_::Enums.*/:
object <:</*<-_empty_::Enums.`<:<`.*/ :
given [T] as /*<-_empty_::Enums.`<:<`.given_T().*//*<-_empty_::Enums.`<:<`.given_T().[T]*/(T/*->_empty_::Enums.`<:<`.given_T().[T]*/ <:</*->_empty_::Enums.`<:<`#*/ T/*->_empty_::Enums.`<:<`.given_T().[T]*/) = Refl/*->_empty_::Enums.`<:<`.Refl.*//*->_empty_::Enums.`<:<`.Refl.apply().*/()

extension [A/*<-_empty_::Enums.extension_unwrap().[A]*/, B/*<-_empty_::Enums.extension_unwrap().[B]*/](opt/*<-_empty_::Enums.extension_unwrap().(opt)*/: Option/*->scala::Option#*/[A/*->_empty_::Enums.extension_unwrap().[A]*/]) def unwrap(using ev:/*<-_empty_::Enums.extension_unwrap().*//*<-_empty_::Enums.extension_unwrap().(ev)*/ A/*->_empty_::Enums.extension_unwrap().[A]*/ <:</*->_empty_::Enums.`<:<`#*/ Option/*->scala::Option#*/[B/*->_empty_::Enums.extension_unwrap().[B]*/]): Option/*->scala::Option#*/[B/*->_empty_::Enums.extension_unwrap().[B]*/] = ev/*->_empty_::Enums.extension_unwrap().(ev)*/ match
extension [A/*<-_empty_::Enums.extension_unwrap().[A]*/, B/*<-_empty_::Enums.extension_unwrap().[B]*/](opt/*<-_empty_::Enums.extension_unwrap().(opt)*/: Option/*->scala::Option#*/[A/*->_empty_::Enums.extension_unwrap().[A]*/]) def unwrap/*<-_empty_::Enums.extension_unwrap().*/(using ev/*<-_empty_::Enums.extension_unwrap().(ev)*/: A/*->_empty_::Enums.extension_unwrap().[A]*/ <:</*->_empty_::Enums.`<:<`#*/ Option/*->scala::Option#*/[B/*->_empty_::Enums.extension_unwrap().[B]*/]): Option/*->scala::Option#*/[B/*->_empty_::Enums.extension_unwrap().[B]*/] = ev/*->_empty_::Enums.extension_unwrap().(ev)*/ match
case Refl/*->_empty_::Enums.`<:<`.Refl.*//*->_empty_::Enums.`<:<`.Refl.unapply().*/() => opt/*->_empty_::Enums.extension_unwrap().(opt)*/.flatMap/*->scala::Option#flatMap().*/(identity/*->scala::Predef.identity().*//*->local0*/[Option/*->scala::Option#*/[B/*->_empty_::Enums.extension_unwrap().[B]*/]])

val some1/*<-_empty_::Enums.some1.*/ = /*->_empty_::Enums.extension_unwrap().*/Some/*->scala::Some.*//*->scala::Some.apply().*/(Some/*->scala::Some.*//*->scala::Some.apply().*/(1))/*->_empty_::Enums.`<:<`.given_T().*/.unwrap
Expand Down
12 changes: 6 additions & 6 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,23 +4,23 @@ package b
object Givens/*<-a::b::Givens.*/:

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

extension [B/*<-a::b::Givens.extension_sayGoodbye().[B]*//*<-a::b::Givens.extension_saySoLong().[B]*/](any/*<-a::b::Givens.extension_sayGoodbye().(any)*//*<-a::b::Givens.extension_saySoLong().(any)*/: B/*->a::b::Givens.extension_sayGoodbye().[B]*//*->a::b::Givens.extension_saySoLong().[B]*/)
def sayGoodbye = s"Goodb/*<-a::b::Givens.extension_sayGoodbye().*//*->scala::StringContext.apply().*/ye, from $any/*->a::b::Givens.extension_sayGoodbye().(any)*/"/*->scala::StringContext#s().*/
def saySoLong = s"So Lo/*<-a::b::Givens.extension_saySoLong().*//*->scala::StringContext.apply().*/ng, from $any/*->a::b::Givens.extension_saySoLong().(any)*/"/*->scala::StringContext#s().*/
def sayGoodbye/*<-a::b::Givens.extension_sayGoodbye().*/ = s"/*->scala::StringContext.apply().*/Goodbye, from $any/*->a::b::Givens.extension_sayGoodbye().(any)*/"/*->scala::StringContext#s().*/
def saySoLong/*<-a::b::Givens.extension_saySoLong().*/ = s"/*->scala::StringContext.apply().*/So Long, from $any/*->a::b::Givens.extension_saySoLong().(any)*/"/*->scala::StringContext#s().*/

val hello1/*<-a::b::Givens.hello1.*/ = /*->a::b::Givens.extension_sayHello().*/1.sayHello
val goodbye1/*<-a::b::Givens.goodbye1.*/ = /*->a::b::Givens.extension_sayGoodbye().*/1.sayGoodbye
val soLong1/*<-a::b::Givens.soLong1.*/ = /*->a::b::Givens.extension_saySoLong().*/1.saySoLong

trait Monoid/*<-a::b::Givens.Monoid#*/[A/*<-a::b::Givens.Monoid#[A]*/]:
def empty/*<-a::b::Givens.Monoid#empty().*/: A/*->a::b::Givens.Monoid#[A]*/
extension (x/*<-a::b::Givens.Monoid#extension_combine().(x)*/: A/*->a::b::Givens.Monoid#[A]*/) def combine(y: A): A
/*<-a::b::Givens.Monoid#extension_combine().*//*<-a::b::Givens.Monoid#extension_combine().(y)*//*->a::b::Givens.Monoid#[A]*//*->a::b::Givens.Monoid#[A]*/
extension (x/*<-a::b::Givens.Monoid#extension_combine().(x)*/: A/*->a::b::Givens.Monoid#[A]*/) def combine/*<-a::b::Givens.Monoid#extension_combine().*/(y/*<-a::b::Givens.Monoid#extension_combine().(y)*/: A/*->a::b::Givens.Monoid#[A]*/): A/*->a::b::Givens.Monoid#[A]*/

given Monoid[String]:
/*<-a::b::Givens.given_Monoid_String.*//*->a::b::Givens.Monoid#*//*->scala::Predef.String#*/ def empty/*<-a::b::Givens.given_Monoid_String.empty().*/ = ""
extension (x/*<-a::b::Givens.given_Monoid_String.extension_combine().(x)*/: String/*->scala::Predef.String#*/) def combine(y: String/*<-a::b::Givens.given_Monoid_String.extension_combine().*//*<-a::b::Givens.given_Monoid_String.extension_combine().(y)*//*->scala::Predef.String#*/) = x/*->a::b::Givens.given_Monoid_String.extension_combine().(x)*/ +/*->java::lang::String#`+`().*/ y/*->a::b::Givens.given_Monoid_String.extension_combine().(y)*/
extension (x/*<-a::b::Givens.given_Monoid_String.extension_combine().(x)*/: String/*->scala::Predef.String#*/) def combine/*<-a::b::Givens.given_Monoid_String.extension_combine().*/(y/*<-a::b::Givens.given_Monoid_String.extension_combine().(y)*/: String/*->scala::Predef.String#*/) = x/*->a::b::Givens.given_Monoid_String.extension_combine().(x)*/ +/*->java::lang::String#`+`().*/ y/*->a::b::Givens.given_Monoid_String.extension_combine().(y)*/

inline given int2String/*<-a::b::Givens.int2String().*/ as Conversion/*->scala::Conversion#*/[Int/*->scala::Int#*/, String/*->scala::Predef.String#*/] = _.toString/*->scala::Any#toString().*/

Expand Down
5 changes: 5 additions & 0 deletions tests/semanticdb/expect/i9727.expect.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package i9727

class Test/*<-i9727::Test#*/(a/*<-i9727::Test#a.*/: Int/*->scala::Int#*/)
/*<-i9727::i9727$package.*/val a/*<-i9727::i9727$package.a.*/ = new Test/*->i9727::Test#*/(1)
val b/*<-i9727::i9727$package.b.*/ = new Test/*->i9727::Test#*/(2)
5 changes: 5 additions & 0 deletions tests/semanticdb/expect/i9727.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package i9727

class Test(a: Int)
val a = new Test(1)
val b = new Test(2)
5 changes: 3 additions & 2 deletions tests/semanticdb/expect/toplevel.expect.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
inline val a = "/*<-_empty_::toplevel$package.*//*<-_empty_::toplevel$package.a.*/"
extension (x/*<-_empty_::toplevel$package.extension_combine().(x)*/: Int/*->scala::Int#*/) def combine (y: Int) /*<-_empty_::toplevel$package.extension_combine().*//*<-_empty_::toplevel$package.extension_combine().(y)*//*->scala::Int#*/= x/*->_empty_::toplevel$package.extension_combine().(x)*/ +/*->scala::Int#`+`(+4).*/ y/*->_empty_::toplevel$package.extension_combine().(y)*/
/*<-_empty_::toplevel$package.*/inline val a/*<-_empty_::toplevel$package.a.*/ = ""
extension (x/*<-_empty_::toplevel$package.extension_combine().(x)*/: Int/*->scala::Int#*/) def combine/*<-_empty_::toplevel$package.extension_combine().*/ (y/*<-_empty_::toplevel$package.extension_combine().(y)*/: Int/*->scala::Int#*/) = x/*->_empty_::toplevel$package.extension_combine().(x)*/ +/*->scala::Int#`+`(+4).*/ y/*->_empty_::toplevel$package.extension_combine().(y)*/
def combine/*<-_empty_::toplevel$package.combine().*/(x/*<-_empty_::toplevel$package.combine().(x)*/: Int/*->scala::Int#*/, y/*<-_empty_::toplevel$package.combine().(y)*/: Int/*->scala::Int#*/, z/*<-_empty_::toplevel$package.combine().(z)*/: Int/*->scala::Int#*/) = x/*->_empty_::toplevel$package.combine().(x)*/ +/*->scala::Int#`+`(+4).*/ y/*->_empty_::toplevel$package.combine().(y)*/ +/*->scala::Int#`+`(+4).*/ z/*->_empty_::toplevel$package.combine().(z)*/
def combine/*<-_empty_::toplevel$package.combine(+1).*/ = 0
def foo/*<-_empty_::toplevel$package.foo().*/ = "foo"
/*<-_empty_::MyProgram#*//*->_empty_::toplevel$package.MyProgram().*//*->scala::util::CommandLineParser.parseArgument().*//*->_empty_::MyProgram#main().(args)*//*->scala::util::FromString.given_FromString_Int.*//*->scala::util::CommandLineParser.showError().*//*->local0*/@main/*->scala::main#*/ def MyProgram/*<-_empty_::toplevel$package.MyProgram().*/(times/*<-_empty_::toplevel$package.MyProgram().(times)*/: Int/*->scala::Int#*/): Unit/*->scala::Unit#*/ = (/*->scala::LowPriorityImplicits#intWrapper().*/1 to/*->scala::runtime::RichInt#to().*/ times/*->_empty_::toplevel$package.MyProgram().(times)*/) foreach/*->scala::collection::immutable::Range#foreach().*/ (_ => println/*->scala::Predef.println(+1).*/("hello"))
1 change: 1 addition & 0 deletions tests/semanticdb/expect/toplevel.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ extension (x: Int) def combine (y: Int) = x + y
def combine(x: Int, y: Int, z: Int) = x + y + z
def combine = 0
def foo = "foo"
@main def MyProgram(times: Int): Unit = (1 to times) foreach (_ => println("hello"))
Loading