New representation for scope handler tests

[pokey]

The problem

We need a better way to represent our test cases for scope handlers, because today we end up needing to record a bunch of commands to test edge cases, and the tests aren't easy to read. In addition, once #1523 is merged, we expect that most scope type development will be done by viewing the output of the scope visualizer, so we'd like test cases that mirror the scope visualizer as closely as possible

The solution

We'd like an ascii representation that is readable by humans and parsable by machine. The idea is as follows:

• We intersperse comment lines among the source code that point out regions in the line above
• The comment lines begin with the language's comment token (eg //), followed by !
• Each source code line is indented by extra whitespace at the beginning to allow the comment lines to be able to point at the start of the line above if necessary. If we didn't do this, then a source code line could begin at the same spot as the // on the line below, so we couldn't point out the beginning of the line.
• The content range is marked by {^^^}
• The domain is marked by [---]
• The removal range is marked by (xxx)
• The iteration scope is marked by <***>
• We need to figure out interior / leadingDelimiter / trailingDelimiter
  • Would like to avoid unicode if possible
  • Here are suggestions from chatGPT:
    • The interior is marked by /###\
    • The leadingDelimiter is marked by «@@@»
    • The trailingDelimiter is marked by §$$$¶
• If any range is equal to contentRange, it does not get a line, as that is the implicit assumption
• In each of the above, the surrounding brackets are outside of the range, so that we can handle empty ranges by using eg {}
• Each range type (eg content, domain, removal, etc) gets its own comment line
• If there are overlapping ranges, the range with later start or earlier end gets its own set of lines after the other scope, beginning with //!2 (or //!3, etc). Note that ranges are overlapping even if they are 1 character apart, because otherwise there's no room for the brackets
• The start of the file will contain metadata lines to indicate scope type being tested and language id, eg

   //!! scope: {type: namedFunction}
   //!! languageId: typescript, plaintext

• Note that multiple examples of the same scope type could be present in one file, so that we don't need a million test files like we have today
• Note that we don't handle indicating isPreferredOver; need special tests for that

Components

• Automatically generate test case from source code. Can probably steal some code from Scope visualizer #1523
• Parse output to run as a test. Before running the scope handler, it should remove all the special comment lines and dedent every line by the correct amount. Note that it should be able to only look at the delimiters (eg {}), as the internal markers (eg ^^^) are just for readability

Considerations

• Automatically test scope handler edge cases #2064

Examples

value

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
//!                    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx 
//!  ----------------------------------------------------------------------------

name

//!! scopeType: {type: name}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!        ^^^^^^^^^^^^                                                          
//!  ----------------------------------------------------------------------------

statement

//!! scopeType: {type: statement}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

namedFunction

//!! scopeType: {type: namedFunction}
//!! languageId: typescript

      function aaa() {
//!1 {^^^^^^^^^^^^^^^^
        function bbb() {
//!2   {^^^^^^^^^^^^^^^^
          const ccc = "ddd";
        }
//!2  ^^^}
      }
//!1  ^}

collectionKey

//!! scopeType: {type: collectionKey}
//!! languageId: typescript

     const aaa = {
       bbb: {
//!1   ^^^   
//!1  [------
         ccc: "ddd",
//!2     ^^^        
//!2    [-----------]
       },
//!1   --]
     }

token

#!! scopeType: {type: token}
#!! languageId: plaintext

     foo.bar.baz
#!1  ^^^        
#!2     ^       
#!3      ^^^    
#!4         ^   
#!5          ^^^

Here are some alternative representations to consider:

Merge ranges onto one line

value

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  ------------------xxx^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-

name

//!! scopeType: {type: name}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  ------^^^^^^^^^^^^----------------------------------------------------------

statement

//!! scopeType: {type: statement}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

namedFunction

//!! scopeType: {type: namedFunction}
//!! languageId: typescript

      function aaa() {
//!1 {^^^^^^^^^^^^^^^^
        function bbb() {
//!2   {^^^^^^^^^^^^^^^^
          const ccc = "ddd";
        }
//!2  ^^^}
      }
//!1  ^}

collectionKey

//!! scopeType: {type: collectionKey}
//!! languageId: typescript

     const aaa = {
       bbb: {
//!1   ^^^ 
//!1  [------
         ccc: "ddd",
//!2     ^^^--------
       },
//!1   --]
     }

token

#!! scopeType: {type: token}
#!! languageId: plaintext

     foo.bar.baz
#!1  ^^^        
#!2     ^       
#!3      ^^^    
#!4         ^   
#!5          ^^^

Always surrounding brackets

value

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!                      {^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^}
//!                   (xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx)
//! [----------------------------------------------------------------------------]

name

//!! scopeType: {type: name}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!       {^^^^^^^^^^^^}
//! [----------------------------------------------------------------------------]

statement

//!! scopeType: {type: statement}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//! {^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^}

namedFunction

//!! scopeType: {type: namedFunction}
//!! languageId: typescript

      function aaa() {
//!  {^^^^^^^^^^^^^^^^
        function bbb() {
//!2   {^^^^^^^^^^^^^^^^
          const ccc = "ddd";
        }
//!2  ^^^}
      }
//!   ^}

collectionKey

//!! scopeType: {type: collectionKey}
//!! languageId: typescript

     const aaa = {
       bbb: {
//!   {^^^}
//!   [------
         ccc: "ddd",
//!2    {^^^}
//!2    [-----------]
       },
//!    --]
     }

token

#!! scopeType: {type: token}
#!! languageId: plaintext

     foo.bar.baz
#!  {^^^}  {^}
#!2    {^}  {^^^}
#!3     {^^^}

Internal markers on every line of range

value, name, statement, and token

(as above)

namedFunction

//!! scopeType: {type: namedFunction}
//!! languageId: typescript

      function aaa() {
//!  {^^^^^^^^^^^^^^^^
        function bbb() {
//!   ^^^^^^^^^^^^^^^^^^
//!2   {^^^^^^^^^^^^^^^^
          const ccc = "ddd";
//!   ^^^^^^^^^^^^^^^^^^^^^^
//!2  ^^^^^^^^^^^^^^^^^^^^^^
        }
//!   ^^^
//!2  ^^^}
      }
//!   ^}

collectionKey

//!! scopeType: {type: collectionKey}
//!! languageId: typescript

     const aaa = {
       bbb: {
//!   {^^^}
//!   [------
         ccc: "ddd",
//!    -------------
//!2    {^^^}
//!2    [-----------]
       },
//!    --]
     }

No internal markers

value

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!                      {                                                      }
//! [                                                                            ]
//!                   (                                                         )

name

//!! scopeType: {type: name}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!       {            }
//! [                                                                            ]

statement

//!! scopeType: {type: statement}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//! {                                                                            }

namedFunction

//!! scopeType: {type: namedFunction}
//!! languageId: typescript

      function aaa() {
//!  {
        function bbb() {
//!2   {
          const ccc = "ddd";
        }
//!2     }
      }
//!    }

collectionKey

//!! scopeType: {type: collectionKey}
//!! languageId: typescript

     const aaa = {
       bbb: {
//!   {   }
//!   [
         ccc: "ddd",
//!2    {   }
//!2    [           ]
       },
//!      ]
     }

token

#!! scopeType: {type: token}
#!! languageId: plaintext

     foo.bar.baz
#!  {   }  { }
#!2    { }  {   }
#!3     {   }

Using `.` to indicate whitespace

value

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  ....................{......................................................}
//! [............................................................................]
//!  .................(.........................................................)

name

//!! scopeType: {type: name}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  .....{............}.........................................................
//! [............................................................................]

statement

//!! scopeType: {type: statement}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//! {............................................................................}

namedFunction

//!! scopeType: {type: namedFunction}
//!! languageId: typescript

      function aaa() {
//!  {................
        function bbb() {
//!2  .{................
          const ccc = "ddd";
        }
//!2  ...}
      }
//!   .}

collectionKey

//!! scopeType: {type: collectionKey}
//!! languageId: typescript

     const aaa = {
       bbb: {
//!  .{...}..
//!  .[......
         ccc: "ddd",
//!2 ...{...}.......
//!2 ...[...........]
       },
//!  ....]
     }

token

#!! scopeType: {type: token}
#!! languageId: plaintext

     foo.bar.baz
#!  {...}..{.}..
#!2  ..{.}..{...}
#!3  ...{...}...

Using `.` only where necessary

value

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  .                 . {.                                                    .}
//! [.                 .                                                       ..]
//!                   (.                                                       .)

name

//!! scopeType: {type: name}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  .    {.          .}                                                        .
//! [.                                                                          .]

statement

//!! scopeType: {type: statement}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//! {.                                                                          .}

namedFunction

//!! scopeType: {type: namedFunction}
//!! languageId: typescript

      function aaa() {
//!  {.
        function bbb() {
//!2   {.
          const ccc = "ddd";
        }
//!2    .}
      }
//!   .}

collectionKey

//!! scopeType: {type: collectionKey}
//!! languageId: typescript

     const aaa = {
       bbb: {
//!   {. .}
//!   [.
         ccc: "ddd",
//!2    {. .}      .
//!2    [.         .]
       },
//!     .]
     }

token

#!! scopeType: {type: token}
#!! languageId: plaintext

     foo.bar.baz
#!  {. .}. {.}
#!2    {.} .{. .}
#!3     {. .}

Using markers only where necessary

value

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  .                 . {^                                                    ^}
//! [-                 .                                                       .-]
//!                   (x                                                       x)

name

//!! scopeType: {type: name}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!  .    {^          ^}                                                        .
//! [-                                                                          -]

statement

//!! scopeType: {type: statement}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//! {^                                                                          ^}

namedFunction

//!! scopeType: {type: namedFunction}
//!! languageId: typescript

      function aaa() {
//!  {^
        function bbb() {
//!2   {^
          const ccc = "ddd";
        }
//!2    ^}
      }
//!   ^}

collectionKey

//!! scopeType: {type: collectionKey}
//!! languageId: typescript

     const aaa = {
       bbb: {
//!   {^ ^}
//!   [-
         ccc: "ddd",
//!2    {^ ^}      .
//!2    [-         -]
       },
//!     -]
     }

token

#!! scopeType: {type: token}
#!! languageId: plaintext

     foo.bar.baz
#!  {^ ^}. {^} .
#!2    {^} .{^ ^}
#!3     {^ ^}

[pokey]

A few questions:

• should we put leading / trailing delimiter on same line as content range? They never overlap
• should we show removal range if it's just equal to trailing ?? leading?
• should we show leading / trailing / removal when it's just token delimiter ranges (ie whitespace)? On the one hand we could do that to write nice test cases for token removal, but on the other hand I worry it will lead to lots of noise for scopes that just get token removal by default, esp if removing them doesn't make much semantic sense. But for those where we actually want those token removal semantics it might be nice to have that tested 🤔. Maybe tree-sitter scopes could opt in / out of token removal behaviour?

cc/ @AndreasArvidsson @phillco

[AndreasArvidsson]

1. Please make an example of that I think it sounds reasonable
2. I believe so. It's not always obvious if it's going to be trailing or leading for a particular case and we might change that algorithm in the future.
3. I guess we could start with them and if it becomes too noisy we could remove them in the future?

[pokey]

1. Please make an example of that I think it sounds reasonable

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!                    xxx^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//!  ----------------------------------------------------------------------------

2. I believe so. It's not always obvious if it's going to be trailing or leading for a particular case and we might change that algorithm in the future.

Mixed feelings. It will be extra noise I think, and while I do agree it can be unintuitive when you're just using Cursorless, in this case the leading / trailing are marked quite clearly visually. But easiest to just show them for now, and then see how annoying it is. If it feels like noise, I think just having a check if

removalRange.isRangeEqual(
  contentRange.union(trailingDelimiter ?? leadingDelimiter ?? contentRange)
);

is easy to do, and should be fairly intuitive

3. I guess we could start with them and if it becomes too noisy we could remove them in the future?

Sounds good

[AndreasArvidsson]

1. That looks like removal range and content range on the same line. I still feel that removal range probably should be its own line.
   Something like this:

//!! scopeType: {type: value}
//!! languageId: typescript

     const removalRange = getRelatedRange(match, scopeTypeType, "removal", true);
//!                    @@@^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//!                    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
//!  ----------------------------------------------------------------------------

[pokey]

Yeah fair. I'm happy with that

[pokey]

I wonder if we should support a compact version of this syntax inside comments in our query definitions 🤔. Eg:

(
  [
    ;;!! const foo = "bar";
    ;;!        ^^^
    ;;!  ------------------
    ;;!! let foo = "bar";
    ;;!      ^^^
    ;;!  ----------------
    (lexical_declaration
      (variable_declarator
          name: (_) @name
      )
    )

    ;;!! var foo = "bar";
    ;;!      ^^^
    ;;!  --------------
    (variable_declaration
      (variable_declarator
          name: (_) @name
      )
    )
  ] @_.domain
  (#not-parent-type? @_.domain export_statement)

  ;; Handle multiple variable declarators in one statement, eg
  ;;!! let aaa = "bbb", ccc = "ddd";
  ;;!      ^^^          ^^^
  ;;!  -----------------------------
  (#allow-multiple! @name)
)

where comments beginning with ;;!! are lines of code, and beginning with ;;! indicate a marker line. We could parse these out and run them as tests on all languages that import the given .scm file. One issue is how to indicate which scope type it is testing. I want the syntax to be quite lightweight, eg as close as possible to the type of comment we try to use above a pattern to indicate its purpose anyway. If there's only one scope type mentioned in a given pattern, we can infer they're referring to that one, but I guess need a way to disambiguate if it refers to multiple. Maybe that's not the common case so having an extra line to disambiguate when that happens is reasonable 🤔

not sure we'd want all our tests to be inline like this, as it could get pretty verbose, eg for that final one we'd want to test const and var as well, but that would make the definition file pretty long

edit: I made a draft of using these style comments in #1535. I didn't go all the way in that PR, because I wanted to preserve some of the convenient notations like (foo | bar) / [baz] / ...

edit: Maybe we could support indicating the scope type with something like the following?

    ;;!! function foo() {}
    ;;!  ^^^^^^^^^^^^^^^^^ namedFunction
    ;;!  ---------^^^----- functionName
    (function_declaration
      name: (_) @functionName
    ) @namedFunction @functionName.domain

edit: It would be cool if there were some way to check that the match for that test case was actually coming from that pattern 🤔

[josharian]

Any thoughts about what to do about tabs that are part of ranges? There's no right answer about how many dashes/whatever to use under them. (Can discuss over voice if the answer is long.)

[pokey]

Yes tabs continue to be the bane of our existence in cursorless. See eg

• Slice fails sometimes if using tabs.  #1586
• Scope visualizer: handle tab characters (\t) #1741

I have no idea how to handle them tbh 😅. For some situations we could possibly read the tab width setting, as suggested here, but I don't think that helps us in this situation, as different users might have different settings, and we're baking in one setting to the recorded test.

My instinct is to avoid tabs wherever possible in our tests, but that will potentially cause skew from real usage in the case of Go, where I'm guessing most users will be using tabs due to gofmt

Very much open to ideas here

[josharian]

The only "works always" fix that I am aware of is to fight tabs with tabs: allows tabs anywhere in ranges as a no-op, and put them below any real tabs so that the alignment is always preserved. But that creates all sorts of knock-on problems.

Let's kick around a few ideas over voice sometime.

[josharian]

lolsob https://stackoverflow.com/questions/68333368/tab-stops-are-messed-up-after-rarer-unicode-characters

[josharian]

terrible idea: replace tabs with:

U+2409 ␉ SYMBOL FOR HORIZONTAL TABULATION

[josharian]

not that it helps us, but elastic tab stops are nice:

https://nick-gravgaard.com/elastic-tabstops/
https://pkg.go.dev/text/tabwriter

[josharian]

Another annoying question. (Sorry, thinking through all this stuff a bit.) How will this representation represent comment scopes? The scope system will pick up the annotations as comments themselves...

[josharian]

FWIW, I think the "fixed but inconvenient length" problem (how do you keep symbol-by-symbol alignment in the face of grapheme clusters) may be easier: use the correct number of zero-width spaces to preserve alignment.

And yes, there's a opposite type of this category of problem, which is zero-width spaces in content. Maybe we can assume nobody uses those. 😅

[pokey]

Another annoying question. (Sorry, thinking through all this stuff a bit.) How will this representation represent comment scopes? The scope system will pick up the annotations as comments themselves...

That's what the ! is for in //!

[AndreasArvidsson]

I have an idea for a really simple format to test scopes

• Folder name denotes language. If all tests in this folders should be the same language there's really no point duplicating the language as a field in the fixture.
• Fixture file name contains facet id from: Language support tables #2010
• Scope type is derived from facet id
• Fixture file only needs example code and the range representations described in this issue

example
typescript/ternary.condition.txt

;;!! true ? 0 : 1
;;!  ^^^^
;;!  ------------

An alternative format could be the code follow by a delimiter and then separate ranges. C=Content range, D=Domain etc...

true ? 0 : 1
---

  true ? 0 : 1
C ^^^^

  true ? 0 : 1
D ^^^^^^^^^^^^

Or even more verbose

true ? 0 : 1
---

[Content]
true ? 0 : 1
^^^^

[Domain]
true ? 0 : 1
^^^^^^^^^^^^

Personally I'm quite partial to the last one. If the file only tests a single facet I see no need for them to be so terse. I also like that the indentation is the actual indentation from the code and there are no prefixes on the line.

[AndreasArvidsson]

Me and @josharian just had discussion about my post above. We are both in agreement that the third and more verbose format is the way to go. We came up with a few thoughts.

• Inside the language folder we can support sub folders with arbitrary names. You might want to group function facets for example.
• Only one range per header. If we need multiple content ranges we would have multiple content headers. This means that there is never range interleaving and there is no need to denote the start or end of a range.
• The only part that's actually gonna be parsed/read from these files are the code at the top and to the delimiter. Therefore we can have a custom format for the rest of the file if we like. If we in the future need a parsable format we could switch to something like txtars