Java 10+ Local-Variable Type Inference

[MBoegers]

Implement a recipe for local variable type inference as discussed here: #217

This Draft contains the test suite, as tim suggested.

[timtebeek]

Might want to explicitly state what we want to do with primitives through tests; that could help indicate what we end up doing is intentional; great start!

Perhaps as a suggestion: right now your tests are a mix of should & should not apply recipe. Could be helpful to separate those into nested classes each such that you can drop the prefix.

[MBoegers]

Yes agree primitives are missing, see my comment here.

You suggest grouping the test by applicability instead, currently they are grouped as in the issue.
I can live with both ;)

[timtebeek]

Yes agree primitives are missing, see my comment here.

👍🏻

You suggest grouping the test by applicability instead, currently they are grouped as in the issue. I can live with both ;)

Hah; we can do both with Nested within Nested. Especially helpful if there's a lot of cases to visually separate, which it appears we might end up doing here: both covering cases where we do want to make changes and exclusions where we do not want to make changes. And then logically group those based on their applicability to primitives, generics, etc.. at the top level.

[MBoegers]

GH and I had issues the last days.. But now I updated the Test as we discussed

[timtebeek]

Thanks a lot @MBoegers ; I'm asking my colleagues to have a look! :)

[MBoegers]

@timtebeek cheers! I have not yet implemented generics, because I felt it really hard to detect them securely. I'd like to get a review for the simple parts and aim for generics a bit later ;)

[knutwannheden]

I think the PR looks really good! Some small comments:

• I found a problem with compound definitions, which currently would do the wrong thing.
• The null-type checking can also be improved
• I also noticed that it would currently not apply to code inside static and non-static initializers
• It might be cool to also support the variables of enhanced for-loops (e.g. for (var s : List.of("a", "b")))

[knutwannheden]

@MBoegers Thanks for looking into this! Regarding your question:

Would I be an acceptable way to implement tests as marker and postpone the implementation to a followup?

Definitely! The recipe doesn't need to be perfect from day one. We do however have the do no harm guideline which boils down to that a recipe should rather not make any change than a wrong change. This can in itself already by quite a challenge (and we certainly still have some recipes which violate this rule), but we should make a best effort to not produce any wrong code. So if we can fix the few cases I think I found, I think we can go ahead and integrate this recipe and do improvements in follow-up commits.

[MBoegers]

I've updated the PR, as you may see I excluded some features for now. They should be a new issue, otherwise this PRs lifetime explodes :D

What is possible by now:

• direct assignments and reassignments for primitive and boxed primitive
• inside static and instance initializer blocks as well as methods
• skipping of var usage, fields definitions, method parameters, multiple variable declarations and null assignments

The leftovers:

• Generics and Method invocation in general, I think we should spend some more time defining what should work for generics and especially what not
• Ternary operator, similar to generics, this needs some more spec work
• Enhanced for loop, is tightly tied to generics
• Splitting the recipe into small recipes, handling each sub feature. Like Methods, Primitives, Ternary and the easy direct assignments

Should I write some issues like I have them in mind for further refinement?

[MBoegers]

I've extracted to open work for ternary operator support to #237 and support for method invocation to #236.
Further, I refactored the recipe into two separate recipes and a combining yml recipe and rebases.
I need a review to aim for merging

[knutwannheden]

Looks very good. It will be interesting to see what turns up when we run this recipe in the SaaS.

[knutwannheden]

@MBoegers Looks like you just committed some changes. Some of my comments may not apply anymore...

[knutwannheden]

Very nice! @timtebeek Anything else you want to add or should we merge it?

[knutwannheden]

@MBoegers Thanks again for your contribution!

[MBoegers]

Thanks to you both for your help

[timtebeek]

Yes thanks a lot! I know it was a long one to get through, with a lot of discovery along the way, but really appreciate the effort!

[knutwannheden]

@MBoegers FYI: I integrated two small fixes for the recipe: ff4d3dc and 688ce47