Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary files about "Change Signature refactoring" #2571

Merged
merged 2 commits into from
Apr 17, 2023

Conversation

CsCherrYY
Copy link
Contributor

The PR is a follow-up of eclipse-jdt/eclipse.jdt.ui#428

@CsCherrYY CsCherrYY requested a review from rgrunber April 4, 2023 02:51
@rgrunber
Copy link
Contributor

rgrunber commented Apr 4, 2023

I'm guessing the test failures are due to Java 20 support having been merged in the 4.28-I-builds ? So we probably need to do a release, then merge #2557 , and then we can merge this PR.

@fbricon
Copy link
Contributor

fbricon commented Apr 17, 2023

test this please

- Use the files directly from jdt.core.manipulation

Signed-off-by: Shi Chen <chenshi@microsoft.com>
@rgrunber
Copy link
Contributor

rgrunber commented Apr 17, 2023

Hold on, there's a small problem here. Most of the classes (not all) have a 2nd copy already! The differences I think are mostly whitespace related but we should verify. So I guess we're saving ~12k lines 😮

 100 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/rename/CuCollectingSearchRequestor.java
  98 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/CuCollectingSearchRequestor.java

 105 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/ExceptionInfo.java
 103 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/ExceptionInfo.java

  235 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/ParameterInfo.java
  232 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/ParameterInfo.java

  62 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/ReturnTypeInfo.java
  61 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/ReturnTypeInfo.java

  43 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/StubTypeContext.java
  42 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/StubTypeContext.java

  875 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/TypeContextChecker.java
  864 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/TypeContextChecker.java

  551 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/rename/RippleMethodFinder2.java
  539 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/rename/RippleMethodFinder2.java

  48 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/BodyUpdater.java
  48 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/structure/BodyUpdater.java

  3102 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/ChangeSignatureProcessor.java
  2914 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeSignatureProcessor.java

  50 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corext/refactoring/IDefaultValueAdvisor.java
  49 ./org.eclipse.jdt.ls.core/src/org/eclipse/jdt/internal/corext/refactoring/structure/IDefaultValueAdvisor.java

Update: Let me just verify that the classes being removed are all functionally identical to their duplicate.

Update 2: I've added a separate commit in the event that we discover some strange difference in behaviour, we can at least revert the part I introduced, since everything was fine before it. As far as I can tell, there isn't a difference though.

- Also remove BodyUpdater & CollectingSearchRequestor

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber
Copy link
Contributor

test this please.

@rgrunber rgrunber merged commit d0ef558 into eclipse-jdtls:master Apr 17, 2023
4 checks passed
@CsCherrYY CsCherrYY deleted the cs-tp branch April 18, 2023 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants