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

Fix #3814 Correct highlighting issues in REPL #3987

Merged
merged 4 commits into from
May 7, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
28 changes: 23 additions & 5 deletions compiler/src/dotty/tools/dotc/printing/SyntaxHighlighting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ object SyntaxHighlighting {
'q' :: 'r' :: 's' :: 't' :: 'u' :: 'v' :: 'w' :: 'x' :: 'y' :: 'z' :: Nil

private val typeEnders =
'{' :: '}' :: ')' :: '(' :: '[' :: ']' :: '=' :: ' ' :: ',' :: '.' ::
'\n' :: Nil
'{' :: '}' :: ')' :: '(' :: '[' :: ']' :: '=' :: ' ' :: ',' :: '.' :: '|' ::
'&' :: '\n' :: Nil

def apply(chars: Iterable[Char]): Iterable[Char] = {
var prev: Char = 0
Expand All @@ -53,7 +53,8 @@ object SyntaxHighlighting {

@inline def keywordStart =
prev == 0 || prev == ' ' || prev == '{' || prev == '(' ||
prev == '\n' || prev == '[' || prev == ','
prev == '\n' || prev == '[' || prev == ',' || prev == ':' ||
prev == '|' || prev == '&'

@inline def numberStart(c: Char) =
c.isDigit && (!prev.isLetter || prev == '.' || prev == ' ' || prev == '(' || prev == '\u0000')
Expand Down Expand Up @@ -289,6 +290,23 @@ object SyntaxHighlighting {
case _ => false
}

val valDefStarterTokens = "var" :: "val" :: "def" :: "case" :: Nil

/** lastToken is used to check whether we want to show something
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we shouldn't call it lastToken then since it's not the last token anymore :).

* in valDef color or not. There are only a few cases when lastToken
* should be updated, that way we can avoid stopping coloring too early.
* eg.: case A(x, y, z) => ???
* Without this function only x would be colored.
*/
def updateLastToken(currentToken: String): String =
(lastToken, currentToken) match {
case _ if valDefStarterTokens.contains(currentToken) => currentToken
case (("val" | "var"), "=") => currentToken
case ("case", ("=>" | "class" | "object")) => currentToken
case ("def", _) => currentToken
case _ => lastToken
}

while (remaining.nonEmpty && !delim(curr)) {
curr = takeChar()
if (!delim(curr)) sb += curr
Expand All @@ -298,12 +316,12 @@ object SyntaxHighlighting {
val toAdd =
if (shouldHL(str))
highlight(str)
else if (("var" :: "val" :: "def" :: "case" :: Nil).contains(lastToken))
else if (valDefStarterTokens.contains(lastToken) && !List("=", "=>").contains(str))
valDef(str)
else str
val suffix = if (delim(curr)) s"$curr" else ""
newBuf append (toAdd + suffix)
lastToken = str
lastToken = updateLastToken(str)
prev = curr
}

Expand Down
167 changes: 167 additions & 0 deletions compiler/test/dotty/tools/dotc/printing/SyntaxHighlightingTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
package dotty.tools.dotc.printing

import org.junit.Assert._
import org.junit.Test

/** Adapted from Ammonite HighlightTests
*/
class SyntaxHighlightingTests {
import SyntaxHighlighting._

private def test(source: String, expected: String): Unit = {
val highlighted = SyntaxHighlighting.apply(source)
.mkString
.replace(NoColor, ">")
.replace(CommentColor, "<C|")
.replace(KeywordColor, "<K|")
.replace(ValDefColor, "<V|")
.replace(LiteralColor, "<L|")
.replace(StringColor, "<S|")
.replace(TypeColor, "<T|")
// .replace(AnnotationColor, "<A|") // is the same color as type color

if (expected != highlighted) {
// assertEquals produces weird expected/found message
fail(s"expected: $expected but was: $highlighted")
}
}

@Test
def comments = {
test("//a", "<C|//a>")
test("/** a */", "<C|/** a */>")
test("/* a */", "<C|/* a */>")
}

@Test
def types = {
test("type Foo = Int", "<K|type> <T|Foo> = <T|Int>")
}

@Test
def literals = {
test("1", "<L|1>")
// test("1L", "<L|1L>")
}

@Test
def strings = {
// For some reason we currently use literal color for string
test("\"Hello\"", "<L|\"Hello\">")
}

@Test
def annotations = {
test("@tailrec", "<T|@tailrec>")
}

@Test
def expressions = {
test("val x = 1 + 2 + 3", "<K|val> <V|x> = <L|1> + <L|2> + <L|3>")
}

@Test
def valVarDef = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you split the tests below into smaller tests, that that test only one specific thing. E.g:

@Test
def unionTypes = {
  test("type A = String|Int| Long", "<K|type> <T|A> = <T|String>|<T|Int>| <T|Long>")
  test("type B = String |Int| Long", "<K|type> <T|B> = <T|String> |<T|Int>| <T|Long>")
  test("type C = String | Int | Long", "<K|type> <T|C> = <T|String> | <T|Int> | <T|Long>")
  test("type D = String&Int& Long", "<K|type> <T|D> = <T|String>&<T|Int>& <T|Long>")
  test("type E = String &Int& Long", "<K|type> <T|E> = <T|String> &<T|Int>& <T|Long>")
  test("type F = String & Int & Long", "<K|type> <T|F> = <T|String> & <T|Int> & <T|Long>")
  test("fn[String|Char](input)", "fn[<T|String>|<T|Char>](input)")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Also note that the code snippets don't need to compile. So if for example you want to test syntax highlighting for pattern matching, you don't need to define an ADT for it

val source =
"""
|package test
|
|object A {
| val a = 123
| var b = 123 /*Int*/
| var c = "123" // String
| var d: Int = 123
| var e:Int = 123;e
| e
| print(a)
| 123;123
| def f = 123
| def f1(x: Int) = 123
| def f2[T](x: T) = { 123 }
|}
""".stripMargin
val expected =
"""
|<K|package> test
|
|<K|object> <T|A> {
| <K|val> <V|a> = <L|123>
| <K|var> <V|b> = <L|123> <C|/*Int*/>
| <K|var> <V|c> = <L|"123"> <C|// String
|> <K|var> <V|d>: <T|Int> = <L|123>
| <K|var> <V|e>:<T|Int> = <L|123>;e
| e
| print(a)
| <L|123>;<L|123>
| <K|def> <V|f> = <L|123>
| <K|def> <V|f1>(x: <T|Int>) = <L|123>
| <K|def> <V|f2>[<T|T>](x: <T|T>) = { <L|123> }
|}
""".stripMargin
test(source, expected)
}

@Test
def patternMatching = {
val source =
"""
|val aFruit: Fruit = Apple("red", 123)
|
|val Apple(color, weight) = aFruit
|
|sealed trait Fruit
|case class Apple(color: String, weight: Double) extends Fruit
|case class Orange(weight: Double) extends Fruit
|case class Melon(weight: Double) extends Fruit
|
|aFruit match {
| case Apple(_, weight) => println(s"apple: $weight kgs")
| case o: Orange => println(s"orange ${o.weight} kgs")
| case m @ Melon(weight) => println(s"melon: ${m.weight} kgs")
|}
""".stripMargin
val expected =
"""
|<K|val> <V|aFruit>: <T|Fruit> = <T|Apple>(<L|"red">, <L|123>)
|
|<K|val> <T|Apple>(<V|color>, <V|weight>) = aFruit
|
|<K|sealed> <K|trait> <T|Fruit>
|<K|case> <K|class> <T|Apple>(color: <T|String>, weight: <T|Double>) <K|extends> <T|Fruit>
|<K|case> <K|class> <T|Orange>(weight: <T|Double>) <K|extends> <T|Fruit>
|<K|case> <K|class> <T|Melon>(weight: <T|Double>) <K|extends> <T|Fruit>
|
|aFruit <K|match> {
| <K|case> <T|Apple>(<V|_>, <V|weight>) <T|=>> println(s<L|"apple: <V|$weight <L|kgs">)
| <K|case> <V|o>: <T|Orange> <T|=>> println(s<L|"orange <V|${o.weight}<L| kgs">)
| <K|case> <V|m> @ <T|Melon>(<V|weight>) <T|=>> println(s<L|"melon: <V|${m.weight}<L| kgs">)
|}
""".stripMargin
test(source, expected)
}

@Test
def unionTypes = {
val source =
"""
|type A = String|Int| Long
|type B = String |Int| Long
|type C = String | Int | Long
|type D = String&Int& Long
|type E = String &Int& Long
|type F = String & Int & Long
|fn[String|Char](input)
""".stripMargin
val expected =
"""
|<K|type> <T|A> = <T|String>|<T|Int>| <T|Long>
|<K|type> <T|B> = <T|String> |<T|Int>| <T|Long>
|<K|type> <T|C> = <T|String> | <T|Int> | <T|Long>
|<K|type> <T|D> = <T|String>&<T|Int>& <T|Long>
|<K|type> <T|E> = <T|String> &<T|Int>& <T|Long>
|<K|type> <T|F> = <T|String> & <T|Int> & <T|Long>
|fn[<T|String>|<T|Char>](input)
""".stripMargin
test(source, expected)
}
}

This file was deleted.