Fix type alias completion, unify completion tests style #15047
Conversation
| @@ -1565,6 +1549,21 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||
| val selType = rawSelectorTpe match | |||
| case c: ConstantType if tree.isInline => c | |||
| case otherTpe => otherTpe.widen | |||
| /** Extractor for match types hidden behind an AppliedType/MatchAlias */ | |||
There was a problem hiding this comment.
Reverts changes from https://github.com/lampepfl/dotty/pull/14639/files#diff-8c9ece1772bd78160fc1c31e988664586c9df566a1d22ff99ef99dd6d5627a90R1518
Match Type completion is also fixed by this commit without using this extractor.
| .completion(m1, Set(("example", Module, "example"))) | ||
| } | ||
|
|
||
| @Test def typeAliasCompletions: Unit = { |
There was a problem hiding this comment.
Tests below check the issue of missing type alias completion. Changes above are style changes to unify them in this file.
| @@ -106,15 +114,16 @@ class CompletionTest { | |||
| } | |||
|
|
|||
| @Test def importCompleteIncludesSynthetic: Unit = { | |||
| val expected = Set( | |||
There was a problem hiding this comment.
Personally I'm not a big fan of putting the expected values before the code snippets. IMHO this makes the tests harder to read (to understand what a test is about I read the snippet first and then Ι go back to the expected results) so I would rather leave expected values inlined at the end. However to make things more readable than currently we could:
- unify formatting (e.g. don't put multiple expected values in the same line)
- get rid of the very repetitive
code"""...""".withSourcepattern (e.g. by adding an implicit conversion) - overload
completionto accept varargs so that we get rid ofSet(...)- also we could makem1default as in most cases we need only one code marker
After these changes this test could look something like
@Test def importCompleteIncludesSynthetic: Unit = {
code"""case class MyCaseClass(foobar: Int)
object O {
val x = MyCaseClass(0)
import x.c${m1}
}"""
.completion(
("copy", Method, "(foobar: Int): MyCaseClass"),
("canEqual", Method, "(that: Any): Boolean")
)
}There was a problem hiding this comment.
I totally agree. I made quite a makeover there but all language server tests can now omit .withSource. Furthermore completions now take marker m1 as default one and allows completions to be passed as varargs.
|
Awesome! Thanks @rochala ! |
Fixes #14984
Also unifies style in completion test as there were different styles of writing same test which made it harded to analyze and read.