[WIP]Typescript target

[ericvergnaud]

All ts tests pass locally.
Submitting a PR to ensure nothing else is broken, and try ts CI.
Also fixes #3868
tsc generates tons of warnings which I need to deal with

[ericvergnaud]

mmm... not sure why my commits aren't signed

[ericvergnaud]

@parrt hey, here's a beta of the typescript target
Also fixes #3868
the CI works on Linux but needs to be fixed for Mac/Windows
beyond my own beta-testing, I think it would be great to bring in beta-testers, but that involves a 'sensitive' announcement so let me know what you think ?

[KvanTTT]

mmm... not sure why my commits aren't signed

Every commit should have a sign, for example:

Signed-off-by: Ivan Kochurkin <[email protected]>

Also, is it possible to squash most part of commits? Commits like

renamed for clarity
fix issues
progressing
...

are not useful for reviewers and git history since they are not informative.

[ericvergnaud]

mmm... not sure why my commits aren't signed

Every commit should have a sign, for example:

Signed-off-by: Ivan Kochurkin <[email protected]>

Also, is it possible to squash most part of commits? Commits like

renamed for clarity
fix issues
progressing
...

are not useful for reviewers and git history since they are not informative.

As written, I know what DCO expects, but I had automated this on my end, and it seems to no longer work..
This is WIP, so no need to review/squash yet.

[KvanTTT]

I had automated this on my end, and it seems to no longer work..

Looks like so because not every commit has a sign.

[ericvergnaud]

@parrt thanks for merging #3878 (which was missing some tweaks, my bad), however I noticed you merged it into master rather than dev ? So I had to merge back from master which brings into this PR a few unrelated files, I guess they'll fade away once you merge master onto dev.
Runtime wise, as far as I'm concerned, this is ready to go.
However not sure about the API (the d.ts files) since we don't know what people need.
So my preferred path would be to publish a beta tool and js/ts runtime only, and call for testers. I expect these will come primarily from antlr4ts, so hopefully they will have mature requirements. I'll also be doing my own testing, but that may take a while since it requires migrating my code from js to ts. I expect the beta to last 2-3 months. Let me know what you think.

[ericvergnaud]

It’s an incorrect implementation therefore it’s a design bug. I’m not really interested in arguing about code style. Your suggestions are welcome but they should remain suggestions. Envoyé de mon iPhone

…

Le 13 sept. 2022 à 23:13, Ivan Kochurkin ***@***.***> a écrit :  @KvanTTT commented on this pull request. In tool/src/org/antlr/v4/codegen/target/TypeScriptTarget.java: > + public String getTargetStringLiteralFromANTLRStringLiteral( + CodeGenerator generator, + String literal, boolean addQuotes) + { + StringBuilder sb = new StringBuilder(); + String is = literal; + + if ( addQuotes ) sb.append('"'); + + for (int i = 1; i < is.length() - 1; ) { + int codePoint = is.codePointAt(i); + int toAdvance = Character.charCount(codePoint); + if (codePoint == '\\') { + // Anything escaped is what it is! We assume that + // people know how to escape characters correctly. However + // we catch anything that does not need an escape in Java (which + // is what the default implementation is dealing with and remove + // the escape. The C target does this for instance. + // + int escapedCodePoint = is.codePointAt(i + toAdvance); + toAdvance++; + switch (escapedCodePoint) { + // Pass through any escapes that Java also needs + // + case 'n': + case 'r': + case 't': + case 'b': + case 'f': + case '\\': + // Pass the escape through + sb.append('\\'); + sb.appendCodePoint(escapedCodePoint); + break; + + case 'u': // Either unnnn or u{nnnnnn} + if (is.charAt(i + toAdvance) == '{') { + while (is.charAt(i + toAdvance) != '}') { + toAdvance++; + } + + toAdvance++; + } else { + toAdvance += 4; + } + + if (i + toAdvance <= is.length()) { + // we might have an invalid \\uAB or something + String fullEscape = is.substring(i, i + toAdvance); + appendUnicodeEscapedCodePoint( + CharSupport.getCharValueFromCharInGrammarLiteral(fullEscape), + sb); + } + + break; + + default: + if (shouldUseUnicodeEscapeForCodePointInDoubleQuotedString(escapedCodePoint)) { + appendUnicodeEscapedCodePoint(escapedCodePoint, sb); + } else { + sb.appendCodePoint(escapedCodePoint); + } + + break; + } + + // Go past the \ character + i++; + } else { + if (codePoint == 0x22) { + // ANTLR doesn't escape " in literal strings, but every other language needs to do so. + sb.append("\\\""); + } else if (shouldUseUnicodeEscapeForCodePointInDoubleQuotedString(codePoint)) { + appendUnicodeEscapedCodePoint(codePoint, sb); + } else { + sb.appendCodePoint(codePoint); + } + } + + i += toAdvance; + } + + if ( addQuotes ) sb.append('"'); + + return sb.toString(); + } IMHO it's a bug to have just one translation method for all targets because inevitably users will complain, and we need to safely fix the translation on a per target basis. Firstly, it's not bug, but maybe incorrect implementation. Secondly, IMHO it's not optimal to create duplicated functionality because of slightly different things. They can be encapsulated to separated methods according to OOP. The less code we have, the less potential errors we have. Moreover, this method not only escapes chars but also joins them. Joining logic should be the same. For that matter, charCount, codePointAt and other char methods are Java-specific, they should not be used for TypeScript or other target literals, but actually it doesn't matter. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

[parrt]

beta

Well, sounds reasonable to me but I need to evaluate the merits of either approach discussed in #3867

What's involved in creating a suitable beta? A zip? A branch?

[ericvergnaud]

It involves publishing a beta in maven central and npm

[jonaskello]

I was going to propose an issue to make the javascript target work better in typescript (exporting context classes so we can import them as typescript types). However having an official typescript target is of course a lot better :-). Just to know which way I should proceed I wanted to check if this PR will have a chance of getting merged in the foreseeable future or if it is more of an experiment?