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

[21] JEP 430 String Templates (First Preview) #544

Closed
Tracked by #890 ...
mpalat opened this issue Nov 17, 2022 · 18 comments · Fixed by #1513
Closed
Tracked by #890 ...

[21] JEP 430 String Templates (First Preview) #544

mpalat opened this issue Nov 17, 2022 · 18 comments · Fixed by #1513
Assignees
Labels
enhancement New feature or request preview-feature Preview Feature
Milestone

Comments

@mpalat
Copy link
Contributor

mpalat commented Nov 17, 2022

JEP 430 at https://openjdk.org/jeps/430
Ref https://cr.openjdk.java.net/~gbierman/jep430/latest for more details on LS changes

At the time of creating this issue, this is not yet targeted but it may be targeted - keeping this in the radar for 20

@mpalat mpalat added the enhancement New feature or request label Nov 17, 2022
@mpalat mpalat added this to the BETA_JAVA20 milestone Nov 17, 2022
@mpalat mpalat changed the title [20] JEP 430 String Templates [20] JEP 430 String Templates (First Preview) Nov 17, 2022
@mpalat mpalat added the preview-feature Preview Feature label Nov 17, 2022
@mpalat mpalat modified the milestones: BETA_JAVA20, BETA_JAVA21 Mar 22, 2023
@mpalat
Copy link
Contributor Author

mpalat commented Mar 22, 2023

This was moved out of JAVA 20 and hence moving t0 BETA_JAVA21

@mpalat mpalat changed the title [20] JEP 430 String Templates (First Preview) [21] JEP 430 String Templates (First Preview) Mar 22, 2023
@jarthana jarthana self-assigned this Apr 18, 2023
@jarthana
Copy link
Member

As per the jdk issue, this is targeted now for 21

@jarthana
Copy link
Member

jarthana commented May 4, 2023

The first goal should be to come up with scanner changes that allow us to recognize and differentiate String literals and templates, preferably using the same code. As per the JEP, I see that the only difference in a template being the presence of an embedded expression that is of the format \{name}.

I have a draft here - https://github.com/jarthana/eclipse.jdt.core/tree/jep430_grammar
But putting it on hold since the latest JDK 21 doesn't seem to have the TemplateProcessor available.

@mpalat mpalat modified the milestones: BETA_JAVA21, 4.30 M3 Aug 21, 2023
@mpalat
Copy link
Contributor Author

mpalat commented Aug 21, 2023

retargeting out of BETA_JAVA21
(opportunistic)

@akurtakov
Copy link
Contributor

akurtakov commented Oct 19, 2023

Initial work on the topic seems to have started in #1494

@jarthana
Copy link
Member

Initial work on the topic seems to have started in #1494

Yep, I am fairly close to completion of this, although I am still studying and comparing two possible approaches wrt grammar. The PR is probably not updated. Will keep posted.

@paulholder
Copy link

Is there another PR where the changes will be made to have the now "globally known" STR, FMT and RAW types defined, exposed into the code if the preview feature is enabled? Also, those constants presumably need to me reserved now and a warning issued if they're used when the preview feature is not enabled.

@jarthana
Copy link
Member

On suggestion from @srikanth-sankaran (after I ran into grammar issues with the previous PR), I experimented an alternate approach. The approach is to, on the first pass, scan the entire string template and return a TokenNameStringTemplate. During this step, the scanner doesn't bother with the embedded expressions, except for marking the boundaries of the top level embedded expressions. Later, when the parser encounter the said token, we will spawn new parser+scanner with the boundaries stored and

  1. Create the string fragments
  2. Invoke Parser#parseExpression() and create embedded expressions for each
  3. Put them all together and create the string template.

This is a bit more involved on the scanner side, but makes the grammar lot simpler and avoid the need for multiple synthetic tokens. One of the challenges is getting the format of the text block working, especially when there is a nested text block involved. The JEP doesn't seem to say anything about the influence of the indentation of the outer block on the inner text block.

@srikanth-sankaran
Copy link
Contributor

"The JEP doesn't seem to say anything about the influence of the indentation of the outer block on the inner text block" -- is this a concern only for the alternative design where the scanner returns a single string template token ??

@jarthana
Copy link
Member

"The JEP doesn't seem to say anything about the influence of the indentation of the outer block on the inner text block" -- is this a concern only for the alternative design where the scanner returns a single string template token ??

I think it will be something we need to take care of in both designs. The fact that the formatting of a text block can only be done with all lines at hand (and not line by line) means it is even more difficult with the other design where we may be creating multiple text blocks for each fragment. I think this one here gives us more control on the formatting.

@szarnekow
Copy link
Contributor

The approach is to, on the first pass, scan the entire string template and return a TokenNameStringTemplate.

Will this work with text blocks that have embedded expressions that in turn use text blocks?

@jarthana
Copy link
Member

The approach is to, on the first pass, scan the entire string template and return a TokenNameStringTemplate.

Will this work with text blocks that have embedded expressions that in turn use text blocks?

It does, with the known issue of formatting I mentioned. This is WIP, but feel free to try out. I have some tests already added in StringTemplateTest

@jarthana
Copy link
Member

It does, with the known issue of formatting I mentioned. This is WIP, but feel free to try out. I have some tests already added in StringTemplateTest

This is in decent shape for review. However, there are still known issues or design decisions that need to be discussed.

  1. The grammar only works for Name '.' Template. However, this is supposed to work for any expression. This requires some not so simply grammar tweaks.
  2. Javac seems to simplify a template expression with STR to a simple string concatenation before code generation. I haven't bothered to do that as I don't know the implication. The current patch creates a method invocation with the processor as the receiver and the template as the only argument. This generic way should work with all kind of processors.
  3. There's simply too much happening when it comes to handling the template/string delimiters for text blocks inside scanForTextBlock(). This is something we have been living with since the introduction of text blocks. This was cloned and modified from scanForStringLiteral(). Perhaps, when times permits, we should merge these two.
  4. Since the last patch, I also made some changes to completely strip off the text of the embedded expression from the text of the outer text block. This ensures that the formatting of the enclosing text block is not affected by the embedded expressions.

@paulholder
Copy link

alternate approach. The approach is to, on the first pass, scan the entire string template and return a TokenNameStringTemplate.

In relation to the question about the importance of spaces, this was covered in the text blocks https://openjdk.org/jeps/378 that are components that StringTemplates are made from. I think your approach to scanning should be to first recognize the Text Block, then scan it for any indication of \{} internally, and if found, change it from a Text Block to a StringTemplate, and then further process that into the proper code to define the StringTemplate instance.

Also someone made reference to just STR but RAW and FMT are also globally defined in the Java code, and users can define their own.

@srikanth-sankaran
Copy link
Contributor

@jarthana can you share some examples of text block string templates where the formatting/indentation is unclear - also documenting what Javac is doing.

I can review the relevant portions of the spec to see if I can help clarify

@jarthana
Copy link
Member

jarthana commented Nov 1, 2023

@jarthana can you share some examples of text block string templates where the formatting/indentation is unclear - also documenting what Javac is doing.

I can review the relevant portions of the spec to see if I can help clarify

String s = STR."""
....xyz
....\{
.."""
..abc
..def
.."""}
....""";

Javac produces this:

xyz
abc
def

Question is, how does the line that contains the beginning of the inner text block (""") affect the indentation of the outer block?

@jarthana
Copy link
Member

jarthana commented Nov 2, 2023

alternate approach. The approach is to, on the first pass, scan the entire string template and return a TokenNameStringTemplate.

In relation to the question about the importance of spaces, this was covered in the text blocks https://openjdk.org/jeps/378 that are components that StringTemplates are made from. I think your approach to scanning should be to first recognize the Text Block, then scan it for any indication of {} internally, and if found, change it from a Text Block to a StringTemplate, and then further process that into the proper code to define the StringTemplate instance.

Yes, that's what this patch does. Just that, instead of grammar taking care of it, it's mostly hand-written. I am not sure how that (handling the rules in the grammar) will handle delimiters as unicode characters.

Also someone made reference to just STR but RAW and FMT are also globally defined in the Java code, and users can define their own.
The reference, that I made, was about how Javac seems to be handling STR specially, rather than in a generic way. This patch doesn't differentiate between a STR or other kind of processors.

Other quirk, or perhaps even a less than ideal thing to do, is that when we encounter anoter string literal (or text block) inside an embedded expression, we just spawn another instance of scanner and invoke scanForStringLiteral(). This is just to scan past that literal and the literal is discarded. Perhaps, wasteful and can be optimized.

@mpalat
Copy link
Contributor Author

mpalat commented Nov 13, 2023

@jarthana - please ref - https://cr.openjdk.org/~gbierman/jep459/latest/ captured in #1397 - to track the delta and probably. fyi.

jarthana added a commit to jarthana/eclipse.jdt.core that referenced this issue Dec 6, 2023
jarthana added a commit to jarthana/eclipse.jdt.core that referenced this issue Dec 13, 2023
mpalat added a commit that referenced this issue Dec 19, 2023
* configure default output.. = bin/

* JavaSearchScope: improve encloses() performance  #474

* [test] remove outdated latestBREE project

* fix some ecj markers

Especially after moving files to compiler.batch - which has no resource
warnings - the @SuppressWarnings("resource") is not used - leading to a
marker

* Single async "Synchronizing projects" Job #419

Scheduling multiple times "while the job is running, the job will still
only be rescheduled once" (javadoc) so there will be always only a
single job running.
Using a Set prevents touching projects multiple times.

#419

Manually tested by changing Compiler Building options "Circular
dependencies", which triggers a new build.

* [test] fix AbstractJavaModelTests #1333

avoid asynchronous refresh. Implementation taken from
org.eclipse.core.tests.resources.refresh.RefreshProviderTest.joinAutoRefreshJobs()

#1333

* TestVerifier: never wait 100ms

improve test time, accurate timeout

* [performance] ClassFileReader: use already open Zip File

instead of creating new ZipFile instance.

also:
* use Files.readAllBytes
* removed unused code
* faster toUri avoiding isDirectory() check for the JARs which are known
to be no directory

improves performance of read() on windows by factor 2
tested with java reference search to java.lang.Object

* Stop skipping compare-with-baseline for jdt.annotation v2

One less thing that has to be manually tracked and done.

* Version bump for jdt.annotation

Pointed by
https://download.eclipse.org/eclipse/downloads/drops4/I20231130-0020/buildlogs/reporeports/reports/versionChecks.html

* Fix github urls in NOTICE file

* Use try-with-resource and enable warning if not

org.eclipse.jdt.core.compiler.problem.explicitlyClosedAutoCloseable=warning

StringWriter does not need flush() or close() and can not throw
IOException.

Tests excluded.

To get rid of boiler plate code.

* version bumps

* [21] JEP 430 String Templates (#1513)

Grammar, compiler AST, resolution and code generation changes

* Javadoc: fix unclosed <code>

* break from loop within labeled block causes loss of nullness info (#1660)

fixes #1659

* Fixes incorrect Javadoc after StringBuffer to StringBuilder change

* NPE in ASTRewriteFlattener as return value of GuardedPattern.getPattern() is null  (#1647)

NPE in ASTRewriteFlattener as return value of
GuardedPattern.getPattern() is null

* Adding pomless build to JDT core

This enables pomless builds for JDT coreand removes the simple pom
files.

Future commits can reduce the usage of pom files further. This might
require enhancements in pomless builds to specify the test classes and
suites eclipse-tycho/tycho#3105

* Using Simplify lambda expression and method reference syntax cleanup on
core

Using the JDT UI "Simplify lambda expression and method reference
syntax" clean-up on jdt.core.

* Internal compiler error: ArrayIndexOutOfBoundsException in latest i build (#1664)


fixes #1661

* Using Simplify lambda expression and method reference syntax cleanup on
all plug-ins except core

Using the JDT UI "Simplify lambda expression and method reference
syntax" clean-up on all plug-ins except jdt.core.

* Using short-circuit in IncrementalImageBuilder

* Re-normalize line-endings in git of all files to Linux style ("\n")

Some files were checked-in into git having windows style line
endings (\r\n). This is in general not wanted because it can cause
modified files without any difference in git-staging on Windows if
auto-crlf is enabled.

To re-normalize line endings of all files use the following command
(including dot):

git add --renormalize .

* Bump bundle dependencies to trigger a rebuild / fix SDK build error

The change in bytecode of FullSourceWorkspaceBuildTests is due the
constant value change of
o.e.jdt.internal.compiler.parser.TerminalTokens.TokenNameEOF coming
from #1513.

Fixes eclipse-platform/eclipse.platform.releng.aggregator#1617

* False positive "Dead code" compiler error reported on org.eclipse.pde.internal.core.util.PDEJavaHelper.getExternalPackageFragment(String, String) (#1671)

fixes #1667

* Use diamond operator in jdt.core repo

Using the JDT UI clean-up, this removes the redundant type information.

Also activating
org.eclipse.jdt.core.compiler.problem.redundantSpecificationOfTypeArguments=warning
The rest of the changes in the prefs file were done by the tooling, not
by
selecting anther value.

* Improve constructor completions inside method invocations Fix #1587 (#1588)

* Try-with-resource clean-up in JDT core

Using the JDT UI try-with-resource clean-up on core.
Also manually inlining the declaration of multiple places into the try()
clause.

* Make inner classes static where possible in JDT core

Running the JDT performnace clean-up "Make inner classes static where
possible" in JDT core

* Use Use lazy logical operator (&& and ||)

Running the performance clean-up "Use lazy logical operator (&& and ||)"
on JDT core

* Using Integer.toString directly in Disassembler

It is also a JDT UI performance clean-up but this clean-up found only
one occurrence.

* use Path.of() to avoid 'Potential resource leak' warnings

* @NonNullByDefault does not work for type arguments of a local type (#1694)

fixes #1693

* ClassCastException during code completion on Annotation (#1696)

fixes #1440

Also fixes noise from #1662

* CompilationUnitResolver: Name the CU that causes Exception (#1690)

for example during Cleanup
eclipse-jdt/eclipse.jdt.ui#950

Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de>

* Code selection support for String template expressions (#1699)

Fixes  #1685

* Report error if string template is used without preview option enabled (#1697)

Improves the fix for #544

* ECJ crashes when an embedded expression contains broken code (#1702)

Set haltOnSyntaxError when parsing for embedded expressions. Also using the correct delimiters for text blocks in printExpression() methods.

* Selection model tests for string templates (#1704)

* tests: enable discouragedReference=warning, declare x-friends

to get rid of warnings during build (does not respect the jdt
preferences)

* Javadoc format fixes

Contributes to
eclipse-platform/eclipse.platform.releng.aggregator#1531

* Performance: Add public API for Batch Reads in UI - closes #1614

During Batches:
* cache Zip Files
* enable JavaModelManager.temporaryCache

Also:
* uses Batch Reads during some core actions that benefit from it.
* adds trace logging when caching is missing.

#1614

* Javadoc format fixes (part 2)

Contributes to
eclipse-platform/eclipse.platform.releng.aggregator#1531.
It's a pity that this takes multiple cycles but fixing one thing from
the log uncovers the next.

* ECJ 3.36.0 regression: The type 'E extends java.lang.Exception' is not a valid substitute for the type parameter 'E extends java.lang.@nonnull Exception' (#1708)

fixes #1691

+ also slightly updates NullAnnotationTests18 as NullAnnotationTests21

* Implement support for code completion inside embedded expression of (only) string templates (not text block templates) (#1712)

* Implement support for code completion inside embedded expression of
(only) string templates (not text block templates)

* Fixes #1711

* Fixes #1641 (#1713)

* Add new testcase #1701 (#1705)

* Content assist does not propose overrides in records (#1718)

* Fixes #1095

* JavaModelManager: lazy initialize TouchJob #1720

#1720

* Run the JavaCoreStandaloneTest in the build

#1720

* Bogus error about return expression involving pattern matching (#1731)

Fixes #1726

* Compiler fails to recognize an exhaustive switch (#1733)

*  Fixes #1725

* [21] Processed string templates falsely contain backslash characters (#1730)

#1719

* 1641_enum_further_fixes (#1739)

* Fix one too many pops arising from string concat invoke dynamic (#1740)

* Fix one too many pops arising from string concat invoke dynamic

* Fixes #1394

* Wrong placement of exception range closure results in AIOOB (#1744)

Fixes #1686

* Improper warning - Potential null pointer access (#1469)

Fixes #1461
Sets the flow info reach mode to FlowInfo.UNREACHABLE_BY_NULLANALYSIS
after 'Object o = null;if (Objects.isNull(o)) return;', 'Object o = "";if (Objects.nonNull(o)) return;

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

* Touch bundles affected by the changed ecj version

See #1394
See eclipse-platform/eclipse.platform.releng.aggregator#1659

* Upload eventfile and unit test results

* 2023-06->2023-09 Seems to have broke dependency graph management in our project (#1698)

* Add test for reproducing #1654
* Fix to make that test pass

---------

Co-authored-by: Stephan Herrmann <stephan.herrmann@berlin.de>

* [memory] SoftReference for ResourceCompilationUnit.contents #1743

Ability to reduce memory during searches that find many files

#1743

* deduplicate "eclipse" #1743

CharDeduplication was not designed to deduplicate tokens with length 7+
which could lead to high memory consumption. With this change tokens of
all sizes can be deduplicated.

#1743

A benchmark implemented in CharDeduplicationTest.main(String[]) shows
the new deduplication is performed at similar speed (.21s instead of
.16s) but deduplicates much more tokens (99% instead of 36%).

* 1703.constant definitions (#1756)

* Fixes [21] AIOOB at switchStatement TNode.addPattern  (#1757)

org.eclipse.jdt.internal.compiler.ast.SwitchStatement $TNode.addPattern

---------

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de>
Co-authored-by: Eric Milles <eric.milles@thomsonreuters.com>
Co-authored-by: Александър Куртаков <akurtakov@gmail.com>
Co-authored-by: Jay Arthanareeswaran <jarthana@in.ibm.com>
Co-authored-by: Stephan Herrmann <stephan.herrmann@berlin.de>
Co-authored-by: Lars Vogel <Lars.Vogel@vogella.com>
Co-authored-by: Suby S Surendran <suby.surendran@ibm.com>
Co-authored-by: Hannes Wellmann <wellmann.hannes1@gmx.net>
Co-authored-by: Andrey Loskutov <loskutov@gmx.de>
Co-authored-by: Gayan Perera <gayanper@gmail.com>
Co-authored-by: Jörg Kubitz <51790620+jukzi@users.noreply.github.com>
Co-authored-by: Srikanth Sankaran <131454720+srikanth-sankaran@users.noreply.github.com>
Co-authored-by: Ed Merks <ed.merks@gmail.com>
Co-authored-by: Snjeza <snjezana.peco@redhat.com>
Co-authored-by: Christoph Läubrich <laeubi@laeubi-soft.de>
Co-authored-by: Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
rgrunber pushed a commit to eclipse-jdtls/eclipse-jdt-core-incubator that referenced this issue Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request preview-feature Preview Feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants