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

Completion no longer shows sensible candidates in else branches #1800

Closed
mfussenegger opened this issue Jun 15, 2021 · 14 comments · Fixed by #1803
Closed

Completion no longer shows sensible candidates in else branches #1800

mfussenegger opened this issue Jun 15, 2021 · 14 comments · Fixed by #1803

Comments

@mfussenegger
Copy link
Contributor

mfussenegger commented Jun 15, 2021

On latest master (f4fad03)
Completion candidates in the else branches started to be useless, see this short demo:

recording.mp4

This is in the https://github.com/crate/crate codebase: https://github.com/crate/crate/blob/master/server/src/main/java/io/crate/protocols/postgres/PostgresWireProtocol.java#L645 in case you need something to reproduce. (I haven't yet tried to see if this happens on smaller projects)

If I checkout the commit before 9c41c18 everything is good again.

@fbricon
Copy link
Contributor

fbricon commented Jun 15, 2021

That's bad. @snjeza please investigate

@rgrunber
Copy link
Contributor

rgrunber commented Jun 15, 2021

I can reproduce with Eclipse 2021-06 (RC2) (I20210603-0040) + latest buildship. Looks like an upstream regression sometime between M1 (I20210407-1800) and RC2 M3 (I20210519-1800).

@rgrunber
Copy link
Contributor

rgrunber commented Jun 15, 2021

I'm able to reproduce with a simpler snippet :

Test.java

public class Test {
    private void test() {
        Foo foo;
        Object need;
        if (true) {
        } else {
            foo.
            need = null;
        }
    }
}

Foo.java

public class Foo {
    public void foo1() {}
    public void foo2() {}
    public class Bar {}
}

It looks like if the local identifier (eg. foo) matches (case insensitive) a type (eg. Foo), it is interpreted as that type and only shows type (or static?) completions.

@snjeza
Copy link
Contributor

snjeza commented Jun 15, 2021

The following code doesn't work too:

private void test(String s, int i) {
  if (i > 2) {
  } else {
    s.|
    System.out.println("b");
}

@snjeza
Copy link
Contributor

snjeza commented Jun 15, 2021

I have created https://bugs.eclipse.org/bugs/show_bug.cgi?id=574215

@rgrunber
Copy link
Contributor

The fix for this should be in an I-build from the end of today (17th) at http://download.eclipse.org/eclipse/downloads/ . The next milestone build to include this will be for 4.21 M1 (July 16th).

@fbricon , given that we're close to doing a release, does it make sense to use the current 4.20 Release as the target platform and temporarily copy CompletionEngine locally to include the fix ? Or would you rather immediately have a bug fix release that adopts the new I-build ?

@testforstephen
Copy link
Contributor

Since completion is a high frequency feature, i'd like use I-build to include the fix. Once 4.21 M1 is out, then replace the target platform with it.

@fbricon
Copy link
Contributor

fbricon commented Jun 17, 2021

+1 to releasing from an I-Build, until a more stable version is available

@snjeza
Copy link
Contributor

snjeza commented Jun 17, 2021

We already use 4.21-I-builds/I20210606-1800/ - https://github.com/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.target/org.eclipse.jdt.ls.tp.target#L34
I will update it when a new build is released.

snjeza added a commit to snjeza/eclipse.jdt.ls that referenced this issue Jun 18, 2021
snjeza added a commit that referenced this issue Jun 18, 2021
@fbricon
Copy link
Contributor

fbricon commented Jun 19, 2021

Looks like completion is still broken in the latest master:

public void name(String fooo) {
    if (fooo != null) {
      fo| <-fooo is not proposed here
      System.err.println("Done");
    }
}

@fbricon fbricon reopened this Jun 19, 2021
@rgrunber
Copy link
Contributor

Thanks Fred, for https://bugs.eclipse.org/bugs/show_bug.cgi?id=574338 . Corresponding build should be there by end of today.

rgrunber added a commit to rgrunber/eclipse.jdt.ls that referenced this issue Jun 22, 2021
rgrunber added a commit that referenced this issue Jun 22, 2021
@snjeza snjeza closed this as completed Jun 22, 2021
@niko-liu
Copy link

niko-liu commented Aug 4, 2021

image
bug still exists in M1

@rgrunber
Copy link
Contributor

rgrunber commented Aug 4, 2021

@niko-liu Can you confirm this is still an issue on I20210726-1800 (http://download.eclipse.org/eclipse/downloads/drops4/I20210726-1800/). This seems to be what we're currently using. It could be possible that there's additional cases not being handled correctly.

@niko-liu
Copy link

niko-liu commented Aug 4, 2021

@niko-liu Can you confirm this is still an issue on I20210726-1800 (http://download.eclipse.org/eclipse/downloads/drops4/I20210726-1800/). This seems to be what we're currently using. It could be possible that there's additional cases not being handled correctly.

I20210726-1800 has fixed it ,thx

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

Successfully merging a pull request may close this issue.

6 participants