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

Dto Generation is broken for Formatter Options #8557

Closed
svor opened this issue Feb 1, 2018 · 1 comment
Closed

Dto Generation is broken for Formatter Options #8557

svor opened this issue Feb 1, 2018 · 1 comment
Assignees
Labels
kind/bug Outline of a bug - must adhere to the bug report template. sprint/current target/branch Indicates that a PR will be merged into a branch other than master.

Comments

@svor
Copy link
Contributor

svor commented Feb 1, 2018

After applying code format (Edit -> Format or Alt+Shift+L) have an error

SEVERE: Unknown DTO type class org.eclipse.lsp4j.FormattingOptions
java.lang.IllegalArgumentException: Unknown DTO type class org.eclipse.lsp4j.FormattingOptions
	at Unknown.Xo(Throwable.java:66)
	at Unknown.ep(Exception.java:29)
	at Unknown.ip(RuntimeException.java:29)
	at Unknown.yYd(IllegalArgumentException.java:29)
	at Unknown.T2g(DtoFactory.java:113)
	at Unknown.Yfl(DtoFactory.java:38)
	at Unknown._fl(LanguageServerFormatter.java:137)
	at Unknown.O9g(OrionEditorPresenter.java:949)
	at Unknown.x5e(FormatterAction.java:55)
	at Unknown.Wvk(PopupMenu.java:562)
	at Unknown.jwk(PopupMenu.java:719)
	at Unknown.FBd(DOM.java:1481)
	at Unknown.iDd(DOMImplStandard.java:317)
	at Unknown._p(Impl.java:309)
	at Unknown.cq(Impl.java:361)
	at Unknown.anonymous(Impl.java:78)
	at Unknown.bC(StringValidator.java:66)
	at Unknown.aC(StringValidator.java:49)
	at Unknown.OB(RequestBuilder.java:292)
	at Unknown.VMk(AsyncRequest.java:72)
	at Unknown.ZMk(MachineAsyncRequestFactory.java:129)
	at Unknown.anonymous(JsPromise.java:116)
	at Unknown.Vll(OccurrencesProvider.java:92)
	at Unknown.Xll(OccurrencesProvider.java:85)
	at Unknown.anonymous(JsPromise.java:68)
	at Unknown.bC(StringValidator.java:66)
	at Unknown.aC(StringValidator.java:49)
	at Unknown.OB(RequestBuilder.java:292)
	at Unknown.VMk(AsyncRequest.java:72)
	at Unknown.ZMk(MachineAsyncRequestFactory.java:129)
	at Unknown.anonymous(JsPromise.java:116)
	at Unknown.Vll(OccurrencesProvider.java:92)
	at Unknown.Xll(OccurrencesProvider.java:85)
	at Unknown.anonymous(JsPromise.java:68)
	at Unknown.bC(StringValidator.java:66)
	at Unknown.aC(StringValidator.java:49)
	at Unknown.OB(RequestBuilder.java:292)
	at Unknown.VMk(AsyncRequest.java:72)
	at Unknown.ZMk(MachineAsyncRequestFactory.java:129)
	at Unknown.anonymous(JsPromise.java:116)
	at Unknown.bC(StringValidator.java:66)
	at Unknown.aC(StringValidator.java:49)
	at Unknown.OB(RequestBuilder.java:292)
	at Unknown.VMk(AsyncRequest.java:72)
	at Unknown.ZMk(MachineAsyncRequestFactory.java:129)
	at Unknown.anonymous(JsPromise.java:116)
	at Unknown.Vll(OccurrencesProvider.java:92)
	at Unknown.Xll(OccurrencesProvider.java:85)
	at Unknown.anonymous(JsPromise.java:68)
	at Unknown.Vll(OccurrencesProvider.java:92)
	at Unknown.Xll(OccurrencesProvider.java:85)
	at Unknown.anonymous(JsPromise.java:68)
	at Unknown.Vll(OccurrencesProvider.java:92)
	at Unknown.Xll(OccurrencesProvider.java:85)
	at Unknown.anonymous(JsPromise.java:68)

@svor svor added kind/bug Outline of a bug - must adhere to the bug report template. team/enterprise target/branch Indicates that a PR will be merged into a branch other than master. labels Feb 1, 2018
@svor svor self-assigned this Feb 13, 2018
@jonahgraham
Copy link
Contributor

The problem is the LSP4J changed FormattingOptions to be a Map.

Therefore first problem is that org.eclipse.che.api.languageserver.generator.DtoGenerator.generate(File, String, String, String[], String[]) excludes FormattingOptions because its class hierarchy contains LinkedHashMap which is not in one of the listed packages.

You can work around the first problem by doing:

diff --git a/plugins/plugin-languageserver/che-plugin-languageserver-ide/pom.xml b/plugins/plugin-languageserver/che-plugin-languageserver-ide/pom.xml
index e97c25e998..682302578f 100644
--- a/plugins/plugin-languageserver/che-plugin-languageserver-ide/pom.xml
+++ b/plugins/plugin-languageserver/che-plugin-languageserver-ide/pom.xml
@@ -222,6 +222,9 @@
                         <package>org.eclipse.che.api.languageserver.shared.event</package>
                         <package>org.eclipse.che.api.languageserver.shared.model</package>
                     </dtoPackages>
+                    <classes>
+                        <class>org.eclipse.lsp4j.FormattingOptions</class>
+                    </classes>
                     <outputDirectory>${dto-generator-out-directory}</outputDirectory>
                     <genClassName>org.eclipse.che.api.languageserver.shared.dto.DtoClientImpls</genClassName>
                     <impl>client</impl>

which forces the Dto generation to happen and I suppose is there for this kind of situation. But then you hit the second problem.

The second problem is possibly just cosmetic, I am not sure. But the to/from JSON in the generated FormattingOptions does not look like it will work because it will try to create a JSON object from an Either3 directly. This means updating the generator to support this new LSP4J use case.

The simpler option

The alternative solution is to simply not use a Dto for FormattingOptions at all. As FormattingOptions is never used as a LS parameter directly and always used as a field in other types (e.g. DocumentOnTypeFormattingParams.setOptions) then you could just do this because the to/fromJSon of the containing types don't delegate to FormattingOptions and rather handle the FormattingOptions as a Map.

diff --git a/plugins/plugin-languageserver/che-plugin-languageserver-ide/src/main/java/org/eclipse/che/plugin/languageserver/ide/editor/LanguageServerFormatter.java b/plugins/plugin-languageserver/che-plugin-languageserver-ide/src/main/java/org/eclipse/che/plugin/languageserver/ide/editor/LanguageServerFormatter.java
index fe0c8f92e6..1ae5904947 100644
--- a/plugins/plugin-languageserver/che-plugin-languageserver-ide/src/main/java/org/eclipse/che/plugin/languageserver/ide/editor/LanguageServerFormatter.java
+++ b/plugins/plugin-languageserver/che-plugin-languageserver-ide/src/main/java/org/eclipse/che/plugin/languageserver/ide/editor/LanguageServerFormatter.java
@@ -188,7 +188,7 @@ public class LanguageServerFormatter implements ContentFormatter {
   }
 
   private FormattingOptions getFormattingOptions() {
-    FormattingOptions options = dtoFactory.createDto(FormattingOptions.class);
+    FormattingOptions options = new FormattingOptions();
     options.setInsertSpaces(Boolean.parseBoolean(getEditorProperty(EditorProperties.EXPAND_TAB)));
     options.setTabSize(Integer.parseInt(getEditorProperty(EditorProperties.TAB_SIZE)));
     return options;

I'll submit a PR with the second solution.

@tsmaeder tsmaeder changed the title Formatter is broken Dto Generation is broken for Formatter Options Feb 23, 2018
@tsmaeder tsmaeder mentioned this issue Mar 6, 2018
22 tasks
@svor svor closed this as completed Mar 16, 2018
@tsmaeder tsmaeder mentioned this issue Mar 23, 2018
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template. sprint/current target/branch Indicates that a PR will be merged into a branch other than master.
Projects
None yet
Development

No branches or pull requests

4 participants