eclipse-jdtls / eclipse-jdt-core-incubator Goto Github PK

See #12, treat this as a subissue for that epic

I tried to compile against Java 22 and it failed with

[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:4.0.7-SNAPSHOT:compile (default-compile) on project org.eclipse.jdt.core.javac: Compilation failure: Compilation failure:
[ERROR] /Users/fbricon/Dev/projects/eclipse-jdt-core-incubator/org.eclipse.jdt.core.javac/src/org/eclipse/jdt/core/dom/JavacConverter.java:[1252,36] cannot find symbol
[ERROR] symbol: method lastIndexOf(byte)
[ERROR] location: variable javac of type com.sun.tools.javac.util.Name
[ERROR] -> [Help 1]

See

I seem to have fixed it with a simple change:

private Name convert(com.sun.tools.javac.util.Name javac) {
		if (javac == null || Objects.equals(javac, Names.instance(this.context).error) || Objects.equals(javac, Names.instance(this.context).empty)) {
			return null;
		}
		String nameString = javac.toString();
		int lastDot = nameString.lastIndexOf(".");
		if (lastDot < 0) {
			return this.ast.newSimpleName(nameString);
		} else {
			return this.ast.newQualifiedName(convert(javac.subName(0, lastDot)), (SimpleName)convert(javac.subName(lastDot + 1, javac.length() - 1)));
		}
		// position is set later, in FixPositions, as computing them depends on the sibling
	}

See #12, treat this as a subissue for that epic

[DOM-first] Enable DOM-first for the dom-based-operations branch in Jenkins to get detailed report

Currently, many JDT tests are still failing when enabling DOM-first.
We need to reduce that list to near-0.

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

We will soon have some branches with work related to Javac backend and its necessary requirement DOM-first JDT.
To facilitate contribution (ensure it doesn't regress main JDT, but also ensure it doesn't regress the current DOM-first or Javac execution), we need to setup a GitHub action that will also build JDT with the necessary flags to involve the developed features.

Concretely, it can be a matter of looking at the Git history for some keywords (eg stable commit messages) and to add the system properties to the pom files accordingly.

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

By the name these suites look related, so I have grouped them together.

Spent about a week and a half trying to puzzle through CompletionParser, just to get a proof of concept of the main issues with either migrating or rewriting it. The PoC was intended to use a module-info.java file, as it's easier and simpler than the rest and would demonstrate the problems just as well. I became determined to finish the PoC despite the fact that the issues discovered really made it unreasonable to do so, especially if the point was now to use the PoC to show how unfeasible it was to continue along that path.

The tl;dr is, the current DOM simply misses nodes if they are in error. Those nodes are simply ignored and left out. The only way to find those nodes is to reparse, which is what the current CompletionEngine does, and involves itself heavily in the parsing process to determine exactly where and when the parse goes bad and then caches the data in appropriate objects like CompletionOnArgumentName or similar.

Trying to replicate this using only a partial dom and string manipulation is difficult even in the simple module-info.java file, and is downright nonsensical in an actual java file.

The problem is, the official DOM structure, as far as I can tell, only really has room for mostly complete nodes. Incomplete nodes are intended to be removed, or at least it seems that way to me.

One example is that every dom node is expected to have a type, as defined by the type constants in org.eclipse.jdt.core.dom.ASTNode. But if a user has a java file like:

public MyConstructor() {
   int x = 3;
   int [cursor]
   int y = 2;
}

The second expression is invalid, and indeed isn't even an expression. There does not appear to exist any InvalidExpression node or any nodes that would indicate an error situation. This implies the dom tree is intended to include only valid nodes.

Trying to untangle how this could work with a javac implementation necessarily brings in deeper discussions as to whether the official dom tree should contain or otherwise indicate invalid nodes or incomplete nodes, or whether a javac parser implementation should be subclassable in the way AssistParser and CompletionParser are, to participate in the parsing and to respond to errors as they occur.

This issue will likely remain unworked on for the forseeable future until the dom and other APIs are developed to such a level that would allow easier involvement of a completion engine during a javac parse.

Caused by: java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "javac" is null
	at org.eclipse.jdt.core.dom.JavacConverter.convertToType(JavacConverter.java:1250)
	at org.eclipse.jdt.core.dom.JavacConverter.convertStatement(JavacConverter.java:1008)
	at org.eclipse.jdt.core.dom.JavacConverter.convertStatement(JavacConverter.java:958)
	at org.eclipse.jdt.core.dom.JavacConverter.convertBlock(JavacConverter.java:1146)
	at org.eclipse.jdt.core.dom.JavacConverter.convertMethodDecl(JavacConverter.java:451)
	at org.eclipse.jdt.core.dom.JavacConverter.convertBodyDeclaration(JavacConverter.java:396)
	at org.eclipse.jdt.core.dom.JavacConverter.convertClassDecl(JavacConverter.java:281)
	at org.eclipse.jdt.core.dom.JavacConverter.convertClassDecl(JavacConverter.java:220)
	at org.eclipse.jdt.core.dom.JavacConverter.convertBodyDeclaration(JavacConverter.java:399)
	at org.eclipse.jdt.core.dom.JavacConverter.convertClassDecl(JavacConverter.java:281)
	at org.eclipse.jdt.core.dom.JavacConverter.convertClassDecl(JavacConverter.java:220)
	at org.eclipse.jdt.core.dom.JavacConverter.convertBodyDeclaration(JavacConverter.java:399)
	at org.eclipse.jdt.core.dom.JavacConverter.convertClassDecl(JavacConverter.java:281)
	at org.eclipse.jdt.core.dom.JavacConverter.convertClassDecl(JavacConverter.java:220)
	at org.eclipse.jdt.core.dom.JavacConverter.convertBodyDeclaration(JavacConverter.java:399)
	at org.eclipse.jdt.core.dom.JavacConverter.lambda$2(JavacConverter.java:142)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at org.eclipse.jdt.core.dom.JavacConverter.populateCompilationUnit(JavacConverter.java:143)
	at org.eclipse.jdt.core.dom.JavacCompilationUnitResolver.parse(JavacCompilationUnitResolver.java:154)
	at org.eclipse.jdt.core.dom.JavacCompilationUnitResolver.toCompilationUnit(JavacCompilationUnitResolver.java:107)
	at org.eclipse.jdt.core.dom.ASTParser.internalCreateCompilationUnit(ASTParser.java:1246)
	at org.eclipse.jdt.core.dom.ASTParser.internalCreateASTCached(ASTParser.java:1158)
	at org.eclipse.jdt.core.dom.ASTParser.lambda$0(ASTParser.java:1125)
	at org.eclipse.jdt.internal.core.JavaModelManager.cacheZipFiles(JavaModelManager.java:5762)
	at org.eclipse.jdt.core.dom.ASTParser.internalCreateAST(ASTParser.java:1125)
	at org.eclipse.jdt.core.dom.ASTParser.createAST(ASTParser.java:873)
	at org.eclipse.jdt.internal.core.CompilationUnit.buildStructure(CompilationUnit.java:186)
	at org.eclipse.jdt.internal.core.Openable.generateInfos(Openable.java:245)
	at org.eclipse.jdt.internal.core.JavaElement.openWhenClosed(JavaElement.java:585)
	at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(JavaElement.java:308)
	at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(JavaElement.java:294)
	at org.eclipse.jdt.internal.core.Openable.getBuffer(Openable.java:275)
	at org.eclipse.jdt.core.manipulation.CoreASTProvider.hasSource(CoreASTProvider.java:393)
	at org.eclipse.jdt.core.manipulation.CoreASTProvider.createAST(CoreASTProvider.java:269)
	at org.eclipse.jdt.core.manipulation.CoreASTProvider.getAST(CoreASTProvider.java:199)
	at org.eclipse.jdt.ls.core.internal.handlers.SemanticTokensHandler.getAst(SemanticTokensHandler.java:83)
	at org.eclipse.jdt.ls.core.internal.handlers.SemanticTokensHandler.full(SemanticTokensHandler.java:50)
	at org.eclipse.jdt.ls.core.internal.handlers.JDTLanguageServer.lambda$54(JDTLanguageServer.java:1176)
	at org.eclipse.jdt.ls.core.internal.BaseJDTLanguageServer.lambda$0(BaseJDTLanguageServer.java:87)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646)
	... 6 more

Example file is https://github.com/eclipse/lemminx-maven/blob/master/lemminx-maven/src/main/java/org/eclipse/lemminx/extensions/maven/searcher/LocalRepositorySearcher.java

No test case created. But current code does have an apparent NPE weakness which I'll work on.

See #12, treat this as a subissue for that epic

// Inspired from test429738, [1.8][search] Find Declarations (Ctrl + G) shows no result for type-less lambda parameter
public void test429738() throws CoreException {
	org.eclipse.jdt.internal.core.CompilationUnit workingCopy = (org.eclipse.jdt.internal.core.CompilationUnit) getWorkingCopy("/JavaSearchBugs/src/b400905/X.java", "");
	this.workingCopies = new ICompilationUnit[] { workingCopy };
	workingCopy.getBuffer().setContents("""
		@FunctionalInterface
		interface Foo {
			int foo(int x);
		}
		public class X {
			// Select 'x' in lambda body and press Ctrl+G.
			Foo f1= x -> /* here*/ x; //[1]
			Foo f2= (int x) -> x; //[2]
		}
		""");

	String selection = "/* here*/ x";
	int start = workingCopy.getSource().indexOf(selection) + 10;
	int length = 1;
	assertEquals("x", workingCopy.getSource().substring(start, start + length));
	CompilationUnit dom = workingCopy.makeConsistent(AST.getJLSLatest(), true, 0, new HashMap<>(), null);
	SimpleName name = (SimpleName)NodeFinder.perform(dom, start, 1);
	IVariableBinding binding = (IVariableBinding)name.resolveBinding();
	ILocalVariable local = (ILocalVariable)binding.getJavaElement();
	search(local, DECLARATIONS, EXACT_RULE);
	assertSearchResults(
			"src/b400905/X.java int b400905.X.f1:<lambda #1>.foo(int).x [x] EXACT_MATCH");
}

This cause a bunch of other failures as the wrong ILocalVariable seems to be resolved

junit.framework.ComparisonFailure: Unexpected search results.
----------- Expected ------------
src/b400905/X.java int b400905.X.f1:<lambda #1>.foo(int).x [x] EXACT_MATCH
------------ but was ------------
src/b400905/X.java int b400905.Foo.foo(int).x [x] EXACT_MATCH
---------------------- ----------
 expected:<.../X.java int b400905.[X.f1:<lambda #1>].foo(int).x [x] EXAC...> but was:<.../X.java int b400905.[Foo].foo(int).x [x] EXAC...>
	at org.eclipse.jdt.core.tests.junit.extension.TestCase.assertStringEquals(TestCase.java:267)
	at org.eclipse.jdt.core.tests.junit.extension.TestCase.assertEquals(TestCase.java:242)
	at org.eclipse.jdt.core.tests.model.AbstractJavaSearchTests.assertSearchResults(AbstractJavaSearchTests.java:948)
	at org.eclipse.jdt.core.tests.model.AbstractJavaSearchTests.assertSearchResults(AbstractJavaSearchTests.java:906)
	at org.eclipse.jdt.core.tests.model.AbstractJavaSearchTests.assertSearchResults(AbstractJavaSearchTests.java:903)
	at org.eclipse.jdt.core.tests.model.JavaSearchBugs8Tests.test429738(JavaSearchBugs8Tests.java:3448)

This might be an upstream JDT issue, however I didn't yet had the opportunity to try to confirm it with vanilla JDT before reporting it. If it appears to be reproducible with plain JDT, the issue should be cloned, and fixed, upstream.

See #12, treat this as a subissue for that epic

Currently we're a bit struggling with CompletionEngine (and possibly others) in case of a syntaxically incorrect compilation unit. The DOM returned by JDT over ECJ is a bit "weak" and misses instructions.
Would it be possible that Javac recovers it better and adds the necessary information? This needs to be tested.

See #12, treat this as a subissue for that epic

We should try to run some basic tests (a simple project with a couple of deps as jars, edits 3-4 files, run hover/completion...) with Javac vs ECJ and profile them to get some metrics that might allow to focus some attention on particular topics of javac.

See #12, treat this as a subissue for that epic

We need to complete the logic that maps Javac diagnostics to JDT problems, in order to enable proper messages and quick-fixes for compiler problems.

Since Javac work only makes sense when DOM is enabled, we should probably try to automate having the javac branch always rebased on top of dom one (itself rebased on top of develop one, itself rebased on top of upstream master one..), so we can easily test the whole up-to-date stack from that branch.

When using the Javac-based DOM, may bindings are not resolved. There are a lot of TODO left. We need to complete those TODOs to hope Javac-based DOM is complete and usable everywhere.

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

As discussed earlier with @robstryker after he carried out a lot of analysis, enabling completion on top of a potentially faulty DOM is tricky, and what ECJ-based does is that it keeps track of the grammar rules that are being applied and that is usually very helpful.
We should investigate how NetBeans deals with this: does it manage to re-infer rules from a potentially faulty DOM? Or does it reimplement some kind of parser to get some rules and facilitate completion? Or does it do something else...?

I'm looking into resolving the root cause of the test failure for JavaSearchTest.testEnum04

Resolve tests for Java 2 and Java 1.5 are in a separate issue.

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

Many test suites only have one or two failing tests that aren't covered in other issues. Here they are:

• org.eclipse.jdt.core.tests.model.CopyMoveElementsTests.testCopyImportStatic
• org.eclipse.jdt.core.tests.model.ClasspathInitializerTests.testContainerInitializer06
• org.eclipse.jdt.core.tests.model.WorkingCopyOwnerTests.testNewWorkingCopy09
• org.eclipse.jdt.core.tests.model.NullAnnotationModelTests.testBug479389
• org.eclipse.jdt.core.tests.model.Java9ElementTests.test003
• org.eclipse.jdt.core.tests.model.Java9ElementTests.test530653
• org.eclipse.jdt.core.tests.model.RecordsElementTests.test005
• org.eclipse.jdt.core.tests.model.RecordsElementTests.test006

See #12, treat this as a subissue for that epic

The current completion engine cannot be easily reused in a DOM-first approach as it heavily relies on ECJ types.
This needs to be rewritten to only use JDT DOM types as much as possible.

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

!!!NEEDS TO BE VERIFIED BEFORE SUBMITTING TO UPSTREAM JDT!!! because the failure here might be a side-effect of some other bugs we have with dom-based

Name.resolveBinding() seems to sometimes return erroneous results for module-info.java package directives

Pseudo-code

IJavaProject project = createJavaProject("Java9Elements", new String[] {"src"}, new String[] {"JCL19_LIB"}, "bin", "9");
project.open(null);
createFile("/Java9Elements/src/module-info.java", """
  module my.mod{
    exports p.q.r;
    exports a.b.c;
  }""");
ICompilationUnit unit = getCompilationUnit("/Java9Elements/src/module-info.java");
int start = fileContent.indexOf("p.q");
ASTNode ast = buildAST(unit);
Name pqName = NodeFinder.perform(currentAST, offset, 3);
PackageBinding pqBinding = pqName.resolveBinding();
assertEquals("p.q", pqBinding.getName()); // fails, we get `p.q.r` instead

This was discovered while trying to make Java9ElementTest.test003 work with codeSelect using a DOM-first approach (eclipse-jdt#1983 ).
It's probably related to, or caused by, https://bugs.eclipse.org/bugs/show_bug.cgi?id=511923 (cc @mpalat )

CompilationUnitProblemFinder.resolve triggers extra analysis that is often desired by tests.
Currently this is not invoked in the DOM_BASED_OPERATIONS branch. As it's using ECJ APIs, this should probably be encapsulated in ASTParser/CompilationUnitResolver.

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

See #12, treat this as a subissue for that epic

This ticket lists some upstream issues that cause some failures with the current DOM-based operations proposal

Fix the test when building the model with using the DOM.

The root cause is that the existing model builder (that uses SourceElementParser) successfully recovers the anonymous class in the following snippet, whereas the AST that DOMToModelPopulator gets doesn't.

public class X {
        void test() {
                new Object() {
                        /*start*/public void yes() {
                                System.out.println("hello world");
                        }/*end*/
                } // missing semicolon
        }
}