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

Some parameter names of method invocations are not useful #2412

Closed
Micha3lB opened this issue Apr 14, 2022 · 19 comments · Fixed by #2468
Closed

Some parameter names of method invocations are not useful #2412

Micha3lB opened this issue Apr 14, 2022 · 19 comments · Fixed by #2468
Assignees
Labels

Comments

@Micha3lB
Copy link

Micha3lB commented Apr 14, 2022

Screen Shot 2022-04-14 at 7 56 55 PM

[provide a description of the issue]

I was getting an error saying .java file was not a project file. I looked it up and it said to use Clean java language server, after I did that these were highlighted things like s: and x: are showing up randomly throughout my code. Its not the biggest deal its just distracting. I uninstalled all of the extensions and deleted VScode itself but that didn't work.

Environment

Issue Type: Performance Issue

I was trying to get rid of the error saying .Java is a non-project file and saw something that suggested Clean Java Server Workspace. After selecting that my code has things all over the place such as "x:" in my println, "s:" in my print and my variable names showing up in parameters. I've completly deleted all vs code files but when I downloaded it back it was still here. Please help

VS Code version: Code 1.66.2 (Universal) (dfd34e8260c270da74b5c2d86d61aee4b6d56977, 2022-04-11T07:49:20.994Z)
OS version: Darwin arm64 21.4.0
Restricted Mode: No

System Info
Item Value
CPUs Apple M1 (8 x 24)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled
Load (avg) 2, 2, 2
Memory (System) 16.00GB (1.74GB free)
Process Argv
Screen Reader no
VM 0%
Process Info
CPU %	Mem MB	   PID	Process
   18	   213	  1597	code main
    4	   131	  1598	   gpu-process
    0	    49	  1600	   utility-network-service
    0	   541	  1605	   window (Settings)
    6	   311	  1617	   shared-process
    0	    66	  1620	     ptyHost
    0	     0	  2001	       /bin/zsh -l
    0	     0	  2027	       /bin/zsh -l
    0	    66	  1725	     fileWatcher
    1	     0	  2323	     /bin/ps -ax -o pid=,ppid=,pcpu=,pmem=,command=
    0	   295	  1720	   extensionHost
    0	   737	  1736	     /Users/michaelbauer/.vscode/extensions/redhat.java-1.5.0-darwin-arm64/jre/17.0.2-macosx-aarch64/bin/java --add-modules=ALL-SYSTEM --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/sun.nio.fs=ALL-UNNAMED -Declipse.application=org.eclipse.jdt.ls.core.id1 -Dosgi.bundles.defaultStartLevel=4 -Declipse.product=org.eclipse.jdt.ls.core.product -Djava.import.generatesMetadataFilesAtProjectRoot=false -Dfile.encoding=utf8 -XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true -Xmx1G -Xms100m -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/var/folders/hs/3wwwvyl93psc4vylplm76ms80000gn/T/vscodesws_6224f -jar /Users/michaelbauer/.vscode/extensions/redhat.java-1.5.0-darwin-arm64/server/plugins/org.eclipse.equinox.launcher_1.6.400.v20210924-0641.jar -configuration /Users/michaelbauer/Library/Application Support/Code/User/globalStorage/redhat.java/1.5.0/config_mac -data /var/folders/hs/3wwwvyl93psc4vylplm76ms80000gn/T/vscodesws_6224f/jdt_ws
    0	    98	  1822	     /private/var/folders/hs/3wwwvyl93psc4vylplm76ms80000gn/T/AppTranslocation/A4DD1811-DC25-4106-9B25-09503CC80F28/d/Visual Studio Code.app/Contents/MacOS/Electron --ms-enable-electron-run-as-node /Users/michaelbauer/.vscode/extensions/ms-python.vscode-pylance-2022.4.1/dist/server.bundle.js --cancellationReceive=file:290a59589b007d89f70947144ff4263782cadcfd18 --node-ipc --clientProcessId=1720
    0	   115	  1864	     /private/var/folders/hs/3wwwvyl93psc4vylplm76ms80000gn/T/AppTranslocation/A4DD1811-DC25-4106-9B25-09503CC80F28/d/Visual Studio Code.app/Contents/MacOS/Electron --ms-enable-electron-run-as-node /Users/michaelbauer/.vscode/extensions/streetsidesoftware.code-spell-checker-2.1.11/packages/_server/dist/main.js --node-ipc --clientProcessId=1720
    0	    49	  2022	     /private/var/folders/hs/3wwwvyl93psc4vylplm76ms80000gn/T/AppTranslocation/A4DD1811-DC25-4106-9B25-09503CC80F28/d/Visual Studio Code.app/Contents/MacOS/Electron --ms-enable-electron-run-as-node /private/var/folders/hs/3wwwvyl93psc4vylplm76ms80000gn/T/AppTranslocation/A4DD1811-DC25-4106-9B25-09503CC80F28/d/Visual Studio Code.app/Contents/Resources/app/extensions/json-language-features/server/dist/node/jsonServerMain --node-ipc --clientProcessId=1720
    3	    98	  2316	   issue-reporter
Workspace Info
;
Extensions (15)
Extension Author (truncated) Version
bracket-pair-colorizer-2 Coe 0.2.4
prettier-vscode esb 9.5.0
python ms- 2022.4.1
vscode-pylance ms- 2022.4.1
jupyter ms- 2022.3.1000901801
jupyter-keymap ms- 1.0.0
jupyter-renderers ms- 1.0.6
java red 1.5.0
code-spell-checker str 2.1.11
vscodeintellicode Vis 1.2.19
vscode-java-debug vsc 0.40.0
vscode-java-dependency vsc 0.19.0
vscode-java-pack vsc 0.22.3
vscode-java-test vsc 0.34.2
vscode-maven vsc 0.35.1
A/B Experiments
vsliv368cf:30146710
vsreu685:30147344
python383cf:30185419
vspor879:30202332
vspor708:30202333
vspor363:30204092
vswsl492:30256859
pythonvspyl392:30443607
pythontb:30283811
pythonvspyt551cf:30345471
pythonptprofiler:30281270
vsdfh931cf:30280410
vshan820:30294714
vstes263:30335439
vscoreces:30445986
pythondataviewer:30285071
vscod805cf:30301675
pythonvspyt200:30340761
binariesv615:30325510
bridge0708:30335490
bridge0723:30353136
vsaa593:30376534
vsc1dst:30438360
pythonvs932:30410667
wslgetstarted:30449410
pythonvsnew555:30457759
vscscmwlcmt:30465135

Steps To Reproduce
  1. [step 1]
  2. [step 2]

[Please attach a sample project reproducing the error]
Please attach logs

Current Result

System.out.println(x:"random thing");

Expected Result

System.out.println("random thing");

Additional Informations

You can't delete the things either. Im sorry if I messed anything up this is my first time filing a report.

@jdneo
Copy link
Collaborator

jdneo commented Apr 15, 2022

It's a new feature introduced in 1.5.0, called inlay hint. We use that to display the parameter names of those arguments.

If you do not want to have them in your editor, you can set the setting java.inlayHints.parameterNames.enabled to none

@Eskibear
Copy link
Contributor

There are also inlay hints in Line 6 ~ Line 10, but user is not complaining about it, because they do provide useful information.

Just some thoughts, shall we maintain a list and hardcode to exclude system.out.print/println? as inlay hints for these functions don't provide much help.

@jdneo
Copy link
Collaborator

jdneo commented Apr 15, 2022

Just some thoughts, shall we maintain a list and hardcode to exclude system.out.print/println? as inlay hints for these functions don't provide much help.

Sure, an exclude list can be added for such case, which is also the way that IntelliJ and Eclipse do.

@gruvw
Copy link

gruvw commented Apr 15, 2022

It would be great to also add a right-click context menu to ignore some of the methods we wrote.

@Micha3lB
Copy link
Author

Thank you :)

@rgrunber
Copy link
Member

Would it be safe to assume that a 1 or 2 character length parameter hint is useless and just hide those ? I don't think Eclipse has an exclude list. I checked the preferences and only saw the ability to enable/disable whether parameter names are shown. They do ignore methods with only 1 parameter though.

@rgrunber rgrunber changed the title Random things such as "s:", "x:", and name of parameters are showing up inside of my print statements. Some parameter names of method invocations are not useful Apr 19, 2022
@rgrunber rgrunber added the bug label Apr 19, 2022
@jdneo
Copy link
Collaborator

jdneo commented Apr 20, 2022

The Eclipse embedded code miningonly contains basic support, so it does not have the exclude list. But it can be available once the jdt code mining plugin is installed.

image

The full list: https://github.com/angelozerr/jdt-codemining/blob/master/org.eclipse.jdt.codemining/src/org/eclipse/jdt/experimental/ui/javaeditor/codemining/methods/DefaultMethodFilters.txt

@gruvw
Copy link

gruvw commented Apr 20, 2022

I also think that we should not display parameters that only have 1 or 2 characters, and I think we should not display an inlay hint when there is only one parameter (at least have a setting for the latter).

@jdneo
Copy link
Collaborator

jdneo commented Apr 20, 2022

I think we should not display an inlay hint when there is only one parameter (at least have a setting for the latter)

Looks good to me to have a setting for that. Or maybe just hide it and wait for users' feedback.

we should not display parameters that only have 1 or 2 characters

It makes sense to me to hide parameters only have 1 char. While I'm not sure about 2 characters. Since there are still some words with 2 characters which have a clear meaning. For example: id, db, ms, ...

@rgrunber
Copy link
Member

So ideally :

  1. Hide inlay hints for method invocations with only 1 parameter
  2. Hide inlay hints for for parameters of method invocations that are only 1 char
  3. Adopt the filtering syntax from above for further customization

We can definitely adopt (1) & (2) with no settings and see later if users want further options. (3) would be nice if possible, especially given the code is mostly written.

@jdneo
Copy link
Collaborator

jdneo commented Apr 21, 2022

After discussing with the team, we think (3) should be the final goal we would like to achieve. The reason is that we still can figure out some cases that option (1) and (2) cannot satisfied.

option (1)

Think about the constructor of ArrayList: public ArrayList(int initialCapacity).

Here though the invocation has only one parameter but it's still helpful to tell user that the integer denotes for the capacity.

option (2)

Imagine that for a GUI application, it may have some APIs like this: draw(double x, double y).

Here though x and y have only one character. But it's useful to let user know which is the x-axis and which is y-axis.

@rgrunber
Copy link
Member

rgrunber commented Apr 22, 2022

I guess with (1), part of my assumption is that any ambiguity in what the parameter represents would be cleared up by the name of the method. I'm open to removing this restriction, though even the example, being a constructor, for a class that stores multiple elements is a hint at some form of size. Also this case doesn't suffer from the issue of having to guess the order of parameters, which is really where this feature starts to shine.

Agreed that showing the counter-example for (2) is a benefit. The method name, parameter types, and implicit assumption about ordering (x before y) does help a bit.

Like you said, I think (3) will ultimately solve our problems.

@jdneo
Copy link
Collaborator

jdneo commented May 5, 2022

I'll try to support the exclusion list for the inlay hint in this month

@gruvw
Copy link

gruvw commented May 23, 2022

I also found another interesting example: camel case and snake case should be considered as matching (suppressWhenArgumentMatchesName).
Even further, we should completely ignore case of the parameter: hello_world = HELLO_WORLD = helloWorld.

@rgrunber
Copy link
Member

rgrunber commented May 27, 2022

It looks like the feature by default is empty, so going forward we may want to collect a list of values to exclude. Maybe even just use the DefaultMethodFilters.txt from above.

@pnulty
Copy link

pnulty commented Sep 22, 2022

Just a comment on this issue as a new user: hiding short parameter hints makes sense, but the real problem that I had (and that the creator of this issue had) is that to a new user it might be completely unclear as to what these annotations are. Although they are used in several IDEs, if you haven't come across them before they are confusing. If it's possible to have some kind of hovertext or right click information just to say "this is an inlay hint", that would be enough information for a user to find out what they are and disable them if wanted.

@rgrunber
Copy link
Member

I just tried this modification and it works :

diff --git a/src/inlayHintsProvider.ts b/src/inlayHintsProvider.ts
index e409343..43c7c09 100644
--- a/src/inlayHintsProvider.ts
+++ b/src/inlayHintsProvider.ts
@@ -91,5 +91,6 @@ function asInlayHint(value: LSInlayHint, client: LanguageClient): InlayHint {
     const result = new InlayHint(client.protocol2CodeConverter.asPosition(value.position), label);
     result.paddingRight = true;
     result.kind = InlayHintKind.Parameter;
+    result.tooltip = 'This is an inlay hint. This is not a drill.';
     return result;
 }

We could also do it properly on the JDT-LS side with the inlay hints proposed API. and it'll work. Supported by https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#inlayHint . @jdneo , do you really see a sizeable amount of users confused by inlay hints currently ?

@jdneo
Copy link
Collaborator

jdneo commented Sep 23, 2022

I didn't see many feedbacks about inlay hint recently. Let's track it as a feature request first: #2691

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants