Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for new headless quick fixes from upstream #2996

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

gayanper
Copy link
Contributor

@gayanper gayanper commented Dec 9, 2023

Add new quick fixes namely

  • ChangeLambdaBodyToBlock
  • ChangeLambdaBodyToExpression
  • ConvertLambdaToMethodReference
  • AddInferredLambdaParameterTypes
  • AddVarLambdaParameterTypes
  • RemoveVarOrInferredLambdaParameterTypes
  • InvertEqualsExpression

@gayanper
Copy link
Contributor Author

@jjohnstn @rgrunber Please review

@robstryker
Copy link
Contributor

Code looks neat and orderly. I like the addition of tests. The only complaint I have is more of a stylistic complaint, in that I've not seen the jdt-ls project use the 'var' type at all. In fact, the only file I see using it is ProgressReporterManager.

I'm not saying its wrong or needs to be changed. Just pointing out that it doesn't seem to be part of our normal conventions. Still, I'll let @rgrunber decide whether we have "rigorous coding standards" or if we're more of a free-wheeling repo ;)

@rgrunber
Copy link
Contributor

rgrunber commented Dec 12, 2023

There's definitely a push at Eclipse to use newer language features (pretty much anything up to Java 17 is fair game). var is a bit interesting in that it definitely seems to remove information that would have been very clear without even loading the codebase into an IDE, which has implications for doing shallow reviews / instant feedback.

On the other hand, the var usages here are either on the same line, or within a few lines of its initialization, or matching the type of the member that's called statically, so it remains readable, and not really an issue.

@rgrunber rgrunber merged commit 2b5e99a into eclipse-jdtls:master Dec 13, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants