Skip to content

Add IDE tests #3766

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

Merged
merged 19 commits into from
Apr 12, 2018
Merged

Add IDE tests #3766

merged 19 commits into from
Apr 12, 2018

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

.drone.yml Outdated
group: test
image: lampepfl/dotty:2017-11-17
commands:
- cp -R . /tmp/3/ && cd /tmp/3/
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be a unique directory

@@ -59,6 +59,13 @@ object DiffUtil {
(fnd, exp, totalChange.toDouble / (expected.length + found.length))
}

def mkColoredLineDiff(expected: Seq[String], actual: Seq[String]): String = {
Copy link
Member

Choose a reason for hiding this comment

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

Add some documentation?


class CompletionTest {

@Test def competion0: Unit = {
Copy link
Member

Choose a reason for hiding this comment

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

typo: competion -> completion


import dotty.tools.languageserver.util.embedded._

object Code {
Copy link
Member

Choose a reason for hiding this comment

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

This file, and every other file in this directory would benefit from some documentation.


ai.next() match {
case emb: CodeMarker =>
positions += Tuple3(emb, line, char)
Copy link
Member

Choose a reason for hiding this comment

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

You can use += ((a, b, c))

var char = 0
def scan(str: String): Unit = {
for (c <- str)
if (c == '\n') { line += 1; char = 0 } else { char += 1 }
Copy link
Member

@smarter smarter Feb 16, 2018

Choose a reason for hiding this comment

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

Isn't this just line = str.count(_ == '\n'); char = str.length - line ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not exactly, char needs to be set to 0 when we encounter \n


override def onMarker(marker: CodeMarker): Exec[Unit] = {
val res = server.definition(fix(marker.toTextDocumentPositionParams)).get()
assert(res.size() == refOpt.size, res)
Copy link
Member

Choose a reason for hiding this comment

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

size() -> size

Copy link
Contributor Author

Choose a reason for hiding this comment

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

res is a java collection, hence it requires size().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was complaining or waring at the time. I will double check.

import dotty.tools.languageserver.util._
import dotty.tools.languageserver.util.embedded.CodeMarker

class CodeDefinition(val range: CodeRange, refOpt: Option[CodeRange]) extends ActionOnRange {
Copy link
Member

Choose a reason for hiding this comment

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

We can return more than one definition.


def to(other: CodeMarker): CodeRange = CodeRange(this, other)

def file: PosCtx[TestFile] = implicitly[PositionContext].positionOf(this)._1
Copy link
Member

Choose a reason for hiding this comment

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

You could add implicit def posCtx(implicit ctx: PositionContext): PositionContext = ctx in the class to be able to do posCtx.positionOf ...

| "compilerVersion" : "0.6.0-bin-SNAPSHOT-nonbootstrapped",
| "compilerArguments" : [ "-feature", "-deprecation", "-unchecked", "-Xfatal-warnings", "-encoding", "UTF8", "-language:existentials,higherKinds,implicitConversions" ],
| "sourceDirectories" : [ "$baseDir/out/ide-tests/src" ],
| "dependencyClasspath" : [ "$baseDir/library/../out/bootstrap/dotty-library-bootstrapped/scala-0.7/classes", "$ivyDir/cache/org.scala-lang/scala-library/jars/scala-library-2.12.4.jar" ],
Copy link
Member

Choose a reason for hiding this comment

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

The hardcoded version number should be fixed before this is merged.

import dotty.tools.languageserver.util.PositionContext._
import org.eclipse.lsp4j._

class SymInfo(name: String, kind: String, range: CodeRange, container: String) {
Copy link
Member

Choose a reason for hiding this comment

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

use SymbolKind for kind instead of String ?

@smarter
Copy link
Member

smarter commented Feb 22, 2018 via email

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

This looks great!

@@ -21,8 +21,8 @@ class CodeCompletion(override val marker: CodeMarker,

override def execute(): Exec[Unit] = {
val result = server.completion(marker.toTextDocumentPositionParams).get()
assert(result.isRight, result)
assert(!result.getRight.isIncomplete, s"Completion results were 'incomplete': $result")
assertTrue(s"Completion results where not 'right': $result", result.isRight)
Copy link
Member

Choose a reason for hiding this comment

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

typo: where -> where

Choose a reason for hiding this comment

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

-> were

*
* @see dotty.tools.languageserver.util.actions.CodeHover
*/
def hover(range: CodeRange, expected: String): CodeTester =
Copy link
Member

Choose a reason for hiding this comment

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

This method and the following ones could return this.type instead of CodeTester, that way you don't have to document @return, it's obvious.

@Duhemm
Copy link
Contributor

Duhemm commented Apr 12, 2018

@nicolasstucki Do you want to add something?

Copy link
Contributor Author

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

Changes from @Duhemm LGTM

@allanrenucci
Copy link
Contributor

@Duhemm Can you please merge master into topic/sbt1 if this happens to be merged before sbt1?

@allanrenucci allanrenucci removed their request for review April 12, 2018 11:05
nicolasstucki and others added 19 commits April 12, 2018 18:55
While working on the IDE tests, I noticed that the Dotty IDE didn't
behave as I expected in the following scenario:

  // Rename `Foo` to `Bar`
  class Foo { new Foo }

The Dotty IDE would consider that 3 places need changing, instead of
only 2:

 - The `Ident(Foo)` in the `TypeDef`
 - The `Ident(Foo)` in the constructor
 - The `Ident(Foo)` in `Select(new Foo, <init>)`

The third tree, however is synthetic: it doesn't need to be changed in
the editor.

Trees whose positions are not derived from the source are now excluded
when doing a rename.
@Duhemm Duhemm merged commit 0ba6d2e into scala:master Apr 12, 2018
@Duhemm Duhemm deleted the ide-tests branch April 12, 2018 17:29
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.

5 participants