Git Product home page Git Product logo

Comments (2)

testforstephen avatar testforstephen commented on September 23, 2024 2

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().

image

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.

image

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.

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.

from eclipse.jdt.ls.

rgrunber avatar rgrunber commented on September 23, 2024

Doing a comparison between

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.

from eclipse.jdt.ls.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.