Consistent highlight on derives' qualIds #240

tanishiking · 2022-08-04T06:30:53Z

vscode-scala-syntax couldn't provide consistent highlights for
C and D in derives C, D.

Problem

Consider we have the following code

enum A extends B derives C, D

Previously,
B and C have the following TextMate Scopes

entity.other.inherited-class.scala
source.scala

while D has

entity.name.class
source.scala

because it had accepted only one qualId after derived

vscode-scala-syntax/src/typescript/Scala.tmLanguage.ts

Line 1156 in 0b2b1e8

    
           match: `\\b(extends|with|derives)\\b\\s*(${idUpper}|${backQuotedId}|(?=\\([^\\)]+=>)|(?=${plainid})|(?="))?`,

even though Scala3 syntax allows us to put multiple qualIds.

Therefore, the first one will have entity.other.inherited-class.scala, but the following qualIds won't be named as
entity.other.inherited-class.scala. Instead, falllback to entity.name.class based on it's name (upper camel).

fix

This commit makes it provides consistent highlight for each qualId after derives keyword.
Now, both C and D will have entity.name.class instead of entity.other.inherited-class.scala.

alternative solution

It would be ideal to put entity.other.inherited-class.scala to both C and D,
but it looks like impossible with TextMate grammar.
Because according to the syntax
(see the syntax spec for Scala3 here https://docs.scala-lang.org/scala3/reference/syntax.html)
we can't tell where's the end of derives clause.

In the complete program, we can tell { or : is the end of derives
clause, but it messes up if the program is incomplete like

enum A extends B derives C, D

where there's no : or {.

So, just give up providing entity.other.inherited-class.scala and put entity.name.class based on their name (upper camel).

tanishiking · 2022-08-04T06:32:12Z

@Meowcolm024 What do you think about this? you can check it by

check out this branch tanishiking:derives
yarn install, yarn build
code --install-extension scala-0.5.6.vsix
restart vscode

fix: scala#239 vscode-scala-syntax couldn't provide consistent highlights for `C` and `D` in `derives C, D`. Consider we have the following code ```scala enum A extends B derives C, D ``` Previously, `B` and `C` have the following TextMate Scopes ``` entity.other.inherited-class.scala source.scala ``` while `D` has ``` entity.name.class source.scala ``` because it had accepted only one qualId after `derived` https://github.com/scala/vscode-scala-syntax/blob/0b2b1e8829254f0d190e723d3a49874cb7d135c0/src/typescript/Scala.tmLanguage.ts#L1156 even though Scala3 syntax allows us to put multiple qualIds. Therefore, the first one will have `entity.other.inherited-class.scala`, but the following qualIds won't be named as `entity.other.inherited-class.scala`. Instead, falllback to `entity.name.class` based on it's name (upper camel). This commit makes it provide consistent highlight for each qualId after `derives` keyword. Now, both `C` and `D` will have `entity.name.class` instead of `entity.other.inherited-class.scala`. It would be ideal to put `entity.other.inherited-class.scala` to both `C` and `D`, but it looks like impossible with TextMate grammer. Because, according to the [syntax](https://dotty.epfl.ch/docs/reference/contextual/derivation.html#syntax) (see the syntax spec for Scala3 here https://docs.scala-lang.org/scala3/reference/syntax.html) we can't tell where's the end of `derives` clause. In the complete program, we can tell `{` or `:` is the end of `derives` clause, but it messes up if the program is incomplete like ```scala enum A extends B derives C, D ``` where there's no `:` or `{`. Just give up providing `entity.other.inherited-class.scala` and put `entity.name.class` based on their name (upper camel).

tgodzik

Thanks for looking into this! 🎉

tgodzik · 2022-08-04T09:22:18Z

tests/snap/scala_spec.test.scala

+  case B, c, D
+
+enum A extends B derives X, Y, Z


Suggested change

enum A extends B derives X, Y, Z

enum A extends B, C derives X, Y, Z

wouldn't we have the same problem with extends?

tgodzik · 2022-08-04T09:43:25Z

src/typescript/Scala.tmLanguage.ts

@@ -1153,7 +1156,7 @@ export const scalaTmLanguage: TmLanguage = {
    inheritance: {
      patterns: [
        {
-          match: `\\b(extends|with|derives)\\b\\s*(${idUpper}|${backQuotedId}|(?=\\([^\\)]+=>)|(?=${plainid})|(?="))?`,
+          match: `\\b(extends|with)\\b\\s*(${idUpper}|${backQuotedId}|(?=\\([^\\)]+=>)|(?=${plainid})|(?="))?`,


Suggested change

match: `\\b(extends|with)\\b\\s*(${idUpper}|${backQuotedId}|(?=\$[^\$]+=>)|(?=${plainid})|(?="))?`,

match: `\\b(extends|with|derives|,)\\b\\s*(${idUpper}|${backQuotedId}|(?=\$[^\$]+=>)|(?=${plainid})|(?="))?`,

something like this wouldn't work?

I don't think so because it's too generic, that rule accepts any identifiers separated by , as entity.other.inherited-class.scala. For example A , B, B will be entity.other.inherited-class.scala though it's not.

It would be great if we can apply such rules only in extends and derives clause by

patterns: [ { start: `\\b(extends|derives)`, end: ???, patterns: [ // patterns that matches `qualId`s and assign name as `entity.other.inherited-class.scala` ] } ]

However, the problem is we can't tell what is the end of extends or derives.
(At first, I thought maybe we can end with : or { or <newline> but it doesn't work if derives clause contains new line)

class A derives B, C

With new line that should be ok since we ended with , we still expect something to be after it, no?

tanishiking · 2022-08-09T08:22:53Z

Decided to assign entity.name.class to A, B in extends A, B, maybe we should do the same thing for with A (even though it doesn't accept with A, B) for consistency 🤔

tgodzik · 2022-08-09T09:44:40Z

src/typescript/Scala.tmLanguage.ts

@@ -1165,6 +1168,18 @@ export const scalaTmLanguage: TmLanguage = {
        }
      ]
    },
+    derives: {


Derives is a soft keyword, wouldn't it be an issue when someone uses it as an identifier?

Yeah, it would be a problem.

derives, if it appears after an extension clause or after the name and possibly parameters of a class, trait, object, or enum definition.
https://docs.scala-lang.org/scala3/reference/soft-modifier.html

However, I don't think there's a way for regex to check the previous clause is "extension clause or after the name and possibly parameters of a class, trait, object, or enum definition."
(for using, we could determine it's after ( by positive-lookahead though).

tanishiking · 2022-09-16T13:18:35Z

Closing for #242

tanishiking force-pushed the derives branch from 95e4628 to 0e50367 Compare August 4, 2022 06:35

tgodzik reviewed Aug 4, 2022

View reviewed changes

assign entity.name.class for A, B in extends A, B

6cf16c6

tgodzik reviewed Aug 9, 2022

View reviewed changes

tanishiking closed this Sep 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Consistent highlight on derives' qualIds #240

Consistent highlight on derives' qualIds #240

Uh oh!

tanishiking commented Aug 4, 2022

Uh oh!

tanishiking commented Aug 4, 2022

Uh oh!

tgodzik left a comment

Uh oh!

tgodzik Aug 4, 2022

Uh oh!

tanishiking Aug 9, 2022

Uh oh!

tgodzik Aug 4, 2022

Uh oh!

tanishiking Aug 9, 2022

Uh oh!

tgodzik Aug 9, 2022

Uh oh!

tanishiking commented Aug 9, 2022

Uh oh!

tgodzik Aug 9, 2022

Uh oh!

tanishiking Sep 7, 2022

Uh oh!

tanishiking commented Sep 16, 2022

Uh oh!

Uh oh!

	enum A extends B derives X, Y, Z
	enum A extends B, C derives X, Y, Z

	match: `\\b(extends\|with)\\b\\s*(${idUpper}\|${backQuotedId}\|(?=\\([^\\)]+=>)\|(?=${plainid})\|(?="))?`,
	match: `\\b(extends\|with\|derives\|,)\\b\\s*(${idUpper}\|${backQuotedId}\|(?=\\([^\\)]+=>)\|(?=${plainid})\|(?="))?`,

Consistent highlight on derives' qualIds #240

Consistent highlight on derives' qualIds #240

Uh oh!

Conversation

tanishiking commented Aug 4, 2022

Problem

fix

alternative solution

Uh oh!

tanishiking commented Aug 4, 2022

Uh oh!

tgodzik left a comment

Choose a reason for hiding this comment

Uh oh!

tgodzik Aug 4, 2022

Choose a reason for hiding this comment

Uh oh!

tanishiking Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

tgodzik Aug 4, 2022

Choose a reason for hiding this comment

Uh oh!

tanishiking Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

tgodzik Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

tanishiking commented Aug 9, 2022

Uh oh!

tgodzik Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

tanishiking Sep 7, 2022

Choose a reason for hiding this comment

Uh oh!

tanishiking commented Sep 16, 2022

Uh oh!

Uh oh!