Semanticdb usability enhancements #9768

bishabosha · 2020-09-10T16:17:45Z

This has a bunch of enhancements,

primary constructor definition occurence is zero length
- fixes SemanticDB occurrence for default contructor should be zero length #9727
trees with synthetic spans have zero length nameSpan
- @main def proxy wrappers now have synthetic spans
extension method nameSpan shortened in ExtractSemanticDB to remove the hidden extension_ offset

tgodzik

Thanks for the great work! LGTM I left just a single question, though that might be obvious.

tgodzik · 2020-09-10T16:32:41Z

tests/semanticdb/expect/Enums.expect.scala

      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


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?

Sorry, I'm not sure what you mean

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.

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

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

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.

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

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

compiler/src/dotty/tools/dotc/semanticdb/ExtractSemanticDB.scala

smarter · 2020-09-18T15:47:18Z

compiler/src/dotty/tools/dotc/ast/Trees.scala

+          var length = realName.length
+          if symbol.is(ExtensionMethod) && name.isExtensionName then
+            length -= "extension_".length


Not essential but it might be nicer to have a stripExtensonMethodPrefix just like we have a stripModuleClassSuffix (are there other situations where we need to strip the extenson_ prefix?)

there is the dropExtension method but its only used for import suggestions so far

compiler/src/dotty/tools/dotc/ast/Trees.scala

bishabosha · 2020-09-18T18:23:13Z

cats.effect.concurrent.MVarConcurrentTests failed in the community build

smarter · 2020-09-18T18:30:39Z

Try rebasing, it should be fixed now

Also, proxy wrappers for main annotated methods get a synthetic span

bishabosha · 2020-09-18T21:22:20Z

rebased and added a check to look in the mods also

allanrenucci · 2020-09-19T17:50:43Z

compiler/src/dotty/tools/dotc/ast/Trees.scala

        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


Is the symbol.is(ExtensionMethod) check necessary?

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

liufengyun · 2020-09-28T14:18:14Z

This PR breaks the master.

bishabosha requested review from smarter and tgodzik September 10, 2020 16:17

bishabosha changed the title ~~Semanticdb changes usability~~ Semanticdb usability enhancements Sep 10, 2020

tgodzik approved these changes Sep 10, 2020

View reviewed changes

bishabosha assigned smarter Sep 10, 2020

smarter approved these changes Sep 18, 2020

View reviewed changes

compiler/src/dotty/tools/dotc/semanticdb/ExtractSemanticDB.scala Outdated Show resolved Hide resolved

smarter assigned bishabosha and unassigned smarter Sep 18, 2020

bishabosha assigned smarter and unassigned bishabosha Sep 18, 2020

bishabosha requested a review from smarter September 18, 2020 15:19

smarter approved these changes Sep 18, 2020

View reviewed changes

allanrenucci reviewed Sep 18, 2020

View reviewed changes

compiler/src/dotty/tools/dotc/ast/Trees.scala Outdated Show resolved Hide resolved

bishabosha added 5 commits September 18, 2020 22:10

fix scala#9727: use point span for primary constructor in semanticdb

b51151b

semanticdb: drop extension_ for extension methods

569925d

synthethic span gets zero length nameSpan

1cd1054

Also, proxy wrappers for main annotated methods get a synthetic span

adjust extension method name in nameSpan

f54eecb

also check mods for ExtensionMethod

bc94382

bishabosha force-pushed the semanticdb-changes-usability branch from dd87339 to bc94382 Compare September 18, 2020 21:20

allanrenucci reviewed Sep 19, 2020

View reviewed changes

bishabosha merged commit 374eb94 into scala:master Sep 28, 2020

bishabosha deleted the semanticdb-changes-usability branch September 28, 2020 13:08

bishabosha mentioned this pull request Sep 28, 2020

Revert "Semanticdb usability enhancements" #9906

Merged

bishabosha restored the semanticdb-changes-usability branch September 28, 2020 14:39

bishabosha mentioned this pull request Sep 29, 2020

Reopen - semanticDB usability enhancements #9910

Merged

dos65 mentioned this pull request Jan 21, 2021

SemanticDB occurrence for default contructor should be zero length scalameta/metals#2051

Closed

bishabosha added the area:semanticdb label Aug 7, 2021

Kordyjan added this to the 3.0.0 milestone Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Semanticdb usability enhancements #9768

Semanticdb usability enhancements #9768

bishabosha commented Sep 10, 2020 •

edited

Loading

tgodzik left a comment

tgodzik Sep 10, 2020

bishabosha Sep 10, 2020

tgodzik Sep 11, 2020

bishabosha Sep 11, 2020

bishabosha Sep 11, 2020

tgodzik Sep 11, 2020

bishabosha Sep 23, 2020

bishabosha Sep 23, 2020

smarter Sep 18, 2020

bishabosha Sep 18, 2020

bishabosha commented Sep 18, 2020 •

edited

Loading

smarter commented Sep 18, 2020

bishabosha commented Sep 18, 2020

allanrenucci Sep 19, 2020

bishabosha Sep 19, 2020

liufengyun commented Sep 28, 2020

Semanticdb usability enhancements #9768

Semanticdb usability enhancements #9768

Conversation

bishabosha commented Sep 10, 2020 • edited Loading

tgodzik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishabosha commented Sep 18, 2020 • edited Loading

smarter commented Sep 18, 2020

bishabosha commented Sep 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liufengyun commented Sep 28, 2020

bishabosha commented Sep 10, 2020 •

edited

Loading

bishabosha commented Sep 18, 2020 •

edited

Loading