| Summary: | [content assist] Wrong completions for parameter on a method chain | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Gayan Perera <gayanper> |
| Component: | Core | Assignee: | Stephan Herrmann <stephan.herrmann> |
| Status: | VERIFIED FIXED | QA Contact: | Jay Arthanareeswaran <jarthana> |
| Severity: | normal | ||
| Priority: | P3 | CC: | loskutov, mauromol, stephan.herrmann |
| Version: | 4.20 | ||
| Target Milestone: | 4.20 RC1 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=573632 https://bugs.eclipse.org/bugs/show_bug.cgi?id=539685 https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180899 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=e211a44735f4142c81518de31a4da690328c1c35 |
||
| Whiteboard: | |||
Gayan, probably this is related to bug 573632. No @Andrey even with the patch for bug 573632 this issue can still be reproduced. Seems something really wrong in completion parser. The CompletionOnMessageSendName looks like this <CompleteOnMessageSendName:CompletioError.getDataProperty().addListener()> And the recieverType is resolved to ObjectProperty which is the reason i get addListener methods as completion. (In reply to Gayan Perera from comment #2) > No @Andrey even with the patch for bug 573632 this issue can still be > reproduced. The patxh is probably not complete yet, but common root cause could be bug 539685 changes. Could you check if the problem disappears if you undo bug 539685 changes? Seems like caused by Bug 539685. Without this fix the completion parser doesn't parse beyond the cursor location. So we endup with a expression like new SingleFireInvalidationListener(<CompleteOnName:dat>); But with the fix it looks like this <CompleteOnMessageSendName:CompletioError.getDataProperty().addListener()>; This happens because it start consuming identifiers beyond the completion point and at the end the parser thinks the first node is the node which belongs the cursor position since the start is always less than cursor position. After Bug 539685 fix i don't get completion at all until the last fix on completion parser. (In reply to Andrey Loskutov from comment #4) > (In reply to Gayan Perera from comment #2) > > No @Andrey even with the patch for bug 573632 this issue can still be > > reproduced. > > The patxh is probably not complete yet, but common root cause could be bug > 539685 changes. Could you check if the problem disappears if you undo bug > 539685 changes? Yes I looked at this and found the following Previously we only scan upto the cusorLocation so we find the "data" as a identifier on the constructor call. But now since we scan the whole method we endup creating expressions for data.values() as a argument for the constructor call. Ideally we should try to see if we are creating expressions which should be part of the completion position and make it a CompletionOn* node instead of a normal AST. (In reply to Gayan Perera from comment #7) > I looked at this and found the following > > Previously we only scan upto the cusorLocation so we find the "data" as a > identifier on the constructor call. But now since we scan the whole method > we endup creating expressions for data.values() as a argument for the > constructor call. Ideally we should try to see if we are creating > expressions which should be part of the completion position and make it a > CompletionOn* node instead of a normal AST. I have a patch coming up in just a few minutes, which extends CompletionParser.getUnspecifiedReference() to do exactly what you describe: create CompletionOn*NameReference nodes, when we cross cursorLocation. (Creating CompletionOnMessageSendName was simply caused by this.assistNode == null, where it should have been set already). New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180899 (In reply to Eclipse Genie from comment #9) > New Gerrit change created: > https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180899 Here you go. @Gayan, your feedback on the patch would be appreciated. @Jay, may I burden you (again) with driving the approval process for RC1? I should note that the gerrit was intentionally created on-top of https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180822/5 For the unlikely case that only this one gets approval, not the other one, the gerrit would need to be rebased on master. Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/180899 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=e211a44735f4142c81518de31a4da690328c1c35 Verified for 4.20 RC1 using build I20210525-1800 |
Take the following code package workbench; import java.util.Collection; import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javafx.beans.InvalidationListener; import javafx.beans.Observable; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; public class App { private static final Logger LOGGER = LoggerFactory.getLogger(App.class); private static ObjectProperty<Map<String, String>> dataProperty = new SimpleObjectProperty<Map<String, String>>(); public static void boo(Map<String, String> data) { LOGGER.debug("PopOver direct buffer delayed patch installation started"); App.getDataProperty().addListener(new SingleFireInvalidationListener(data.values())); LOGGER.debug("PopOver direct buffer delayed patch installation done"); } public static ObjectProperty<Map<String, String>> getDataProperty() { return dataProperty; } private static class SingleFireInvalidationListener implements InvalidationListener { public SingleFireInvalidationListener(Collection<String> list) { } public void invalidated(Observable observable) { } } } Try to invoke completion behind "SingleFireInvalidationListener(data" which will give you wrong completions which are method completions for the Observable class.