Skip to content
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

Add support for Scala 3.0.1-RC1 #2852

Merged
merged 1 commit into from Jun 4, 2021
Merged

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Jun 2, 2021

This also includes a bunch of fixes that were kind of a chain reaction due to some tests failing for 3.0.1-RC1

  • print outer type instead of singleton type for objects
  • make sure to take method signature into account when filtering symbols

@tgodzik tgodzik force-pushed the add-support branch 3 times, most recently from e922088 to 0560fa4 Compare June 4, 2021 10:01
@tgodzik tgodzik marked this pull request as ready for review June 4, 2021 10:01
@tgodzik tgodzik requested a review from dos65 June 4, 2021 10:01
@tgodzik tgodzik force-pushed the add-support branch 2 times, most recently from 8e03515 to 50ece65 Compare June 4, 2021 10:05
@@ -151,7 +172,9 @@ class CompletionProvider(

def hasGetter = try {
// isField returns true for some classes
sym.isField || sym.getter != NoSymbol
(sym.isField && !sym.is(JavaDefined)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sym.isField was returning true for a bunch of java classes

@@ -259,7 +282,7 @@ class CompletionProvider(
)
if (byParamCount != 0) byParamCount
else {
s1.show.compareTo(s2.show)
s1.detailString.compareTo(s2.detailString)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should compare the full signature, show for symbol was mostly just name, which wasn't an issue prior to 3.0.1, since before the completions with the same symbol name were grouped together.

}
val typeString = symbols.headOption.map { symbol =>
symbols.flatMap(ParsedComment.docOf(_))
val hoverString = symbols.headOption.map { symbol =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improved it to return the full hover string here, full separation was actually problematic to maintain.

@@ -147,7 +147,7 @@ class HoverDefnSuite extends BaseHoverSuite {
|""".stripMargin,
"",
compat = Map(
"3.0" -> "object MyObject: MyObject".hover
"3.0" -> "object MyObject: object".hover
Copy link
Contributor Author

Choose a reason for hiding this comment

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

object is the name of the package, same in all the other tests.

@@ -147,7 +147,7 @@ class HoverTermSuite extends BaseHoverSuite {
|""".stripMargin.hover,
compat = Map(
// https://github.com/lampepfl/dotty/issues/8835
"3.0" -> "object num: Xtension#num".hover
"3.0" -> "object num: interpolator-unapply.a$.Xtension".hover
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is wrong anyway, we can fix it in a separate PR.

@tgodzik
Copy link
Contributor Author

tgodzik commented Jun 4, 2021

Integration tests seem to be failing unrelated to the changes. I need to take a look at it separately, I think it's one of the test that randomly start failing :(

Copy link
Member

@dos65 dos65 left a comment

Choose a reason for hiding this comment

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

LGTM!

This also includes a bunch of fixes that were kind of a chain reaction due to some tests failing for 3.0.1-RC1
- print outer type instead of singleton type for objects
- make sure to take method signature into account when filtering or sorting symbols
@tgodzik
Copy link
Contributor Author

tgodzik commented Jun 4, 2021

The weird test failure disappeared 🤔 not sure what it was caused by.

@tgodzik tgodzik merged commit 4ceb905 into scalameta:main Jun 4, 2021
@tgodzik tgodzik deleted the add-support branch June 4, 2021 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants