[#528] remove runtime dependencies to xbase.lib by cdietrich · Pull Request #529 · eclipse-lsp4j/lsp4j

cdietrich · 2021-03-26T06:56:03Z

[#528] remove some runtime dependencies to xbase.lib
Signed-off-by: Christian Dietrich christian.dietrich@itemis.de

cdietrich · 2021-03-26T07:15:02Z

 			val accessorsUtil = new AccessorsProcessor.Util(context)
 			body = '''
-				«ToStringBuilder» b = new «ToStringBuilder»(this);
+				«MoreObjects.ToStringHelper» b = «MoreObjects».toStringHelper(this);


alternatively we may add a copy of ToStringBuilder to our codebase

alternatively we may add a copy of ToStringBuilder to our codebase

I would vouch for that. It would be a pity to lose the nicely formatted output provided by ToStringBuilder for deeply nested structures that are so common in LSP, both when debugging and in error logs. It seems that copying ToStringBuilder would add little if anything to maintenance costs.

I'd do the same. Copy the ToStringBuilder as a package private class.

nixel2007 · 2021-03-26T12:00:16Z

 		val nextToken = in.peek()
 		if (nextToken == JsonToken.STRING) {
-			val List<Either<String, MarkedString>> value = newArrayList(Either.forLeft(in.nextString))
+			val List<Either<String, MarkedString>> value = Lists.newArrayList(Either.forLeft(in.nextString))


when it comes to guava, less is better)

is java 9+

ah, i see. sorry then

de-guavaize would be a second step.

did replace it with pure java

cdietrich · 2021-03-26T12:16:45Z

 		val nextToken = in.peek()
 		if (nextToken == JsonToken.STRING) {
-			val List<Either<String, MarkedString>> value = newArrayList(Either.forLeft(in.nextString))
+			val List<Either<String, MarkedString>> value = new ArrayList<Either<String, MarkedString>>()


replaced it with a bit more verbose pure java

If verbosity is a concern, you could remove the type arguments from the constructor invocation. They would be inferred. But I doubt it matters much.

dhuebner · 2021-03-26T12:36:57Z

+			if (purified !== null) {
+				method.removeAnnotation(purified)
+			}
+		]


Is that also a part of this issue?

yes it removes the pure annotation

jonahgraham · 2022-08-03T19:35:17Z

What is the state of this PR - is this something you are maintaining on the side for those that need it or does it need to get to completion one day?

cdietrich · 2022-08-03T19:38:09Z

i dont know if it is properly working as expected as where else we use xbase.lib

jonahgraham · 2022-08-03T19:50:05Z

OK - me neither, and I don't know how to tell. I guess if it looks good we can release it and have a trial by fire on it. We are still an incubating project.

cdietrich · 2022-08-03T19:54:59Z

Guess we have to Grep generated code where Xbase and Xtend lib still are used

cdietrich · 2023-02-14T11:52:20Z

@pisv @szarnekow @jonahgraham
i have updated this pr by

also remove the annotation for the adapters TypeAdapterImpl annotation
add copies of ToStringBuilder so we are compatible and dont need guava dependency
moved generator dependency to compileOnly (so no longer maven dependency to it)
added ArchUnit tests to make sure we no longer depend on xbase.lib, xtend.lib, guava and lsp4j.generator at runtime.

what do you think. maybe we can merge this shortly after the release
cc @rgrunber this might be of interest for you on jdt.ls side

https://ci.eclipse.org/lsp4j/job/lsp4j-multi-build/job/cd_issue528/lastSuccessfulBuild/artifact/build/maven-repository/
https://ci.eclipse.org/lsp4j/job/lsp4j-multi-build/job/cd_issue528/lastSuccessfulBuild/artifact/build/p2-repository/

szarnekow

I like this.

cdietrich · 2023-02-14T13:57:11Z

should we also remove NoAnnotationTest or keep it?

szarnekow · 2023-02-14T14:28:47Z

should we also remove NoAnnotationTest or keep it?

I don't think it does any harm but does not add any value either.

szarnekow · 2023-02-14T14:29:11Z

should we also remove NoAnnotationTest or keep it?

I don't think it does any harm but does not add any value either.

Read: inconclusive :)

jonahgraham · 2023-02-14T14:56:19Z

what do you think. maybe we can merge this shortly after the release

Sounds like a good plan, lets just give it a few days in case we need to respin LSP4J for 2022-03 release.

rgrunber · 2023-02-22T18:36:31Z

what do you think. maybe we can merge this shortly after the release cc @rgrunber this might be of interest for you on jdt.ls side

We plan to update to LSP4J 0.20.0 soon, and once that's done we'll probably be able to remove our dependence on xbase.lib as well.

Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>

cdietrich · 2023-03-01T11:42:07Z

@jonahgraham what do you think? ready to merge or wait another week?
didnt follow if you built the patch from master or a dedicate maintenance branch

jonahgraham · 2023-03-01T13:54:59Z

@cdietrich go ahead and merge. I now have the release_0.20 branch if I need to do any further respins for 2023-03

cdietrich · 2023-03-02T10:45:07Z

this pr is merged now. so using the nightly it should be possible to consume without guava, xtext and xtend
cc @rgrunber @mickaelistria

When the dependency on xbase.lib was removed in #529 it put the implicit requirement that there is a class called ToStringBuilder in a sub-package of the xtend class called `util`. This change provides the ToStringBuilder in an util package in the org.eclipse.lsp4j.jsonrpc bundle so that consumers of LSP4J do not need to copy ToStringBuilder into their own source code. Fixes #742

When the dependency on xbase.lib was removed in eclipse-lsp4j#529 it put the implicit requirement that there is a class called ToStringBuilder in a sub-package of the xtend class called `util`. This change provides the ToStringBuilder in an util package in the org.eclipse.lsp4j.jsonrpc bundle so that consumers of LSP4J do not need to copy ToStringBuilder into their own source code. Fixes eclipse-lsp4j#742

When the dependency on xbase.lib was removed in #529 it put the implicit requirement that there is a class called ToStringBuilder in a sub-package of the xtend class called `util`. This change provides the ToStringBuilder in an util package in the org.eclipse.lsp4j.jsonrpc bundle so that consumers of LSP4J do not need to copy ToStringBuilder into their own source code. Fixes #742

cdietrich force-pushed the cd_issue528 branch from aca6eba to c225e9d Compare March 26, 2021 07:14

cdietrich commented Mar 26, 2021

View reviewed changes

cdietrich marked this pull request as draft March 26, 2021 08:13

cdietrich force-pushed the cd_issue528 branch from c225e9d to 5b838e0 Compare March 26, 2021 10:44

nixel2007 reviewed Mar 26, 2021

View reviewed changes

cdietrich force-pushed the cd_issue528 branch from 5b838e0 to 4f5aa44 Compare March 26, 2021 12:15

cdietrich commented Mar 26, 2021

View reviewed changes

dhuebner reviewed Mar 26, 2021

View reviewed changes

jonahgraham mentioned this pull request Mar 26, 2021

Remove dependency on xtext's @Pure and ToStringBuilder #494

Closed

jonahgraham linked an issue Mar 26, 2021 that may be closed by this pull request

Remove dependency on xtext's @Pure and ToStringBuilder #494

Closed

cdietrich force-pushed the cd_issue528 branch from 4f5aa44 to 2f1f0af Compare March 27, 2021 05:57

cdietrich force-pushed the cd_issue528 branch from 2f1f0af to d961ebb Compare April 12, 2021 11:59

cdietrich force-pushed the cd_issue528 branch from d961ebb to 17aab08 Compare May 28, 2021 10:42

cdietrich force-pushed the cd_issue528 branch from 17aab08 to bd3e73e Compare July 29, 2021 12:10

cdietrich force-pushed the cd_issue528 branch from bd3e73e to 450204b Compare December 6, 2021 10:59

cdietrich force-pushed the cd_issue528 branch from 450204b to f97666e Compare January 13, 2022 13:23

cdietrich force-pushed the cd_issue528 branch from f97666e to 8a04deb Compare February 9, 2022 06:45

cdietrich force-pushed the cd_issue528 branch from 8a04deb to abb3ef3 Compare April 29, 2022 08:09

cdietrich force-pushed the cd_issue528 branch from abb3ef3 to 522cb11 Compare May 20, 2022 05:46

cdietrich force-pushed the cd_issue528 branch from 522cb11 to bc06eb0 Compare June 21, 2022 04:54

cdietrich force-pushed the cd_issue528 branch from bc06eb0 to 698a84a Compare July 21, 2022 08:28

cdietrich force-pushed the cd_issue528 branch 2 times, most recently from cc436c1 to 22ee77d Compare August 5, 2022 04:13

cdietrich marked this pull request as ready for review August 5, 2022 04:32

cdietrich force-pushed the cd_issue528 branch 2 times, most recently from bda8bf0 to b48f059 Compare September 22, 2022 09:27

cdietrich changed the title ~~[#528] remove some runtime dependencies to xbase.lib~~ [#528] remove runtime dependencies to xbase.lib Feb 14, 2023

szarnekow approved these changes Feb 14, 2023

View reviewed changes

cdietrich force-pushed the cd_issue528 branch 2 times, most recently from 8826bbe to a8874df Compare February 17, 2023 12:57

cdietrich force-pushed the cd_issue528 branch 2 times, most recently from fcf9674 to ebce1f9 Compare February 22, 2023 17:56

cdietrich mentioned this pull request Feb 28, 2023

Upgrade Jakarta websocket from 2.0.0 to 2.1.0 #661

Closed

cdietrich added 4 commits March 1, 2023 12:40

[#528] remove some runtime dependencies to xbase.lib

f7d46bf

Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>

use generator as compile only dependency

404128b

Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>

add and use copy of ToStringBuilder from xbase.lib

086ffce

Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>

added archunit tests for no xbase/xtend/guava dep

a2e0f68

Signed-off-by: Christian Dietrich <christian.dietrich@itemis.de>

cdietrich force-pushed the cd_issue528 branch from ebce1f9 to a2e0f68 Compare March 1, 2023 11:40

cdietrich merged commit 4bad511 into main Mar 1, 2023

cdietrich deleted the cd_issue528 branch March 1, 2023 14:17

jonahgraham added this to the 0.21.0 milestone May 17, 2023

cdietrich mentioned this pull request Nov 8, 2023

Consider migrating away from guava library (or make its use internal) #771

Closed

jonahgraham mentioned this pull request Feb 12, 2024

Supply a ToStringBuilder in common location #798

Merged

Conversation

cdietrich commented Mar 26, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahgraham commented Aug 3, 2022

Uh oh!

cdietrich commented Aug 3, 2022

Uh oh!

jonahgraham commented Aug 3, 2022

Uh oh!

cdietrich commented Aug 3, 2022

Uh oh!

cdietrich commented Feb 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szarnekow left a comment

Choose a reason for hiding this comment

Uh oh!

cdietrich commented Feb 14, 2023

Uh oh!

szarnekow commented Feb 14, 2023

Uh oh!

szarnekow commented Feb 14, 2023

Uh oh!

jonahgraham commented Feb 14, 2023

Uh oh!

rgrunber commented Feb 22, 2023

Uh oh!

cdietrich commented Mar 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahgraham commented Mar 1, 2023

Uh oh!

cdietrich commented Mar 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cdietrich commented Feb 14, 2023 •

edited

Loading

cdietrich commented Mar 1, 2023 •

edited

Loading