Bugzilla – Full Text Bug Listing
| Summary: | Debug Shell execution fails with Stream.forEach() | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | David Eisner <deisner> | ||||
| Component: | Debug | Assignee: | Sarika Sinha <sarika.sinha> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | gayanper, jasper, jesper, loskutov, sarika.sinha | ||||
| Version: | 4.15 | ||||||
| Target Milestone: | 4.16 M3 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=448473 https://git.eclipse.org/r/161375 https://git.eclipse.org/r/161771 https://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=62bfd38e3c777285d709e2266c6d1fee2badd55d https://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=cf32e05491d7e904dc7656d3442ea829f321e688 |
||||||
| Whiteboard: | 4.16 M2 | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 448473, 562305 | ||||||
| Attachments: |
|
||||||
This is due to a name resolution for sysout. It actually adds wrong qualifiers in the snippet which get executed. Will dig more in the evening unless @Jasper takes this in 🙂 The problem seems to be in org.eclipse.jdt.internal.debug.eval.RemoteEvaluatorBuilder.FunctionalEvalVisitor.visit(QualifiedName)
in following lines which adds the Qualified name
node.getQualifier().accept(this);
buffer.append(".");//$NON-NLS-1$
Because the last line resolution
node.getName().accept(this);
cause the FQN of the "out" field to be resolved as "java.lang.System.out".
Do we really need the
node.getQualifier().accept(this);
buffer.append(".");//$NON-NLS-1$
?
On what scenarios of LambdaExpressions we need it ?
removing the
node.getQualifier().accept(this);
buffer.append(".");//$NON-NLS-1$
actually solves the issue. But i would like to run the tests cases related to this and the reason to have this in the first place.
I think i was wrong, the problem lies in the following method
org.eclipse.jdt.internal.debug.eval.RemoteEvaluatorBuilder.FunctionalEvalVisitor.visit(SimpleName)
ITypeBinding declaringClass = vb.getDeclaringClass();
buffer.append(declaringClass.getName());
buffer.append("."); //$NON-NLS-1$
Why we really need this, removing this solves the issue. @Jasper @Sarika any explanation for this code block ?
With this modification in place, i ran all Java8Tests and all evaluations passes. Hi all please provide me some hints so that i can provide a fix for this issue ? Qualifier or class name might be required for the scoping, we might not have a test case to test if 2 variables with different scope works after the change. If the scope is related to java then we can safely remove this. (In reply to Sarika Sinha from comment #7) > Qualifier or class name might be required for the scoping, we might not have > a test case to test if 2 variables with different scope works after the > change. > > If the scope is related to java then we can safely remove this. Isn’t the scoping is solved by the editor is self ? For example if you want to refer a class variable with the same name you would use the fqn of the field right in the expression? (In reply to Sarika Sinha from comment #7) > Qualifier or class name might be required for the scoping, we might not have > a test case to test if 2 variables with different scope works after the > change. > > If the scope is related to java then we can safely remove this. Can you give a simple example for the scenario you are describing? @Sarika Is it something like this
package workbench;
import java.util.Arrays;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import com.google.common.base.Predicates;
public class ScopeTest {
private Predicate<String> P = Predicates.alwaysTrue();
public static void main(String[] args) {
(new ScopeTest()).exec();
}
private void exec() {
(new Runner()).run();
}
class Runner {
private Predicate<String> P = Predicates.alwaysFalse();
public void run() {
Arrays.asList("a", "b", "c").stream().collect(Collectors.toList());
}
}
}
evaluating
Arrays.asList("a", "b", "ac").stream().filter(ScopeTest.this.P).collect(java.util.stream.Collectors.toList())
and
Arrays.asList("a", "b", "ac").stream().filter(this.P).collect(java.util.stream.Collectors.toList())
?
(In reply to Sarika Sinha from comment #7) > If the scope is related to java then we can safely remove this. What you mean by that ? you mean we should check if we are inspecting variables on a java stackframe and ignore the qualified resolution of the variable ? (In reply to Gayan Perera from comment #11) > (In reply to Sarika Sinha from comment #7) > > If the scope is related to java then we can safely remove this. > > What you mean by that ? you mean we should check if we are inspecting > variables on a java stackframe and ignore the qualified resolution of the > variable ? Yes. Something like or better than this : if (!declaringClass.getQualifiedName().startsWith("java")) { //$NON-NLS-1$ buffer.append(declaringClass.getQualifiedName()); buffer.append("."); //$NON-NLS-1$ }
>
> Yes. Something like or better than this :
> if (!declaringClass.getQualifiedName().startsWith("java")) { //$NON-NLS-1$
> buffer.append(declaringClass.getQualifiedName());
> buffer.append("."); //$NON-NLS-1$ }
Well i think this issue is not related to java.* packages. This problem is due to referring static variables in a class like System.out.foo so this could happen on any class with static variables in between a method invocation.
(In reply to Gayan Perera from comment #13) > > > > Yes. Something like or better than this : > > if (!declaringClass.getQualifiedName().startsWith("java")) { //$NON-NLS-1$ > > buffer.append(declaringClass.getQualifiedName()); > > buffer.append("."); //$NON-NLS-1$ } > > Well i think this issue is not related to java.* packages. This problem is > due to referring static variables in a class like System.out.foo so this > could happen on any class with static variables in between a method > invocation. Give an example for normal static defined variable? sample
package workbench;
import java.util.Arrays;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import com.google.common.base.Predicates;
public class ScopeTest {
public static Predicate<String> P = Predicates.alwaysTrue();
public static void main(String[] args) {
(new ScopeTest()).exec();
}
private void exec() {
(new Runner()).run();
}
class Runner {
public void run() {
Arrays.asList("a", "b", "c").stream().collect(Collectors.toList());
}
}
}
Evaluating
Arrays.asList("a", "b", "ac").stream().filter(v -> ScopeTest.P.test(v)).collect(java.util.stream.Collectors.toList())
With the latest IBuilds now we get the NPE.
I think the NPE is something to do with inner scoping. May be related to https://bugs.eclipse.org/bugs/show_bug.cgi?id=562295. @Sarika do you have a more clear example explaining your scope questions? (In reply to Gayan Perera from comment #10) > @Sarika Is it something like this > > package workbench; > > import java.util.Arrays; > import java.util.function.Predicate; > import java.util.stream.Collectors; > > import com.google.common.base.Predicates; > > public class ScopeTest { > private Predicate<String> P = Predicates.alwaysTrue(); > > public static void main(String[] args) { > (new ScopeTest()).exec(); > } > > private void exec() { > (new Runner()).run(); > } > > class Runner { > private Predicate<String> P = Predicates.alwaysFalse(); > > public void run() { > Arrays.asList("a", "b", "c").stream().collect(Collectors.toList()); > } > } > } > > evaluating works fine DebugShell.test(Arrays.asList("a", "b", "c").stream().count()); public class DebugShell { public static void test(long l) { System.out.println(l); } }
> evaluating works fine
>
> DebugShell.test(Arrays.asList("a", "b", "c").stream().count());
>
> public class DebugShell {
>
> public static void test(long l) {
> System.out.println(l);
> }
>
> }
What you mean @Sarika ?
(In reply to Gayan Perera from comment #18) > > What you mean @Sarika ? I meant that when we use a static method in normal case it is not a problem. will test more to see if other static methods from jars work fine or not. (In reply to Sarika Sinha from comment #19) > (In reply to Gayan Perera from comment #18) > > > > > What you mean @Sarika ? > > I meant that when we use a static method in normal case it is not a problem. > will test more to see if other static methods from jars work fine or not. Does the original code sample from the reporter working for you @Sarika ? Are uou trying with a fix ? (In reply to Gayan Perera from comment #20) > > Does the original code sample from the reporter working for you @Sarika ? > Are uou trying with a fix ? Not yet reached to a fix, trying to understand the scope of the problem first. @Sarika have you been able to reproduce the problem what reporter has reported with System.out.println. This is same if you have a statement like java.util.Collections.EMPTY_LIST.equals as well. @Sarika have you been able to reproduce the problem what reporter has reported with System.out.println. This is same if you have a statement like java.util.Collections.EMPTY_LIST.equals as well. (In reply to Gayan Perera from comment #23) > @Sarika have you been able to reproduce the problem what reporter has > reported with System.out.println. > > This is same if you have a statement like > java.util.Collections.EMPTY_LIST.equals as well. Yes, and suggestion in Comment #12 solves the problem but haven't got the time to test with other jars yet. Can you check what happens if there is a method like java.util.Collections.EMPTY_LIST or System.out.println in a jar, does it work? Isn’t JVM classes qualify for that since they are coming from binary source? Is there a difference? (In reply to Gayan Perera from comment #25) > Isn’t JVM classes qualify for that since they are coming from binary source? > Is there a difference? No difference, I wanted to test how does it behave for other binary classes. If it's only Java related binary class issue, only filtering can work else we need to provide a more generic solution. I don’t think its a binary class issue. Did you try my example with the P predicate variable. It has the same issue. So this is not a binary only or java only problem. I checked with a non JRE class as well since you wanted to check @Sarika, and as i explained the problem is still there. (In reply to Gayan Perera from comment #28) > I checked with a non JRE class as well since you wanted to check @Sarika, > and as i explained the problem is still there. ok, do you have a solution in mind? Suggestion in https://bugs.eclipse.org/bugs/show_bug.cgi?id=562079#c4 I could not find any case, where we will need to add the fully qualified name of the declared class when field is static. So I propose to remove that. If any one has a scenario which fails with this, we can work on it. New Gerrit change created: https://git.eclipse.org/r/161375 Didn't see this before now (horribly swamped with workstuff). There are multiple issues with the debug shell executor, as I recall. Some of the specially-hacked parser stuff is from before generics and has issues. I don't recall the specifics, sorry. Removing the qualified class name and that fixes things, fine by me, but it really should work, so you might just be masking the real issue. @Jasper well by the time we are processing the static field SimpleName we have already resolved the qualified name by that time. I will release this as a work around fix to unlock the users till we find the root cause. @Sarika, I found one reason this qualified name resolution was introduced.
Take the following class
package workbench;
import java.util.Arrays;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import com.google.common.base.Predicates;
public class ScopeTest {
public static Predicate<String> Q = Predicates.alwaysTrue();
public static void main(String[] args) {
(new ScopeTest()).exec();
}
private void exec() {
(new Runner()).run();
}
class Runner {
private Predicate<String> P = Predicates.alwaysFalse();
public void run() {
Arrays.asList("a", "b", "c").stream().collect(Collectors.toList());
}
}
}
If I add a debug point inside the run method and evaluate the following
java.util.Arrays.asList("a", "b", "ac").stream().filter(v -> Q.test(v)).collect(java.util.stream.Collectors.toList())
The Q variable is not resolved with this fix in place. I wonder whether we should allow such evaluation expressions.
But this could fix the issue
if (!(node.getParent() instanceof QualifiedName)) {
ITypeBinding declaringClass = vb.getDeclaringClass();
buffer.append(declaringClass.getQualifiedName());
buffer.append("."); //$NON-NLS-1$
}
What do you think @Sarika?
(In reply to Gayan Perera from comment #37) > But this could fix the issue > > if (!(node.getParent() instanceof QualifiedName)) { > ITypeBinding declaringClass = vb.getDeclaringClass(); > buffer.append(declaringClass.getQualifiedName()); > buffer.append("."); //$NON-NLS-1$ > } > > What do you think @Sarika? I get an NPE as soon as we make Q as static. (In reply to Sarika Sinha from comment #38) > (In reply to Gayan Perera from comment #37) > > But this could fix the issue > > > > if (!(node.getParent() instanceof QualifiedName)) { > > ITypeBinding declaringClass = vb.getDeclaringClass(); > > buffer.append(declaringClass.getQualifiedName()); > > buffer.append("."); //$NON-NLS-1$ > > } > > > > What do you think @Sarika? > > I get an NPE as soon as we make Q as static. @Sarika i think you need to update the jdt debug. The NPE related to Bug562295. (In reply to Gayan Perera from comment #39) > > @Sarika i think you need to update the jdt debug. The NPE related to > Bug562295. Yes, JDT core was not refreshed :) I don't think a check like this will work if (!(node.getParent() instanceof QualifiedName)) { If we add this then if a user uses ScopeTest.Q it will not work. I think the addition of prefix needs to be more smarter to validate if it is already added or is the prefix valid.
>
> I don't think a check like this will work
> if (!(node.getParent() instanceof QualifiedName)) {
>
> If we add this then if a user uses ScopeTest.Q it will not work.
> I think the addition of prefix needs to be more smarter to validate if it is
> already added or is the prefix valid.
Yes you are correct. So you suggest that the evaluation expression should be qualified in this scenario?
Looked back at my workspace, and fix the suggested fix all following evaluations are working for me.
java.util.Arrays.asList("a", "b", "ac").stream().filter(v -> Q.test(v)).collect(java.util.stream.Collectors.toList())
java.util.Arrays.asList("a", "b", "ac").stream().filter(v -> ScopeTest.Q.test(v)).collect(java.util.stream.Collectors.toList())
java.util.Arrays.asList("a", "b", "ac").stream().filter(v -> workbench.ScopeTest.Q.test(v)).collect(java.util.stream.Collectors.toList())
@Sarika can you check and let me know what exactly failing for you ?
(In reply to Gayan Perera from comment #42) > Looked back at my workspace, and fix the suggested fix all following > evaluations are working for me. > > java.util.Arrays.asList("a", "b", "ac").stream().filter(v -> > Q.test(v)).collect(java.util.stream.Collectors.toList()) > > java.util.Arrays.asList("a", "b", "ac").stream().filter(v -> > ScopeTest.Q.test(v)).collect(java.util.stream.Collectors.toList()) > > java.util.Arrays.asList("a", "b", "ac").stream().filter(v -> > workbench.ScopeTest.Q.test(v)).collect(java.util.stream.Collectors.toList()) > > @Sarika can you check and let me know what exactly failing for you ? I think your suggetsion is int he right direction, Will update the patch with a little modified version from your suggestion. @Sarika shall we add some unit tests for this scenario ? If you want i can commit the test that i wrote to reproduce this for debugging. (In reply to Gayan Perera from comment #44) > @Sarika shall we add some unit tests for this scenario ? If you want i can > commit the test that i wrote to reproduce this for debugging. Yes, Please! @Sarika i will commit tonight. Do want me to have it as a separate file or add into existing file ? If later what should be the testcase file ? (In reply to Gayan Perera from comment #46) > @Sarika i will commit tonight. Do want me to have it as a separate file or > add into existing file ? If later what should be the testcase file ? Let's create a new file RemoteEvaluatorTests under org.eclipse.jdt.debug.tests.eval as we need to grow tests in that area. We can add a case for the NPE as well which was observed. New Gerrit change created: https://git.eclipse.org/r/161771 @Sarika added unit tests, I couldn't commit to the same patchset, so I commit as a new one. Gerrit change https://git.eclipse.org/r/161375 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=62bfd38e3c777285d709e2266c6d1fee2badd55d Gerrit change https://git.eclipse.org/r/161771 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=cf32e05491d7e904dc7656d3442ea829f321e688 Wil create new bugs to track further issues. Build id: I20200429-1800 Thanks so much, Gayan and Sarika. I just downloaded 2020-06 and this bug is indeed squashed. I really appreciate your hard work, and the fast turnaround. |
Created attachment 282429 [details] Screenshot showing IDE state and Debug Shell with error messages. When using Debug Shell I can't execute stream pipelines that include Stream.forEach(). I get an "Evaluation failed ... message: java cannot be resolved or is not a field" error. Consider this simple example: ---8<--- package scratch; import java.util.Arrays; import java.util.List; public class DebugShellDemo { public static void main(String... args) { List<String> strs = Arrays.asList("one", "two", "three"); strs.stream().forEach(System.out::println); // breakpoint here } } --->8--- This code runs without error. But if I run it in the debugger and break at the indicated line, and then try to execute that line in the **Debug Shell**, I get this error: ---8<--- strs.stream().forEach(System.out::println); Evaluation failed. Reason(s): [Marker [on: /, id: 269692, type: org.eclipse.jdt.core.transient_problem, attributes: [charEnd: 67, charStart: 63, id: 33554502, lineNumber: 1, message: java cannot be resolved or is not a field, severity: 2, sourceId: JDT], created: 4/13/20, 3:01 PM]] --->8--- I get this error even when not using a method reference: ---8<--- strs.stream().forEach(v -> System.out.println(v)); Evaluation failed. Reason(s): [Marker [on: /, id: 269693, type: org.eclipse.jdt.core.transient_problem, attributes: [charEnd: 72, charStart: 68, id: 33554502, lineNumber: 1, message: java cannot be resolved or is not a field, severity: 2, sourceId: JDT], created: 4/13/20, 3:01 PM]] --->8--- And here's an even simpler example (from Gayan Perara in the forum) demonstrating the problem: ---8<--- Arrays.asList("a", "b", "ac").forEach(v -> System.out.println(v)) Evaluation failed. Reason(s): [Marker [on: /, id: 269694, type: org.eclipse.jdt.core.transient_problem, attributes: [charEnd: 72, charStart: 68, id: 33554502, lineNumber: 1, message: java cannot be resolved or is not a field, severity: 2, sourceId: JDT], created: 4/13/20, 3:01 PM]] --->8--- I'm using Eclipse 2020-03 on macOS 10.15.4, but this also happens in Windows 10.