Refactoring/code cleanup

[sebthom]

To work on #816 I imported the lsp4j project into my Eclipse workspace and was greeted with a lot of compiler and style warnings. With this PR I am addressing a few of the issues. Each type of change is encapsulated in a separate commit for easier review.

If you disagree with some changes I can remove the respective commits.

[pisv]

@sebthom Thanks a lot for putting the effort into cleaning up and modernizing the code base, looks really impressive! 👍

There are a few minor formatting-related remarks, and I'm not a big fan of the last commit that introduces vars, but that's just my personal opinion. Otherwise, LGTM.

BTW, it was a pleasure to review this PR. Thanks again!

[jonahgraham]

I'm fine with this - but was confused because I didn't see these warnings, until I realized that the projects were defaulting to workspace settings and my workspace and yours must be different.

Can you please consider adding a commit where you save the compiler warnings/etc and code style setting to the project?

Without your change, in my workspace the only warnings I see are deprecated warnings.

[pisv]

Without your change, in my workspace the only warnings I see are deprecated warnings.

Same here.

BTW, there is also @SuppressWarnings("all") on the interface Tuple, which seems completely unnecessary and has been suppressing the warnings about unused imports in that file.

[sebthom]

@pisv

I'm not a big fan of the last commit that introduces vars, but that's just my personal opinion

I am aware that this is a sensitive topic :-) On the one hand I like code that is explicit on the other hand I like code that is terse. To get the best of both worlds I therefore only use the var keyword when the actual type is still visible on the right side of the assignment either via an explicit cast var foo = (SomeType) obj or if it is a constructor call var foo = new SomeType(). In those cases the providing the type on the left side is really redundant. However I am not a fan of relying on type interference in other cases like var foo = someService.someMethod().get(2).

[pisv]

@sebthom Regarding introducing vars into the code base, your approach certainly makes sense to me, thanks for the detailed description.

It is just that I have grown very much accustomed to always seeing the type on the left side of the variable declaration in the good old Java days :-) So, it is just my personal preference, and I understand that I'm quite biased towards it. Besides, I'm currently spending much more time with TypeScript, and slowly getting the type inference thing :-)

Therefore, I'd like to leave this decision to others. Since @jonahgraham as the project lead apparently has no objections, I think we are good to go with it 👍

[jonahgraham]

Since @jonahgraham as the project lead apparently has no objections

No objections :-)

FWIW your journey sounds just like mine re vars and living in typescript land more and more.

[sebthom]

Can you please consider adding a commit where you save the compiler warnings/etc and code style setting to the project?

@jonahgraham I added my Eclipse compiler settings to the build.gradle file. If you regenerate the eclipse settings using gradle eclipse all projects will be adjusted accordingly. Feel free to change the settings to your liking.

[jonahgraham]

LGTM - lets start with the setting as you have them. If we run into problems we can change later.

At some point we should do the same for formatting and other style issues such as the fixed trailing whitespace. (I'm not going to get around to that, but welcome anyone else who wants to put in the effort)

@pisv Please let me know if you approve before I submit.

@sebthom to make your life easier, would you like to see this PR or #816 merged first?

[pisv]

Looks great to me 👍 Wholeheartedly approve, and much appreciate this PR.

[pisv]

FWIW your journey sounds just like mine re vars and living in typescript land more and more.

After having lived in the Eclipse IDE land for almost 20 years, I started contributing to Eclipse Theia about a year ago :-)