[JavaScript] Changing declaration keywords from storage to keyword.declaration

[mitranim]

As per #1842, we seem to have the new convention where declaration keywords receive keyword.declaration rather than the longtime-traditional storage.type. JS/TS now uses this for function and =>, but not for the other declaration keywords.

    function X() {}
//  ^^^^^^^^ keyword.declaration.function

    () => {}
//     ^^ keyword.declaration.function.arrow

    var X: T
//  ^^^ storage.type
//         ^ support.class

This affects the following keywords (that I'm aware of):

function X() {}
() => {}
var X
let X
const X
class X
interface X

Even though function class interface are capable of denoting an anonymous construct, all of them qualify as keywords that declare a name. For var let const, this is their only purpose. I believe they should all be keyword.declaration.

import might qualify as well. It's a keyword that introduces 0 to N names into the scope. As a side effect, it also includes an additional module into your tree. This side effect could disqualify it from being keyword.declaration, but I believe it to be a strong contender.

Meanwhile, export might qualify being a storage modifier like public, since it serves the exact same purpose. But perhaps this needs a separate issue.

In addition, I'm surprised by the scoping of arbitrary types in TS as support.class. Shouldn't it be storage.type? Might need a separate issue.

[jwortmann]

Yes, I think those keywords should get keyword.declaration.
The same scope may be possible for import too, but other syntaxes seem to prefer keyword.control.import or keyword.other.import, see #2507. It was pointed out there, that keyword.control probably should be preserved for actual control flow keywords, and I would agree to that. It would be nice to have a common convention for import keywords, while still allowing color schemes to easily distinguish them from preprocessor statements (#include) somehow (#1860).

In addition, I'm surprised by the scoping of arbitrary types in TS as support.class. Shouldn't it be storage.type? Might need a separate issue.

The scope naming guidelines don't make a definite statement about which scope to use for user-defined types/classes. There is

Types should use the following scope. Examples include int, bool and char.

• storage.type

and later also

While also used for base frameworks, many syntaxes apply these to scopes unrecognized classes and types, effectively scoping all user constructs.

• support.type
• support.class

Afaik the support scopes are currently used in e.g. Java and C# for arbitrary (user-defined and built-in) classes, while D uses storage.type and C++ doesn't scope user-defined classes at all. Personally I don't like any of these scopes for user-defined classes very much, because I find it a better editing experience when a built-in type/class (e.g. String or ArrayList in Java) changes its color after typing the last letter, so that I know I haven't made a typo. But perhaps it's unfeasible to specify all possible classes from a standard library in these syntaxes, or sometimes it's a matter of interpretation or would become outdated quickly with language updates. However, especially storage.type should be preserved only for intrinsic type keywords now (often reserved words), like the examples mentioned in the guidelines, in my opinion. This is because many color schemes highlight storage.type the same way or very similar to keyword, since it is or was often used for definition keywords. While I think that user-defined types/classes should get some kind of highlighting color, I'm unsure about a good scope for it which would work well with current color schemes. Seems like there were some suggestions like variable.type, variable.class or variable.other.class for it in #1842, but there hasn't been made a conclusion about it yet.

[Thom1729]

I agree completely. I've written a PR to scope var, let, and const as keyword.declaration, and another that covers class, interface, and namespace.

I'm mostly sold on import as well, and on storage.modifier for export, but I'll punt that to #2507.

Types in TypeScript should probably be variable.type or storage.type. The current scope is inherited from JS Custom's experimental TS extension, where it was copied from the Flow extension, where I probably just copied the scope from babel-sublime.

[deathaxe]

Fixed by #2644 and #2645