Fix compile errors for records with custom constructors, update JDT#10358
Fix compile errors for records with custom constructors, update JDT#10358zbynek wants to merge 3 commits into
Conversation
20b2d41 to
1f69840
Compare
|
@niloc132 the JSP test fails with Given JettyLauncher's deprecated status, is this something worth spending time on, or can we just remove the test? EDIT: I think I found a solution, but I'm not sure if it's too hacky. |
| <param-value>%s</param-value> | ||
| </init-param> | ||
| <init-param> | ||
| <param-name>compilerTargetVM</param-name> | ||
| <param-value>%s</param-value> | ||
| </init-param> | ||
| <load-on-startup>3</load-on-startup> | ||
| </servlet>""", jspLevel, jspLevel)); |
There was a problem hiding this comment.
I think you can avoid the repetition here
| <param-value>%s</param-value> | |
| </init-param> | |
| <init-param> | |
| <param-name>compilerTargetVM</param-name> | |
| <param-value>%s</param-value> | |
| </init-param> | |
| <load-on-startup>3</load-on-startup> | |
| </servlet>""", jspLevel, jspLevel)); | |
| <param-value>%1$s</param-value> | |
| </init-param> | |
| <init-param> | |
| <param-name>compilerTargetVM</param-name> | |
| <param-value>%1$s</param-value> | |
| </init-param> | |
| <load-on-startup>3</load-on-startup> | |
| </servlet>""", jspLevel)); |
That said, it would be nice to explicitly escape this string just in case it is truly an arbitrary string
There was a problem hiding this comment.
Simple validation added instead of escaping.
| } | ||
|
|
||
| protected String getJspLevel() { | ||
| return null; |
There was a problem hiding this comment.
Does this mean that JSPs don't work any more in dev mode? Did they before this patch?
There was a problem hiding this comment.
Did they before this patch?
Yes, if you either stick with Java 1.7 syntax or provide your own web.xml.
JSPs don't work any more in dev mode
If you provide your own web.xml (setting Java level to 1.8 or higher), they should still work. Without it, they will fail.
I wasn't sure what we want (could we break something by creating web.xml for projects that don't need it?)
There was a problem hiding this comment.
Okay, so need a release note entry on this, that 1.7 won't work any more, and newer formats require an explicit web.xml entry.
Co-authored-by: Colin Alworth <colin@vertispan.com>
| "package java.util;", | ||
| "public interface Map<K,V> { }"); | ||
|
|
||
| public static final MockJavaResource METHOD_HANDLE = |
There was a problem hiding this comment.
Going over the rest of GWT, I don't see actual emulation for these provided, so how does a normal compile have these (or not need them) while a test must get them from here?
I do see com.google.gwt.dev.javac.JdtCompiler.INameEnvironmentImpl#getLambdaMetafactoryBytes and friends, but nothing that provides MethodHandle or MethodHandles itself, nor StringConcatFactory. Does the newer JDT require these so we'll want general emulation for these, or maybe these were needed for an earlier iteration but no longer (e.g. to try to deal with emulating Record method before Objects was added)?
Updating JDT by itself is enough to fix the issue described in #10045 .
There is also a similar issue with
where JDT provides two canonical constructors:
Skipping the one without a binding prevents a NPE.
Unit tests added for both, minor tweaks needed to make tests pass with new JDT.
Fixes #10045
Relies on gwtproject/tools#41