Switch to make some operations rely on DOM instead of ECJ

Introduced 2 system property switches to control whether some IDE operations are using ECJ parser (legacy/default) or whether to make those operations powered by a full DOM. * CompilationUnit.DOM_BASED_OPERATIONS will make codeSelect (hover, link to definition...), buildStructure (JDT Project element model), reconciler (code diagnostics feedback); this one seems currently working ✔ * CompilationUnit.codeComplete.DOM_BASED_OPERATIONS controls completion (unlike other operations, completion based on DOM is far from being complete or straightforward) 🏗️ * SourceIndexer.DOM_BASED_INDEXER controls whether the indexation of a source document should first build/use a full DOM. This one is currently incomplete 🏗️ The DOM-based operation can then allow to plug alternative compiler then ECJ and still get JDT functionalities working. Also-By: David Thompson <[email protected]> Also-By: Snjezana Peco <[email protected]>
eclipse-jdt · Mar 15, 2024 · 7649a67 · 7649a67
1 parent 329c4da
commit 7649a67
Show file tree

Hide file tree

Showing 12 changed files with 2,266 additions and 136 deletions.
diff --git a/.github/workflows/ci-dom-javac.yml b/.github/workflows/ci-dom-javac.yml
@@ -0,0 +1,53 @@
+name: Continuous Integration with DOM/Javac
+concurrency: 
+    group: ${{ github.workflow }}-${{ github.ref }}-dom
+    cancel-in-progress: true
+
+on:
+  push:
+    branches: [ 'dom-based-operations', 'dom-with-javac' ]
+  pull_request:
+    branches: [ 'dom-based-operations', 'dom-with-javac' ]
+
+jobs:
+  build-dom-javac:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Install xmllint
+      shell: bash
+      run: |
+        sudo apt update
+        sudo apt install -y libxml2-utils
+    - uses: actions/checkout@v3
+      with:
+        submodules: recursive
+        fetch-depth: 0
+    - name: Enable DOM-first and Javac
+      run: sed -i 's$</argLine>$ -DCompilationUnit.DOM_BASED_OPERATIONS=true -DSourceIndexer.DOM_BASED_INDEXER=false -DICompilationUnitResolver=org.eclipse.jdt.core.dom.JavacCompilationUnitResolver -DAbstractImageBuilder.compiler=org.eclipse.jdt.internal.javac.JavacCompiler_</argLine>$g' */pom.xml
+    - name: Set up JDKs ☕
+      uses: actions/setup-java@v3
+      with:
+        java-version: |
+          8
+          17
+          20
+        mvn-toolchain-id: |
+          JavaSE-1.8
+          JavaSE-17
+          JavaSE-20
+        distribution: 'temurin'
+    - name: Set up Maven
+      uses: stCarolas/setup-maven@07fbbe97d97ef44336b7382563d66743297e442f # v4.5
+      with:
+        maven-version: 3.9.3
+    - name: Build with Maven 🏗️
+      run: |
+        mvn clean install --batch-mode -f org.eclipse.jdt.core.compiler.batch -DlocalEcjVersion=99.99
+        mvn -U clean verify --batch-mode --fail-at-end -Ptest-on-javase-20 -Pbree-libs -Papi-check -Djava.io.tmpdir=$WORKSPACE/tmp -Dproject.build.sourceEncoding=UTF-8 -Dtycho.surefire.argLine="--add-modules ALL-SYSTEM -Dcompliance=1.8,11,17,20 -Djdt.performance.asserts=disabled" -Dcbi-ecj-version=99.99
+    - name: Test Report
+      if: success() || failure()    # run this step even if previous step failed
+      run: |
+        echo ▶️ TESTS RUN: $(xmllint --xpath 'string(/testsuite/@tests)' */target/surefire-reports/TEST-*.xml | awk '{n += $1}; END{print n}' -)
+        echo ❌ FAILURES: $(xmllint --xpath 'string(/testsuite/@failures)' */target/surefire-reports/TEST-*.xml | awk '{n += $1}; END{print n}' -)
+        echo 💥 ERRORS: $(xmllint --xpath 'string(/testsuite/@errors)' */target/surefire-reports/TEST-*.xml | awk '{n += $1}; END{print n}' -)
+        echo 🛑 SKIPPED: $(xmllint --xpath 'string(/testsuite/@skipped)' */target/surefire-reports/TEST-*.xml | awk '{n += $1}; END{print n}' -)
diff --git a/Jenkinsfile b/Jenkinsfile
@@ -13,7 +13,7 @@ pipeline {
 		jdk 'openjdk-jdk21-latest'
 	}
 	stages {
-		stage('Build') {
+		stage('Build and Test using ECJ') {
 			steps {
 					sh """#!/bin/bash -x
 					
@@ -30,10 +30,10 @@ pipeline {
 					# export MAVEN_OPTS="-Xmx2G"
 					
 					mvn clean install -f org.eclipse.jdt.core.compiler.batch -DlocalEcjVersion=99.99 -Dmaven.repo.local=$WORKSPACE/.m2/repository -DcompilerBaselineMode=disable -DcompilerBaselineReplace=none
-					
+
+					# Build and test without DOM-first to ensure no regression takes place
 					mvn -U clean verify --batch-mode --fail-at-end -Dmaven.repo.local=$WORKSPACE/.m2/repository \
 						-Ptest-on-javase-21 -Pbree-libs -Papi-check -Pjavadoc -Pp2-repo \
-						-Dmaven.test.failure.ignore=true \
 						-Dcompare-version-with-baselines.skip=false \
 						-Djava.io.tmpdir=$WORKSPACE/tmp -Dproject.build.sourceEncoding=UTF-8 \
 						-Dtycho.surefire.argLine="--add-modules ALL-SYSTEM -Dcompliance=1.8,11,17,20,21 -Djdt.performance.asserts=disabled" \
@@ -54,8 +54,40 @@ pipeline {
 					// The eclipse compiler name is changed because the logfile not only contains ECJ but also API warnings.
 					// "pattern:" is used to collect warnings in dedicated files avoiding output of junit tests treated as warnings   
 					junit '**/target/surefire-reports/*.xml'
-					discoverGitReferenceBuild referenceJob: 'eclipse.jdt.core-github/master'
-					recordIssues publishAllIssues:false, ignoreQualityGate:true, tool: eclipse(name: 'Compiler and API Tools', pattern: '**/target/compilelogs/*.xml'), qualityGates: [[threshold: 1, type: 'DELTA', unstable: true]]
+					//discoverGitReferenceBuild referenceJob: 'eclipse.jdt.core-github/master'
+					//recordIssues publishAllIssues:false, ignoreQualityGate:true, tool: eclipse(name: 'Compiler and API Tools', pattern: '**/target/compilelogs/*.xml'), qualityGates: [[threshold: 1, type: 'DELTA', unstable: true]]
+				}
+			}
+		}
+		stage("Build and Test with DOM-first") {
+			steps {
+				sh """
+				# Then enable DOM-first
+				sed -i 's|</argLine>| -DCompilationUnit.DOM_BASED_OPERATIONS=true -DSourceIndexer.DOM_BASED_INDEXER=false -DICompilationUnitResolver=org.eclipse.jdt.core.dom.JavacCompilationUnitResolver -DAbstractImageBuilder.compiler=org.eclipse.jdt.internal.javac.JavacCompiler_</argLine>|g' */pom.xml
+				# and build/run it
+				mvn -U clean verify --batch-mode --fail-at-end -Dmaven.repo.local=$WORKSPACE/.m2/repository \
+					-Ptest-on-javase-21 -Pbree-libs -Papi-check -Pjavadoc -Pp2-repo \
+					-Dcompare-version-with-baselines.skip=false \
+					-Djava.io.tmpdir=$WORKSPACE/tmp -Dproject.build.sourceEncoding=UTF-8 \
+					-Dtycho.surefire.argLine="--add-modules ALL-SYSTEM -Dcompliance=1.8,11,17,20,21 -Djdt.performance.asserts=disabled -DCompilationUnit.DOM_BASED_OPERATIONS=true -DSourceIndexer.DOM_BASED_INDEXER=false -DICompilationUnitResolver=org.eclipse.jdt.core.dom.JavacCompilationUnitResolver -DAbstractImageBuilder.compiler=org.eclipse.jdt.internal.javac.JavacCompiler_ " \
+					-Dtycho.surefire.error=ignore -Dtycho.surefire.failure=ignore \
+					-DDetectVMInstallationsJob.disabled=true \
+					-Dtycho.apitools.debug \
+					-Dcbi-ecj-version=99.99
+				"""
+			}
+			post {
+				always {
+					archiveArtifacts artifacts: '*.log,*/target/work/data/.metadata/*.log,*/tests/target/work/data/.metadata/*.log,apiAnalyzer-workspace/.metadata/*.log,repository/target/repository/**', allowEmptyArchive: true
+					// The following lines use the newest build on master that did not fail a reference
+					// To not fail master build on failed test maven needs to be started with "-Dmaven.test.failure.ignore=true" it will then only marked unstable.
+					// To not fail the build also "unstable: true" is used to only mark the build unstable instead of failing when qualityGates are missed
+					// Also do not record mavenConsole() as failing tests are logged with ERROR duplicating the failure into the "Maven" plugin
+					// To accept unstable builds (test errors or new warnings introduced by third party changes) as reference using "ignoreQualityGate:true"
+					// To only show warnings related to the PR on a PR using "publishAllIssues:false"
+					// The eclipse compiler name is changed because the logfile not only contains ECJ but also API warnings.
+					// "pattern:" is used to collect warnings in dedicated files avoiding output of junit tests treated as warnings   
+					junit '**/target/surefire-reports/*.xml'
 				}
 			}
 		}

diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ResolveTests.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ResolveTests.java
@@ -14,6 +14,7 @@
 package org.eclipse.jdt.core.tests.model;
 
 import java.io.IOException;
+import java.util.Set;
 
 import junit.framework.Test;
 
@@ -975,10 +976,9 @@ public void testLocalVarIsStructureKnown() throws JavaModelException {
  */
 public void testLocalVarTypeSignature1() throws JavaModelException {
 	ILocalVariable localVar = getLocalVariable("/Resolve/src/ResolveLocalName.java", "var1 = new Object();", "var1");
-	assertEquals(
-		"Unexpected type signature",
-		"QObject;",
-		localVar.getTypeSignature());
+	assertTrue("Unexpected type signature",
+		Set.of("QObject;", "Ljava.lang.Object;").contains(
+		localVar.getTypeSignature()));
 }
 /*
  * Resolve a local reference and ensure its type signature is correct.
@@ -1466,10 +1466,9 @@ public void testDuplicateLocals1() throws JavaModelException {
 			elements
 		);
 
-	assertEquals(
-			"Unexpected type",
-			"QTestString;",
-			((ILocalVariable)elements[0]).getTypeSignature());
+	assertTrue("Unexpected type",
+		Set.of("QTestString;", "Ltest.TestString;").contains(
+		((ILocalVariable)elements[0]).getTypeSignature()));
 	assertFalse(((ILocalVariable)elements[0]).isParameter());
 }
 // https://bugs.eclipse.org/bugs/show_bug.cgi?id=144858
@@ -1509,10 +1508,9 @@ public void testDuplicateLocals2() throws JavaModelException {
 			elements
 		);
 
-	assertEquals(
-			"Unexpected type",
-			"QTestException;",
-			((ILocalVariable)elements[0]).getTypeSignature());
+	assertTrue("Unexpected type",
+		Set.of("QTestException;", "Ltest.TestException;").contains(
+		((ILocalVariable)elements[0]).getTypeSignature()));
 }
 // https://bugs.eclipse.org/bugs/show_bug.cgi?id=144858
 public void testDuplicateLocals3() throws JavaModelException {
@@ -1548,10 +1546,9 @@ public void testDuplicateLocals3() throws JavaModelException {
 			elements
 		);
 
-	assertEquals(
-			"Unexpected type",
-			"QTestString;",
-			((ILocalVariable)elements[0]).getTypeSignature());
+	assertTrue("Unexpected type",
+		Set.of("QTestString;", "Ltest.TestString;").contains(
+			((ILocalVariable)elements[0]).getTypeSignature()));
 }
 // https://bugs.eclipse.org/bugs/show_bug.cgi?id=144858
 public void testDuplicateLocals4() throws JavaModelException {
@@ -1589,10 +1586,9 @@ public void testDuplicateLocals4() throws JavaModelException {
 			elements
 		);
 
-	assertEquals(
-			"Unexpected type",
-			"QTestString;",
-			((ILocalVariable)elements[0]).getTypeSignature());
+	assertTrue("Unexpected type",
+		Set.of("QTestString;", "Ltest.TestString;").contains(
+		((ILocalVariable)elements[0]).getTypeSignature()));
 }
 // https://bugs.eclipse.org/bugs/show_bug.cgi?id=144858
 public void testDuplicateLocals5() throws JavaModelException {
@@ -1630,10 +1626,9 @@ public void testDuplicateLocals5() throws JavaModelException {
 			elements
 		);
 
-	assertEquals(
-			"Unexpected type",
-			"QTestString;",
-			((ILocalVariable)elements[0]).getTypeSignature());
+	assertTrue("Unexpected type",
+		Set.of("QTestString;", "Ltest.TestString;").contains(
+		((ILocalVariable)elements[0]).getTypeSignature()));
 }
 // https://bugs.eclipse.org/bugs/show_bug.cgi?id=165662
 public void testDuplicateLocalsType1() throws JavaModelException {

diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/TypeResolveTests.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/TypeResolveTests.java
@@ -16,6 +16,7 @@
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;
 
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -1399,7 +1400,9 @@ public void test531046g() throws CoreException, IOException {
 		IJavaElement[] elements = unit.codeSelect(source.lastIndexOf(select), select.length());
 		assertEquals("should not be empty", 1, elements.length);
 		ILocalVariable variable = (ILocalVariable) elements[0];
-		assertEquals("incorrect type", "&QCharSequence;:QComparable<QString;>;", variable.getTypeSignature());
+		assertTrue("incorrect type",
+			Set.of("&QCharSequence;:QComparable<QString;>;", "&Ljava.lang.CharSequence;:Ljava.lang.Comparable<Ljava.lang.String;>;").contains(
+			variable.getTypeSignature()));
 	} finally {
 		deleteProject("P");
 	}
@@ -1424,7 +1427,9 @@ public void test531046h() throws CoreException, IOException {
 		IJavaElement[] elements = unit.codeSelect(source.lastIndexOf(select), select.length());
 		assertEquals("should not be empty", 1, elements.length);
 		ILocalVariable variable = (ILocalVariable) elements[0];
-		assertEquals("incorrect type", "&QCharSequence;:QComparable<QString;>;", variable.getTypeSignature());
+		assertTrue("incorrect type",
+			Set.of("&QCharSequence;:QComparable<QString;>;", "&Ljava.lang.CharSequence;:Ljava.lang.Comparable<Ljava.lang.String;>;").contains(
+			variable.getTypeSignature()));
 	} finally {
 		deleteProject("P");
 	}

diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ASTHolderCUInfo.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/ASTHolderCUInfo.java
@@ -19,7 +19,7 @@
 import org.eclipse.jdt.core.dom.CompilationUnit;
 
 public class ASTHolderCUInfo extends CompilationUnitElementInfo {
-	int astLevel;
+	public int astLevel;
 	boolean resolveBindings;
 	int reconcileFlags;
 	Map<String, CategorizedProblem[]> problems = null;