Adding inline type hint for 'var' variables

[jlahoda]

Adding inline type hint for 'var' variables:

---

^Add meaningful description above

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

• are all your own work, and you have the right to contribute them.
• are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

[lkishalmi]

Cool!

[mbien]

code looks good - very compact for what it does which is great. There is probably going to be a need to toggle those inline features individually? Or is all-or-nothing ok for users which use it?

standard rant for features which modify the text:

not a fan of IDEs which display information inline which is not in the code, since it influences how code is written/formatted and makes it somewhat IDE dependent. Also worth noting that simply declaring the type takes less space than using var + inline hints in most cases (e.g in the example given above).
Removing information just to add it right back a few chars to the right via an IDE assisted feature is a bit weird to me :)
I don't think enabling it is a good default since it is too invasive (but the decision has been already made).

[neilcsmith-net]

-0 on this for me. I agree with @mbien here -

not a fan of IDEs which display information inline which is not in the code, since it influences how code is written/formatted and makes it somewhat IDE dependent. Also worth noting that simply declaring the type takes less space than using var + inline hints in most cases (e.g in the example given above).
Removing information just to add it right back a few chars to the right via an IDE assisted feature is a bit weird to me :)

In general inline hints serve a purpose, but I've definitely noticed their influence on writing / formatting - I think how we display the data might need some rethinking.

Showing parameter names and other information that cannot (easily) be in the source makes more sense than showing the type information for var which could be declared by choice.

[mbien]

just a thought:

If the IDE is already changing the text, why not put the type where it belongs? A direct way of helping with readability of code which overuses var could be to replace var with the actual type and draw it in a different color. Could be a toggle button somewhere or hotkey. It would have to switch back to var if the text cursor is over it which complicates things unfortunately I have to admit.

This wouldn't change the syntax of the language and wouldn't incentivize overuse of var / change of formatting / create IDE dependence.

Maybe there is a way to solve those use cases differently so that NB doesn't have to enable inline hints by default (even though the feature itself is probably required anyway since some users might want to have feature parity with other IDEs)

[jlahoda]

Sorry for belated reply. A few comments:
-I suspect for a long time that we will need a way to customize which inline hints are shown and which are not. Whether the number of hints right now requires that is a question.
-using an explicit type for a var instead of an inline hint works only in code where one can write the explicit type. It does not work for reading existing code written by someone else, for example.
-replacing var with an explicit type - even if we found a way to replace the views, it would still change the formatting, correct? For example, if we replace var with Map<? extends String, ? extends String>, the length of the text will be different?

[matthiasblaesing]

The code looks right. The discussion whether or not the var construct should be used and types should be shown is IMHO independent from this implementation. I personally don't user inline hints, BUT if I had to read code using var extensively, it could be helpful to activate inline hints temporary. It is IMHO in line with the other inline hints.

[mbien]

-replacing var with an explicit type - even if we found a way to replace the views, it would still change the formatting, correct? For example, if we replace var with Map<? extends String, ? extends String>, the length of the text will be different?

it would change the formatting too. The differences are in the other aspects as e.g. it doesn't incentivize overuse of var. Replacing var with the type via a toggle would purely work as reading aid, while inline hints change the syntax of the language and change how code is written / incentivize overuse.

If var is used correctly, there is no reading aid needed, since:

• it is used in places where the type truly doesn't matter to reduce ceremony as the JEP states
• it can be used where the type is obvious or if its defined somewhere else
• (..)

e.g to use a Map of Maps with verbose generic types (maybe even with upper or lower bounds) you can simply use var and extract the elements later with the concrete type. Generic declarations can quickly get out of hand - var helps here a lot.

while looking at:
Path root = manager.getRoot();

the jep would suggest to use a descriptive name when using var, since the type could be anything here:
var rootPath = manager.getRoot();

with hints this starting to get verbose again:
var rootPath : Path = manager.getRoot();

and a dev might ignore the advice and switch back to a short name since the IDE is providing this info anyway:
var root : Path = manager.getRoot();

first of all: this isn't java. Second: the initial version is the most compact and the easiest to read since var is used here in a completely unnecessary situation.
Path root = manager.getRoot();

Thats why I spoke out for not enabling it by default (in #4358), and maybe trying to find a better way of providing reading aid for code which overuses var, esp in a way which doesn't influence how the code is written / incentivize overuse.

E.g. having a toggle which replaces var with the type would not influence how devs write code - it would be a pure reading aid devs could enable it when needed.

[matthiasblaesing]

I think this moves from a technical review to a philosophical question. I don't think the IDE should make decisions what is "a right way" to code. If a team/person decides, to use var without thinking about it and relies on its IDE to read it, it is its decision.

So from my POV it comes down to these questions:

1. Is the code correct?
2. Is the introduced behavior consistent with the current behavior?

We might indeed better configurability for the feature (for example to disable var type display and enable parameter name hints), but that is a different discussion.

[mbien]

I think this moves from a technical review to a philosophical question. I don't think the IDE should make decisions what is "a right way" to code. If a team/person decides, to use var without thinking about it and relies on its IDE to read it, it is its decision.

Yes! If the IDE shouldn't make the decision we should keep it disabled by default (it is enabled since NB 15). I approved this PR too - from a technical perspective I have no complains.

"Good defaults" are very important, esp for NetBeans which has traditionally been an IDE which needed only a few clicks before it was ready to go. Having this fairly intrusive hint enabled by default, does already make a decision for the user how var should be used. To not make this decision ahead of time, we should reconsider having this enabled by default at the very least. (and maybe explore better ways how to aid with var-readability problems if there is demand)

if var is used to reduce "ceremony" (again taking the words from the jep), having this inline hint enabled would make it an anti-feature. Since it displays the type in the exact situations where you truly don't care about the type or want to hide it due to verbosity. While at the same time incentivizing the use in trivial situations, like the getRoot() example I gave above where var + inline hint is more verbose than java 8.

The language feature was certainly not implemented with the intention that the IDEs would one day add the type right next to the keyword again (by default) - this negates it's purpose - its there to hide the type.

If a team wants to use it that way, fine, but I don't think this is a good default that is all I am trying to say :)

[neilcsmith-net]

"Good defaults" are very important,

+1! And why I disagree with @matthiasblaesing that this is "philosophical" and doesn't belong in the review. Where else do we review usability?!

using an explicit type for a var instead of an inline hint works only in code where one can write the explicit type. It does not work for reading existing code written by someone else, for example.

Firstly there may be a point to having different inline hint defaults in editable vs non-editable code. Not that that covers all bases.

Secondly, could this particular hint be moved to the end of the line? Similar to #2365 (and the still open #2785). Direct use of var doesn't seem that important compared to assignment by method call? ie. hints to the return type from methods in assignments (where the assignment is var or an explicit supertype) might be more useful? And putting it to the right hand side covers some of my concerns about how these influence formatting and written code.

[jtulach]

This is what I have been desperately waiting for!

[ratcashdev]

var is making me nuts, especially, if it's not in front of a new keyword and constructor call, where the type is clearly given, but in front of a method call: var x = callMethod(someArgs);

The team I work in, decided to make extensive use of var, regardless of the context. As a result, I have to move my cursor to the method and hit CTRL+SPACE. That will show the type, but i will still have to go to the method (CTRL+Click) to be able to navigate into the class of the type that's returned (to look up some implementation details).

The pragmatic objective should be the ease of reading and writing code and making the type always apparent, but not overly verbose. Therefore:

As I see it:

1. inline hint: GOOD
2. mousover-hint of the type when on top of var: GOOD.
3. navigable type hint (i.e. CTRL+CLICK on the inline hint or var directly, taking me to the class): GREAT
4. Propose to leave var as it is, and instead add type hints for method return types when that type is not apparent: FANTASTIC

In practice, the type hint would be shown only if:

• the call itself is not a constructor call
• the type is defined using var and not using an explicit type

it would look like: var x = [String] callMethod(someArgs); or String x = callMethod(someArgs); (the latter without the type hint).
And if I may ask, please make that type hint CTRL+clickable.

[Chris2011]

@jlahoda I can remember your implementation for virtual-text-prepend: #1247 (comment) so I also want to add some hints to other files than Java and I was searching not for prepending but for appending text, what you already add but I'm missing the point of finding the keyword virtual-text-append. All of your implementation (showing types of chaining collections and var implementation) looks like some hardcoded just for those things without having such a keyword. Is this correct? Shouldn't we add this also as a keyword (virtual-text-append) to make this more consistent with the other implementation to can be reused in other contexted.

I know cases where we can have virtual-text-prepend, virtual-text-append and virtual-text-above.

[matthiasblaesing]

Merged via #5013