Fix wrongly encoded semantic tokens around class keyword

[0dinD]

Fixes #2920

I added some helper functions for making sure that we only scan the gap between the AST nodes (to avoid overlapping/out-of-order tokens). The more general helper functions can be used in the future if we discover more "gaps" in the AST that we need to scan.

[rgrunber]

This definitely fixes the issue. Just some comments.

[rgrunber]

Isn't the -1 mainly because at https://github.com/eclipse-jdtls/eclipse.jdt.ls/pull/2921/files?diff=unified&w=1#diff-3d9e2f2cbb49110b03fcd0c90c05df6fab5f92b5d27d6aab40a074fd6fe79d6aR591-R592 you're passing in the start position of another element as opposed to the correct ending position.

Wouldn't it make more sense to change that line to int gapBeforeNameEnd = typeDeclaration.getName().getStartPosition() - 1; ?

If you agree, then don't forget to update endPosition to mention "(inclusive)" also in javadoc.

[0dinD]

I think this is somewhat subjective. My preference is to use [A, B) intervals (inclusive start, exclusive end), for a number of reasons (nicely summarized here).

you're passing in the start position of another element as opposed to the correct ending position

The start position of one element is (conveniently) the end position of another (adjacent) element in the [A, B) world.

Wouldn't it make more sense to change that line to int gapBeforeNameEnd = typeDeclaration.getName().getStartPosition() - 1; ?

I could, if I decided to live in the [A, B] world, but then I will have to introduce a bunch of +1's and/or -1's everywhere, which I find annoying (as it can lead to off-by-one errors). One of the nice things about the [A, B) world is that start + length = end, as opposed to the [A, B] world where start + length = end - 1. The only time you need to do +1 or -1 in the [A, B) world is when working with an API that doesn't use [A, B) intervals (e.g. IScanner).

I think (without having done much research at all) that [A, B) is very common, probably for the advantages listed above. For example, java.lang.String#substring(int beginIndex, int endIndex) uses [A, B) intervals.

Anyways, these were just my thoughts 🙂. I don't mind changing the code to use [A, B] intervals if you prefer it.

[rgrunber]

Ok, let's leave it as-is then 😜 . It's not that I prefer having the end index be inclusive, but I felt that as tokenizeWithScanner(..) is a helper method for the scanner, it should respect the same convention for those that might be familiar with the API. With that said, the javadocs make it clear it's exclusive.

Maybe it's better that those making the nice contributions decide what convention they prefer :)