Bug 562295 - [1.8] NPE when trying to inspect evaluation expression
Summary: [1.8] NPE when trying to inspect evaluation expression
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 4.15   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.16 M3   Edit
Assignee: Gayan Perera CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 448473 560626
  Show dependency tree
 
Reported: 2020-04-19 14:45 EDT by Gayan Perera CLA
Modified: 2020-05-21 08:03 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gayan Perera CLA 2020-04-19 14:45:20 EDT
Sample code :

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 {
		private Predicate<String> P = Predicates.alwaysFalse();

		public void run() {
			Arrays.asList("a", "b", "c").stream().collect(Collectors.toList());
		}
	}
}

Try to evaluate the following expression in DebugShell
Arrays.asList("a", "b", "ac").stream().filter(v -> P.test(v)).collect(java.util.stream.Collectors.toList())

Error:
java.lang.NullPointerException
	at sun.reflect.UnsafeFieldAccessorImpl.ensureObj(UnsafeFieldAccessorImpl.java:57)
	at sun.reflect.UnsafeObjectFieldAccessorImpl.get(UnsafeObjectFieldAccessorImpl.java:36)
	at java.lang.reflect.Field.get(Field.java:393)
	at workbench.CodeSnippet_1.lambda$0(CodeSnippet_1.java:10)
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
	at java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:566)
	at workbench.ScopeTest$Runner.run(ScopeTest.java:24)
	at workbench.ScopeTest.exec(ScopeTest.java:17)
	at workbench.ScopeTest.main(ScopeTest.java:13)
Comment 1 Simeon Andreev CLA 2020-04-20 03:32:18 EDT
What is "workbench.CodeSnippet_1.lambda$0(CodeSnippet_1.java:10)" from the stack trace?
Comment 2 Gayan Perera CLA 2020-04-20 03:51:06 EDT
Its the generated snippet by jdt. Generated by RemoteEvaluator i think ?
Comment 3 Sarika Sinha CLA 2020-04-20 04:02:39 EDT
	

Thread [ModalContext] (Suspended (exception NullPointerException))	
	owns: Object  (id=157)	
	CodeSnippetScope.findFieldForCodeSnippet(TypeBinding, char[], InvocationSite) line: 284	
	CodeSnippetScope.getFieldForCodeSnippet(TypeBinding, char[], InvocationSite) line: 588	
	CodeSnippetQualifiedNameReference.getOtherFieldBindings(BlockScope) line: 522	
	CodeSnippetQualifiedNameReference(QualifiedNameReference).resolveType(BlockScope) line: 1111	
	CodeSnippetMessageSend.resolveType(BlockScope) line: 211	
	ReturnStatement.resolve(BlockScope) line: 328	
	LambdaExpression.resolveType(BlockScope, boolean) line: 454	
	LambdaExpression(FunctionalExpression).resolveType(BlockScope) line: 188	
	CastExpression.resolveType(BlockScope) line: 621	
	CodeSnippetReturnStatement.resolve(BlockScope) line: 130	
	Block.resolveUsing(BlockScope) line: 144	
	TryStatement.resolve(BlockScope) line: 1196	
	MethodDeclaration(AbstractMethodDeclaration).resolveStatements() line: 649	
	MethodDeclaration.resolveStatements() line: 359	
	MethodDeclaration(AbstractMethodDeclaration).resolve(ClassScope) line: 558	
	CodeSnippetTypeDeclaration(TypeDeclaration).resolve() line: 1309	
	CodeSnippetTypeDeclaration(TypeDeclaration).resolve(CompilationUnitScope) line: 1434	
	CompilationUnitDeclaration.resolve() line: 667	
	CodeSnippetCompiler(Compiler).process(CompilationUnitDeclaration, int) line: 901	
	CodeSnippetCompiler(Compiler).processCompiledUnits(int, boolean) line: 575	
	CodeSnippetCompiler(Compiler).compile(ICompilationUnit[], boolean) line: 475	
	CodeSnippetCompiler(Compiler).compile(ICompilationUnit[]) line: 426	
	CodeSnippetEvaluator(Evaluator).getClasses() line: 135	
	EvaluationContext.evaluate(char[], char[][], char[][], int[], char[], boolean, boolean, INameEnvironment, Map<String,String>, IRequestor, IProblemFactory) line: 305	
	EvaluationContextWrapper.evaluateCodeSnippet(String, String[], String[], int[], IType, boolean, boolean, ICodeSnippetRequestor, IProgressMonitor) line: 233	
	RemoteEvaluatorBuilder.build() line: 116	
	ASTInstructionCompiler.visit(LambdaExpression) line: 2941	
	LambdaExpression.accept0(ASTVisitor) line: 195	
	LambdaExpression(ASTNode).accept(ASTVisitor) line: 2971	
	ASTInstructionCompiler.pushMethodArguments(IMethodBinding, List<Expression>) line: 3142	
	ASTInstructionCompiler.visit(MethodInvocation) line: 3075	
	MethodInvocation.accept0(ASTVisitor) line: 220	
	MethodInvocation(ASTNode).accept(ASTVisitor) line: 2971	
	ReturnStatement(ASTNode).acceptChild(ASTVisitor, ASTNode) line: 3019	
	ReturnStatement.accept0(ASTVisitor) line: 128	
	ReturnStatement(ASTNode).accept(ASTVisitor) line: 2971	
	Block(ASTNode).acceptChildren(ASTVisitor, ASTNode$NodeList) line: 3042	
	Block.accept0(ASTVisitor) line: 128	
	Block(ASTNode).accept(ASTVisitor) line: 2971	
	MethodDeclaration(ASTNode).acceptChild(ASTVisitor, ASTNode) line: 3019	
	MethodDeclaration.accept0(ASTVisitor) line: 698	
	MethodDeclaration(ASTNode).accept(ASTVisitor) line: 2971	
	TypeDeclaration(ASTNode).acceptChildren(ASTVisitor, ASTNode$NodeList) line: 3042	
	TypeDeclaration.accept0(ASTVisitor) line: 447	
	TypeDeclaration(ASTNode).accept(ASTVisitor) line: 2971	
	TypeDeclaration(ASTNode).acceptChildren(ASTVisitor, ASTNode$NodeList) line: 3042	
	TypeDeclaration.accept0(ASTVisitor) line: 447	
	TypeDeclaration(ASTNode).accept(ASTVisitor) line: 2971	
	CompilationUnit(ASTNode).acceptChildren(ASTVisitor, ASTNode$NodeList) line: 3042	
	CompilationUnit.accept0(ASTVisitor) line: 258	
	CompilationUnit(ASTNode).accept(ASTVisitor) line: 2971	
	ASTEvaluationEngine.createExpressionFromAST(String, EvaluationSourceGenerator, CompilationUnit) line: 643	
	ASTEvaluationEngine.getCompiledExpression(String, IJavaStackFrame) line: 392	
	ASTEvaluationEngine.evaluate(String, IJavaStackFrame, IEvaluationListener, int, boolean) line: 148	
	EvaluateAction$1.run(IProgressMonitor) line: 261	
	ModalContext$ModalContextThread.run() line: 122
Comment 4 Gayan Perera CLA 2020-04-21 07:45:56 EDT
@Sarika i found that the reason for this that the current instance variables are not pushed in to evaluation engine. Fixing that cause a syntax error in the snippet which still I cannot figure out what causing it.
Comment 5 Gayan Perera CLA 2020-04-22 10:17:06 EDT
@Sarika any comments on this issue. May be i can add the exact problem i found in the evening.
Comment 6 Gayan Perera CLA 2020-04-22 10:58:25 EDT
@Sarika

The problem is in this method
org.eclipse.jdt.internal.debug.eval.ast.engine.ASTEvaluationEngine.getCompiledExpression(String, IJavaStackFrame)

When processing this class we get the following variables if we set a break point at line
Arrays.asList("a", "b", "c").stream().collect(Collectors.toList());

P, this$0

Here this$0 is for the outer scope instance.

None of this variables are pushed into EvaluationSourceGenerator because they don't start with val$. That is the reason we get this NPE because the P that is refereed in the evaluation expression is null.
Comment 7 Gayan Perera CLA 2020-04-22 12:40:00 EDT
@Sarika

There is a problem at here as well

org.eclipse.jdt.internal.debug.eval.ast.instructions.PushLocalVariable.execute() line 62

The variable is not prefixed with val$ in innerThis object.

Do we use val$ for none static inner classes as well, or we only use this for Lambda ?
Comment 8 Gayan Perera CLA 2020-04-22 12:41:12 EDT
Test class to reproduce the problem

package org.eclipse.jdt.debug.tests.eval;

import org.eclipse.debug.core.DebugException;
import org.eclipse.debug.core.model.IValue;
import org.eclipse.jdt.debug.core.IJavaPrimitiveValue;
import org.eclipse.jdt.internal.debug.core.model.JDIObjectValue;

public class NestedTypeFieldValue_InnerTypeVariableTest extends Tests {

	public NestedTypeFieldValue_InnerTypeVariableTest(String name) {
		super(name);
	}

	@Override
	protected void setUp() throws Exception {
		super.setUp();
		initializeFrame("EvalNestedTypeTests", 97, 4);
	}

	@Override
	protected void tearDown() throws Exception {
		destroyFrame();
		super.tearDown();
	}

	public void testInnerTypeField_AB_i() throws DebugException {
		IValue value = eval("i");
		String typeName = value.getReferenceTypeName();
		assertEquals("i : wrong type : ", "int", typeName);
		int intValue = ((IJavaPrimitiveValue) value).getIntValue();
		assertEquals("i : wrong result : ", 9, intValue);

		value = eval("ii");
		typeName = value.getReferenceTypeName();
		assertEquals("i : wrong type : ", "java.lang.String", typeName);
		String strValue = ((JDIObjectValue) value).getValueString();
		assertEquals("i : wrong result : ", "nine", strValue);
	}

	public void testInnerTypeField_A_g() throws DebugException {
		IValue value = eval("A.this.g");
		String typeName = value.getReferenceTypeName();
		assertEquals("g : wrong type : ", "int", typeName);
		int intValue = ((IJavaPrimitiveValue) value).getIntValue();
		assertEquals("g : wrong result : ", 7, intValue);

		value = eval("A.this.gg");
		typeName = value.getReferenceTypeName();
		assertEquals("g : wrong type : ", "java.lang.String", typeName);
		String strValue = ((JDIObjectValue) value).getValueString();
		assertEquals("g : wrong result : ", "seven", strValue);
	}
}
Comment 9 Eclipse Genie CLA 2020-04-22 13:01:54 EDT
New Gerrit change created: https://git.eclipse.org/r/161389
Comment 10 Eclipse Genie CLA 2020-04-22 13:02:17 EDT
New Gerrit change created: https://git.eclipse.org/r/161390
Comment 11 Eclipse Genie CLA 2020-04-22 13:02:20 EDT
New Gerrit change created: https://git.eclipse.org/r/161391
Comment 12 Andrey Loskutov CLA 2020-04-22 13:06:22 EDT
(In reply to Eclipse Genie from comment #9)
> New Gerrit change created: https://git.eclipse.org/r/161389

This fixes NPE in JDT (unrelated to the debugger problem itself JDT shouldn't do stupid things).

(In reply to Eclipse Genie from comment #10)
> New Gerrit change created: https://git.eclipse.org/r/161390

This logs a warning with the stack trace if debugger fails to evaluate due some unexpected error like NPE above.

(In reply to Eclipse Genie from comment #11)
> New Gerrit change created: https://git.eclipse.org/r/161391

This prevents debugger evaluation to print such obscure messages (if we fix NPE) like:

"Marker [on: /, id: 240, type: org.eclipse.jdt.core.transient_problem,
attributes: [charEnd: 74, charStart: 73, id: 33554502, lineNumber: 1,
message: P cannot be resolved or is not a field, severity: 2, sourceId:
JDT], created: 4/22/20 6:58 PM], Marker [on: /, id: 241, type:
org.eclipse.jdt.core.transient_problem, attributes: [charEnd: 210,
charStart: 184, id: 16777222, lineNumber: 6, message: The nested type
workbench.ScopeTest$Runner cannot be referenced using its binary name,
severity: 2, sourceId: JDT], created: 4/22/20 6:58 PM]"

Insead it will report now for the current problem:

"P cannot be resolved or is not a field, The nested type
workbench.ScopeTest$Runner cannot be referenced using its binary name"
Comment 13 Gayan Perera CLA 2020-04-22 13:16:04 EDT
@Andrey where should we fixed the actual bug. Did you read my comments ? What do you think ?
Comment 14 Andrey Loskutov CLA 2020-04-22 13:32:15 EDT
(In reply to Gayan Perera from comment #13)
> @Andrey where should we fixed the actual bug. Did you read my comments ?
> What do you think ?

I haven't yet analyzed this, I've started with your example but first wanted to clean up other issues bothered me during analysis. I hope to find time to look at this today or tomorrow.
Comment 15 Gayan Perera CLA 2020-04-22 13:44:47 EDT
I have create some unit tests as well, i have commented here as well. I can commit them if you want once you fix the issue or if you think we should have them to start with.
Comment 16 Andrey Loskutov CLA 2020-04-22 18:03:59 EDT
I believe 
org.eclipse.jdt.internal.debug.eval.RemoteEvaluatorBuilder.FunctionalEvalVisitor.visit(SimpleName) produces something strange: 
from this AST:
v -> P.test(v)

it produces:
v -> val$this$0.P.test(v)

So the result looks like:

(java.util.function.Predicate<? super java.lang.String>)(v -> val$this$0.P.test(v))

I think this is the place where we need one fix. Looks like we hit the limitations of bug 448473 implementation here.

Another place (responsible for "The nested type
workbench.ScopeTest$Runner cannot be referenced using its binary name") is in EvaluationContextWrapper.evaluateCodeSnippet() - it uses declaringType.getFullyQualifiedName() that can return "$" in the names, I believe, it should use declaringType.getFullyQualifiedName('.').

I will push a patch for that, let see if any test fails.
Comment 17 Eclipse Genie CLA 2020-04-22 18:04:36 EDT
New Gerrit change created: https://git.eclipse.org/r/161410
Comment 20 Andrey Loskutov CLA 2020-04-22 18:30:28 EDT
(In reply to Eclipse Genie from comment #17)
> New Gerrit change created: https://git.eclipse.org/r/161410

With this patch we don't need patch https://git.eclipse.org/r/#/c/161389/, but we see that the debugger evaluation fails at runtime with this error:

java.lang.NullPointerException
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.ensureObj(UnsafeFieldAccessorImpl.java:57)
	at java.base/jdk.internal.reflect.UnsafeObjectFieldAccessorImpl.get(UnsafeObjectFieldAccessorImpl.java:36)
	at java.base/java.lang.reflect.Field.get(Field.java:418)
	at workbench.CodeSnippet_7.lambda$0(CodeSnippet_7.java:9)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:176)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
	at workbench.ScopeTest$Runner.run(ScopeTest.java:23)
	at workbench.ScopeTest.exec(ScopeTest.java:15)
	at workbench.ScopeTest.main(ScopeTest.java:11)

I believe the expression we build in RemoteEvaluatorBuilder can now resolve "P" field & compile the lambda snippet, but "val$this$0.P" seem to confuse runtime.

In debugger I see that val$this$0 = ScopeTest$Runner, but val$this = null, and this is probably the issue here.
Comment 21 Andrey Loskutov CLA 2020-04-22 18:33:33 EDT
@Jesper: would be nice if you could check comment 16 & 20, you should have an idea what we are doing in RemoteEvaluatorBuilder.FunctionalEvalVisitor.visit() :-)
Comment 22 Gayan Perera CLA 2020-04-23 00:48:39 EDT
@Andrey inthink we have a problem here as well https://bugs.eclipse.org/bugs/show_bug.cgi?id=562295#c6. The variables are not passes at all into the expression if they are not prefixed with val$.
Comment 23 Andrey Loskutov CLA 2020-04-23 01:14:58 EDT
(In reply to Gayan Perera from comment #22)
> @Andrey inthink we have a problem here as well
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=562295#c6. The variables are
> not passes at all into the expression if they are not prefixed with val$.

No, this is correct, they are not local variables at all, they are fields. The errors we see later show that we are failing to resolve fields because we are unable to resolve they enclosing workbench.ScopeTest$Runner type, and the reason is https://git.eclipse.org/r/161410.
Comment 24 Gayan Perera CLA 2020-04-23 01:32:58 EDT
Thanks @Andrey for the explanation
Comment 25 Gayan Perera CLA 2020-04-24 03:44:36 EDT
@Andrey did you manage to fixed the evaluation issue ? Let me know if you need help on testing. I can also help with writing some unit tests if you need.
Comment 26 Jesper Moller CLA 2020-04-24 03:54:05 EDT
If I'm not mistaken, the real issue is that P is private and the class we generate cannot get to it without rewriting more expressions in the generated code, using either Unsafe.

I wrote about it here: https://bugs.eclipse.org/bugs/show_bug.cgi?id=448473#c26
Comment 27 Jesper Moller CLA 2020-04-24 03:55:24 EDT
But we should give a better error message, clearly.
Comment 28 Gayan Perera CLA 2020-04-24 08:40:19 EDT
@Jasper i think rather than giving a proper message, we may try to support this scenario. Because all the different evaluations i tried, i tried both in Eclipse and IJ. IJ seems to support all what i have tested and many failed in Eclipse. Thats pretty bad in a world where java is moving more into functional programming with lambda and method references.
Comment 29 Jesper Moller CLA 2020-04-24 10:06:06 EDT
(In reply to Gayan Perera from comment #28)
> @Jasper i think rather than giving a proper message, we may try to support
> this scenario. Because all the different evaluations i tried, i tried both
> in Eclipse and IJ. IJ seems to support all what i have tested and many
> failed in Eclipse. Thats pretty bad in a world where java is moving more
> into functional programming with lambda and method references.

Right.

From what I've seen, IntelliJ uses reflection to access those private fields from within the lambda exprssion. I'm thinking we could use a MethodHandle instead, by using a Lookup object created through JDI/JDWP. It would be faster when running, but at a slightly higher cost at lambda creation type. But maybe the reflection approach is easier, for use for all accesses of non-public methods or fields. Try it!

Note that these limitations are well known: There is an umbrella bug (bug 560247) noting the known limitatons, and if you scroll back in bug 448473, I did warn about both the lack of bandwidth a few times, and about that this was a "minimally viable product", e.g. https://bugs.eclipse.org/bugs/show_bug.cgi?id=448473#c36 .

Sure - we want to support all these scenarios, but there's no help from the JDK, and if I was ordinarily busy when I built this, I'm completely swamped now, as I'm working 70+ hours on my day job on urgent COVID-19 related tasks.

Besides, my name is Jesper, not Jasper.
Comment 30 Gayan Perera CLA 2020-04-24 11:44:08 EDT
@Jesper sorry for name confusion.

Well i see there are limitations, but if we don't try to fix them in the long run the Eclipse JDT will be a limited IDE and at the same time the VScode for java will suffered by the same limitations.

I think we have great set of commiters in JDT, may be we should focus on one milestone/release to fix all these debugger issues.
Comment 31 Gayan Perera CLA 2020-04-24 12:31:42 EDT
@Jesper, i think you have not read the full issue. The current issue with the private field thing has nothing to do with lambda in evaluation expression.

public class ScopeTest {
	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 you put a break point inside the run method and try to evaluate "P.test("a")" will result in "cannot fine the field P:" This is a pretty simple evaluation right ?

But if we evaluate this as this.P it works. But then again it fails at

CodeSnippet_1.lambda$0(String) line: 10	

The reason is sun.reflect.UnsafeFieldAccessorImpl.ensureObj(Object) the object is null.
Comment 33 Jesper Moller CLA 2020-04-25 13:58:33 EDT
(In reply to Gayan Perera from comment #31)
> @Jesper, i think you have not read the full issue. The current issue with
> the private field thing has nothing to do with lambda in evaluation
> expression.

In the original bug description, it has.

> If you put a break point inside the run method and try to evaluate
> "P.test("a")" will result in "cannot fine the field P:" This is a pretty
> simple evaluation right ?

I should evaluate P.test("a") fine, because it does not use the functional expression rewriting workaround.

However, evaluating P inside a lambda expression doesn't work yet, and is known limitation as I explained above.
Comment 34 Gayan Perera CLA 2020-04-26 11:08:23 EDT
@Jesper do you have any time to give any tips to where to look at if I want to add support to inner class fields inside lambdas? You mentioned about Unsafe access using reflection, where does the unsafe access is made, because when looking at the snippet that is generated it has val$this$0.P in the code snippet.
Comment 35 Jesper Moller CLA 2020-04-26 19:50:54 EDT
Here are some tips, jumbled slightly together.

It is NOT just inner class fields, it is ALL field and method accesses into members of all surrounding scopes, which will have to be wrapped somehow IF THE MEMBERS ARE NOT PUBLIC. Consider:

public class Whatever {
    private int a = 0;

    void method() {
        // breakpoint here, somebody evaluates "new Thread(() -> a=1)"
    }
}

For that to work, both accesses to 'Whatever.this.a' needs to go through a cheating mechanism, e.g. by rewriting it. First, the 'this' reference is turned into a field in the generated class, as you note: val$this$0

This is already handled by the rewriter, in org.eclipse.jdt.internal.debug.eval.RemoteEvaluatorBuilder.FunctionalEvalVisitor.visit(SimpleName), using the 'binder' to bind values (except the ones which are local to the functional expression, obviously).

The basic fixes needed are in "visit(MethodInvocation node)" and "visit(FieldAccess node)":

As for the latter, there is already a nice TODO: Make tricks here when we access fields where we have non-public access.

The strategy from the top of my head is this:

*** Field reads:

The simple case is where we see a FieldAccess which is a read a field which is not public, i.e.

<owner>.<fieldName>

We replace it with:

<class of receiver>.class.getField("<field>").get(<receiver>)

*** Field writes:

Field writes must be detected through visit(Assignment node), but should be rewritten from:

<receiver>.<field> = <value>

into

<class of receiver>.class.getField("<field>").set(<receiver>, <value>)

*** Method invocation:

Method invocation is relatively easy, where non-public calls like:

<receiver>.<method>(<arg0> ... <argN>)

should be rewritten into

<class of receiver>.class.getMethod(
     "<method>",
     <resolved type of parameter0> ... <resolved type of parameterN>
).invoke(<receiver>, <arg0>, <arg1>)

Important here to make sure the method is properly resolved first.
Be careful around varargs, and possibly bridge methods.

Constructors and static methods should follow easily once you have methods nailed.
Even qualified instance creation (someObject.new Inner()) should be doable, it's just a combination of the above techniques.

Oh, you should also ensure that all the accessible members are made accessible first (using setAccessible). That only need to be done once.

--------------------

That was the easy part. Spookier bits:

Postfix and prefix expressions like "++a" and "a += 42" will be tricky to rewrite into expression form. Consider:

<receiver>.<field> += <expr>

This should get rewritten into:

<class of receiver>.class.getField("<field>").set(<receiver>,
     <class of receiver>.class.getField("<field>").get(<receiver>) + <expr>

EXCEPT: We'd evaluate <receiver> twice which would be wrong if it had side effects.

To illustrate:
We can turn expr.field++ into:

(exprTmp = expr) == null ? exprTmp.field : (exprTmp.field = exprTmp.field + 1)

We need exprTmp.field to preserve null-semantics anyway.

To support that, we would need a separate variable to hold the receiver, a la 
<class of receiver> tempReceiver;

Expression:
(tempReceiver = <receiver>) == null ?
  <class of receiver>.class.getField("<field>").get(tempReceiver) :
  <class of receiver>.class.getField("<field>").set(tempReceiver,
  <class of receiver>.class.getField("<field>").get(tempReceiver) + <expr>

That rewrite only works when the expression is "used" (like in an assignment or method call), so you might have to have a spare variable to write the result into.

-----------

Method references referring to non-public methods:
You'd have to convert them into lambdas, really.

Have fun!
Comment 36 Gayan Perera CLA 2020-04-27 12:41:38 EDT
@Jesper thanks for the inputs, This really helps a lot. I think we can keep the spookier parts for later and tackle the straight forward parts.

@Andrey what do you think? Should we continue on this bug or should we report a separate one?
Comment 37 Eclipse Genie CLA 2020-04-27 14:42:16 EDT
New Gerrit change created: https://git.eclipse.org/r/161599
Comment 38 Gayan Perera CLA 2020-04-27 14:47:24 EDT
@Jesper and @Andrey added a temp fix, Lets see if any tests fails and if you guys ok with the fix I can add some Tests.

The problem I found:

All the expression we rewrite has the this variable rewritten to val$this$0. But the variables at org.eclipse.jdt.internal.debug.eval.RemoteEvaluatorBuilder.build() is still just this$0. So the org.eclipse.jdt.core.eval.IEvaluationContext.evaluateCodeSnippet(String, String[], String[], int[], IType, boolean, boolean, ICodeSnippetRequestor, IProgressMonitor) invocation doesn't find the correct variable value instead it finds a null value for val$this$0 in the rewritten instructions.

I might be wrong since I'm very new to this area. Please guide me if you think I can be any help.
Comment 39 Gayan Perera CLA 2020-04-29 09:01:02 EDT
@Andrey @Sarika since Jesper is busy can one of you look at my temp patch to see if its ok ? If its ok i can complete with code cleanup
Comment 40 Andrey Loskutov CLA 2020-04-29 09:56:13 EDT
(In reply to Gayan Perera from comment #39)
> @Andrey @Sarika since Jesper is busy can one of you look at my temp patch to
> see if its ok ? If its ok i can complete with code cleanup

No, sorry, not on my prio list. But I would add tests to the patch so that one can understand which problem it solves (if any :-)).
Comment 41 Gayan Perera CLA 2020-04-29 10:43:04 EDT
@Andrey you mean tests for your patches right ?
Comment 42 Gayan Perera CLA 2020-04-29 10:56:48 EDT
Any one who can help me to review the patch in the issue mailing list. Or if you don’t have time please add some one who may be helpful. This is for all eclipse JDT users.
Comment 43 Sarika Sinha CLA 2020-04-29 12:32:19 EDT
(In reply to Gayan Perera from comment #41)
> @Andrey you mean tests for your patches right ?

Test case which is being fixed by https://git.eclipse.org/r/161599 will help to understand the patch.
Comment 44 Gayan Perera CLA 2020-04-29 14:21:00 EDT
I will update tests after we are done with the https://bugs.eclipse.org/bugs/show_bug.cgi?id=562079. Because the test class I added there can be used for this as well.
Comment 45 Gayan Perera CLA 2020-04-30 12:40:39 EDT
@Sarika I have added the tests, please check and let me know so I can clean up the code a little bit.
Comment 47 Sarika Sinha CLA 2020-05-01 10:13:57 EDT
@Gayan, Can you create a separate bug to track the suggestions by Jesper in comment#35.
Comment 48 Gayan Perera CLA 2020-05-01 10:20:39 EDT
@Sarika what @Jesper mention is already there I think, if you look at my tests it does exactly what he has in his example as well.

I found that even though the code for reflection access is there for fields, it lacks for methods. Let me know if I'm correct on this, I can create an issue for method scenario with a sample code.
Comment 49 Sarika Sinha CLA 2020-05-01 10:25:14 EDT
(In reply to Gayan Perera from comment #48)
> @Sarika what @Jesper mention is already there I think, if you look at my
> tests it does exactly what he has in his example as well.
> 
> I found that even though the code for reflection access is there for fields,
> it lacks for methods. Let me know if I'm correct on this, I can create an
> issue for method scenario with a sample code.

Yes, I meant the bug for method scenario.
Comment 50 Gayan Perera CLA 2020-05-01 10:35:39 EDT
@Sarika you have merged the temp fix for master isn't it? I just pushed the gerrit with an update on the string literals and commit message. The previous code was not cleaned up :(. Can we merged the new changes instead of the old code?
Comment 51 Sarika Sinha CLA 2020-05-01 10:43:43 EDT
(In reply to Gayan Perera from comment #50)
> @Sarika you have merged the temp fix for master isn't it? I just pushed the
> gerrit with an update on the string literals and commit message. The
> previous code was not cleaned up :(. Can we merged the new changes instead
> of the old code?

Yes, but where is the new gerrit, I can't see?
Comment 52 Gayan Perera CLA 2020-05-01 10:54:59 EDT
(In reply to Sarika Sinha from comment #51)
> (In reply to Gayan Perera from comment #50)
> > @Sarika you have merged the temp fix for master isn't it? I just pushed the
> > gerrit with an update on the string literals and commit message. The
> > previous code was not cleaned up :(. Can we merged the new changes instead
> > of the old code?
> 
> Yes, but where is the new gerrit, I can't see?

It doesn't allow me to push, I think its because the gerrit is merged and closed. Can we revert it and then correct it and merge back ?
Comment 54 Sarika Sinha CLA 2020-05-04 00:54:40 EDT
(In reply to Gayan Perera from comment #52)

> It doesn't allow me to push, I think its because the gerrit is merged and
> closed. Can we revert it and then correct it and merge back ?

Changes are reverted https://git.eclipse.org/r/#/c/161911/
You can cretae a new gerrit.
Comment 55 Gayan Perera CLA 2020-05-04 02:53:33 EDT
@Sarika should i commit to same gerrit with the same change id or should push 2 commits i have sqaushed and commit as a new gerrit ?
Comment 56 Sarika Sinha CLA 2020-05-04 03:23:22 EDT
(In reply to Gayan Perera from comment #55)
> @Sarika should i commit to same gerrit with the same change id or should
> push 2 commits i have sqaushed and commit as a new gerrit ?

2 commits squashed into new gerrit will be better.
Comment 57 Eclipse Genie CLA 2020-05-04 11:38:32 EDT
New Gerrit change created: https://git.eclipse.org/r/161986
Comment 58 Gayan Perera CLA 2020-05-04 12:45:42 EDT
@Sarika i have added a new gerrit.
Comment 60 Sarika Sinha CLA 2020-05-05 00:31:09 EDT
Thanks Gayan!
Comment 61 Sarika Sinha CLA 2020-05-21 08:03:31 EDT
 I20200518-2220