Add support for Kotlin.

[wolfmanstout]

What

Adds support for the kotlin programming language

Checklist

• Recorded tests for the new language
• Used "change" / "clear" instead of "take" for selection tests to make recorded tests easier to read
• Added a few specific tests that use "chuck" instead of "change" to test removal behaviour when it's interesting, especially:
  • "chuck arg" with single argument in list
  • "chuck arg" with multiple arguments in list
  • "chuck item" with single argument in list
  • "chuck item" with multiple arguments in list
• Added @textFragment captures. Usually you want to put these on comment and string nodes. This enables "take round" to work within comments and strings.
• Added a test for "change round" inside a string, eg "hello (there)"
• Supported "type" both for type annotations (eg foo: string) and declarations (eg interface Foo {}) (and added tests for this behaviour 😊)
• Supported "item" both for map pairs and list entries (with tests of course)

[wolfmanstout]

Creating a draft pull request as suggested. Please let me know if you have any input on how I'm doing things so far. I'm following the same sequence as java.scm where applicable, although it's not a perfect 1:1 mapping. The tree-sitter-kotlin implementation could definitely be improved, but I'm hoping to avoid changes there, at least for this first PR.

Kotlin does not have any list or map literals, or even array literals. Everything is just a function call, and the closest thing to a map literal is use of the "to" infix operator, like so:

val map = mapOf(1 to "x", 2 to "y", -1 to "zz")
println(map.get(1)) // {1=x, 2=y, -1=zz}

Hence, I guess I don't need to explicitly define item but I should just test that it works with args? I could also assume that pairs constructed with that syntax are [key] to [value] pairs.

Also, should I be supporting and testing @private.switchStatementSubject? I guess I'll need to manually bind this to a scope name.

[wolfmanstout]

I think this is now ready for review. Some open questions:

• I left one item on the checklist unchecked, because the example given doesn't actually seem to align with conventions in either Java or Python: "take type" on a class (or interface) doesn't take the class name. There are many places in Kotlin where a type can appear, and I've covered the vast majority of them (but not this case, to be consistent).
• Thus far I added support for @private.switchStatementSubject but I haven't tested it. Let me know if I should simply drop support or bind this and add a test.

[wolfmanstout]

I can file a bug for this.

[wolfmanstout]

Filed: #2447

[wolfmanstout]

If I could just do (_)? @type.end I wouldn't need this redundant query, but Cursorless complains when it's missing because I refer to it in a predicate below. I think Cursorless could safely ignore constraints when they don't appear in cases such as this where they are tied to an optional match. My reasoning is that if the predicate must be confirmed, then it shouldn't be an optional match. Should I file a bug for this? There are several places throughout this file where I could simplify my code if this were possible.

[wolfmanstout]

Also, I noticed that "drink/pour arg" don't automatically work to add commas. How can I add support for these?

[wolfmanstout]

Filed bug for this: fwcd/tree-sitter-kotlin#127

[AndreasArvidsson]

Also, I noticed that "drink/pour arg" don't automatically work to add commas. How can I add support for these?

Here is the implementation for javascript you can have a look at

cursorless/queries/javascript.core.scm

Lines 692 to 718 in 089ba0d

	;;!! foo(name) {}
	;;! ^^^^
	(
	(formal_parameters
	(_)? @_.leading.endOf
	.
	(_) @argumentOrParameter
	.
	(_)? @_.trailing.startOf
	) @_dummy
	(#not-type? @argumentOrParameter "comment")
	(#single-or-multi-line-delimiter! @argumentOrParameter @_dummy ", " ",\n")
	)
	;;!! foo("bar")
	;;! ^^^^^
	(
	(arguments
	(_)? @_.leading.endOf
	.
	(_) @argumentOrParameter
	.
	(_)? @_.trailing.startOf
	) @_dummy
	(#not-type? @argumentOrParameter "comment")
	(#single-or-multi-line-delimiter! @argumentOrParameter @_dummy ", " ",\n")
	)

[wolfmanstout]

Thanks @AndreasArvidsson, I added argument delimiter support.

[pokey]

Ok before you get too much further, it's worth having a look at our updated docs for adding a new language. In particular, we have a new way of writing tests, and we now advocate for many small PRs rather than one big PR.

Given that I added those new docs after you had already opened this PR, I won't make it a hard requirement for merging, but going forward, let's

a) not add any more scopes to this PR, and
b) use the new test format for subsequent PRs

We're planning to add a migration assistant for migrating tests to the new format (see #2390), so I don't think it's worth migrating these manually

I'll have a look at this PR in the next couple of days, but figured I'd give you a chance to look at those docs in case there's anything you want to tweak before I have a look

[wolfmanstout]

Thanks! I don't think I will have time to make any changes to this pull request within the next couple days, so you should feel free to review.

Unfortunately it's a little late to stop adding more scopes because all the scopes have been added 😂. I'm happy to add the metadata files before submitting that describe what scopes are covered.

[pokey]

Thanks! I don't think I will have time to make any changes to this pull request within the next couple days, so you should feel free to review.

Sounds good; will do 😊

Unfortunately it's a little late to stop adding more scopes because all the scopes have been added 😂

Ha no worries

I'm happy to add the metadata files before submitting that describe what scopes are covered.

You'd need to migrate your recorded tests to new scope tests as well, because we have checks to ensure that all scope facets marked as supported actually have tests. So if you're not planning to manually migrate those tests (which I'm not sure I'd recommend), I'd leave out the support tables for now

[wolfmanstout]

Okay I won't plan to add the tables then. You can just let me know in the review if there are any other small things I should add as part of this change.

[AndreasArvidsson]

There are so many test and I'm not a kotlin developer so I most likely have missed something, but I think this looks fine. If you're happy with it then go ahead and merge this so you can start using it. Anything you find we could fix in a follow up.

[jaresty]

I'm excited to try this one out! Thanks for putting it together.

[wolfmanstout]

Fantastic I'll merge it! I've been using it plenty and the only issues I've seen are upstream in the tree sitter grammar.

[wolfmanstout]

Ah I don't have merge permissions so you'll need to do that part.

[AndreasArvidsson]

Okay here we go! :)

[pokey]

Fantastic I'll merge it! I've been using it plenty and the only issues I've seen are upstream in the tree sitter grammar.

@wolfmanstout def worth filing those upstream and also keeping a tracker issue here. We can often work around oddnesses / bugs with upstream tree-sitter grammars using tricks in our patterns

[wolfmanstout]

Fantastic I'll merge it! I've been using it plenty and the only issues I've seen are upstream in the tree sitter grammar.

@wolfmanstout def worth filing those upstream and also keeping a tracker issue here. We can often work around oddnesses / bugs with upstream tree-sitter grammars using tricks in our patterns

Yep, filed upstream and even sent out PR fixes. I worked around a lot of shortcomings here but these would be extremely awkward to work around and probably introduce other issues. I can create a tracker issue too (a single one for these issues I suppose?).

[pokey]

Fantastic I'll merge it! I've been using it plenty and the only issues I've seen are upstream in the tree sitter grammar.

@wolfmanstout def worth filing those upstream and also keeping a tracker issue here. We can often work around oddnesses / bugs with upstream tree-sitter grammars using tricks in our patterns

Yep, filed upstream and even sent out PR fixes. I worked around a lot of shortcomings here but these would be extremely awkward to work around and probably introduce other issues. I can create a tracker issue too (a single one for these issues I suppose?).

Yeah just a single tracker issue would be awesome 🙏