Editor content out of sync and report false compilation errors

[testforstephen]

The screenshot below shows an error on some non-existent code. The actual problem code was removed a few seconds ago, but the Java language server did not update the document properly. To fix the error, you can save the file and then type some empty lines. This issue happens randomly and does not occur every time.

After some debugging, I found out this is a regression introduced by #2320. That PR changed the code action calculator SimilarElementsRequestor.java so that the code action calculator will update the working copy buffer and interferes with the buffer sync in didChange handler.

To reproduce the issue consistently, follow these steps:

• Add a breakpoint at

  eclipse.jdt.ls/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/SimilarElementsRequestor.java

  Line 308 in fbd482b

  if (cu.isWorkingCopy()) {

• Open a Java file A.java and type some code until the breakpoint is hit
• Pause the breakpoint and continue typing a few more characters in A.java
• Resume the breakpoint
• Go back to A.java and type some code. You will see weird compilation errors due to the document being out-of-sync

[testforstephen]

After a sync with the original author @CsCherrYY , we found it's safe to revert the changes against the SimilarElementsRequestor#getStaticImportFavorites(...) in the PR Add missing import on paste by CsCherrYY · Pull Request #2320 · eclipse-jdtls/eclipse.jdt.ls (github.com).

Here is the old call hierarchy of SimilarElementsRequestor#getStaticImportFavorites(...) when I checkout to PR #2320. The PasteEventHandler.handlePasteEvent() -> getMissingImportsWorkspaceEdit() -> OrganizeImportsHandler.organizeImports() -> wrapStaticImports() -> addImports() -> SimilarElementsRequestor.getStaticImportFavorites().

Here is the new call hierarchy of SimilarElementsRequestor#getStaticImportFavorites(...) with latest source code. Only code action will call SimilarElementsRequestor#getStaticImportFavorites(...), no calls from OrganizeImportsHandler and PasteEventHandler any more.

This is because a later PR organize imports remove static imports (#2396) · eclipse-jdtls/eclipse.jdt.ls@2e430fd (github.com), which deleted the wrapStaticImports() method from OrganizeImportsHandler. So the previous reason to change SimilarElementsRequestor#getStaticImportFavorites(...) for adding missing import on paste becomes invalid.

Next, let ask the second question why the previous PR wanted to add code to restore the CU buffer in SimilarElementsRequestor#getStaticImportFavorites(...)?

This is related to the implementation of cu.getWorkingCopy(null). Depending on whether the original cu was a primary workingcopy or not, it would either create a new workingcopy or return the old one. For the compilationUnit created from the API JDTUtils.resolveCompilationUnit() (which is the case for most usages such as CodeActionHandler in JDT LS), it's a primary unit and therefore it received a new workingcopy and did not affect the original buffer. However, for the old case of the PasteEventHandler, the cu that it passed was a non-primary tempUnit, which meant that it directly manipulated the original buffer in the static-import calculation. That's why that PR #2320 wanted to restore the previous buffer.

eclipse.jdt.ls/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/SimilarElementsRequestor.java

Lines 275 to 278 in 45aeb8d

	String contents = cu.getBuffer().getContents();
	try {
	newCU= cu.getWorkingCopy(null);
	newCU.getBuffer().setContents(dummyCU.toString());

Based on the analysis above, it's fine to revert the changes back in SimilarElementsRequestor#getStaticImportFavorites(...) since both the code action of static import and adding miss imports on paste still work fine.

[rgrunber]

Doing a comparison between

eclipse.jdt.ls/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/SimilarElementsRequestor.java

Line 259 in 73272b0

public static String[] getStaticImportFavorites(ICompilationUnit cu, final String elementName, boolean isMethod, String[] favorites) throws JavaModelException {

& https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/c4f796f006242a2cb6d9422367d7d6c46fcf9b2d/org.eclipse.jdt.core.manipulation/core%20extension/org/eclipse/jdt/internal/corext/util/JavaModelUtil.java#L1326 :

--- jdt-ls.txt	2023-11-22 10:29:37.696189721 -0500
+++ jdt-core.txt	2023-11-22 10:29:31.259087720 -0500
@@ -1,10 +1,9 @@
 public static String[] getStaticImportFavorites(ICompilationUnit cu, final String elementName, boolean isMethod, String[] favorites) throws JavaModelException {
-		StringBuffer dummyCU= new StringBuffer();
+		StringBuilder dummyCU= new StringBuilder();
 		String packName= cu.getParent().getElementName();
 		IType type= cu.findPrimaryType();
-		if (type == null) {
+		if (type == null)
 			return new String[0];
-		}
 
 		if (packName.length() > 0) {
 			dummyCU.append("package ").append(packName).append(';'); //$NON-NLS-1$
@@ -14,7 +13,6 @@
 		dummyCU.append("\n}\n }"); //$NON-NLS-1$
 
 		ICompilationUnit newCU= null;
-		String contents = cu.getBuffer().getContents();
 		try {
 			newCU= cu.getWorkingCopy(null);
 			newCU.getBuffer().setContents(dummyCU.toString());
@@ -25,11 +23,9 @@
 				@Override
 				public void accept(CompletionProposal proposal) {
 					if (elementName.equals(new String(proposal.getName()))) {
-						CompletionProposal[] requiredProposals= proposal.getRequiredProposals();
-						for (int i= 0; i < requiredProposals.length; i++) {
-							CompletionProposal curr= requiredProposals[i];
+						for (CompletionProposal curr : proposal.getRequiredProposals()) {
 							if (curr.getKind() == CompletionProposal.METHOD_IMPORT || curr.getKind() == CompletionProposal.FIELD_IMPORT) {
-								result.add(JavaModelUtil.concatenateName(Signature.toCharArray(curr.getDeclarationSignature()), curr.getName()));
+								result.add(concatenateName(Signature.toCharArray(curr.getDeclarationSignature()), curr.getName()));
 							}
 						}
 					}
@@ -45,14 +41,11 @@
 			}
 			requestor.setFavoriteReferences(favorites);
 
-			newCU.codeComplete(offset, requestor);
-			// if cu is working copy, we should restore the contents saved previously.
-			if (cu.isWorkingCopy()) {
-				cu.getBuffer().setContents(contents);
-			}
+			newCU.codeComplete(offset, requestor, new NullProgressMonitor());
+
 			return result.toArray(new String[result.size()]);
 		} finally {
-			if (newCU != null && !cu.isWorkingCopy()) {
+			if (newCU != null) {
 				newCU.discardWorkingCopy();
 			}
 		}

I think your PR basically brings the duplicated method completely in line with JavaModelUtil.getStaticImportFavorites(...), so we should be able to replace its usage.