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

TypeScript missing critical class definition exports #4110

Closed
kaby76 opened this issue Feb 12, 2023 · 20 comments
Closed

TypeScript missing critical class definition exports #4110

kaby76 opened this issue Feb 12, 2023 · 20 comments

Comments

@kaby76
Copy link
Contributor

kaby76 commented Feb 12, 2023

I was trying to write a template for TypeScript for testing the new target on grammars-v4. There is (are) important classes that need to be exported. Otherwise, I am blocked, and there will be no testing of the target in grammars-v4 because the trgen templates require a consistent runtime API in order to provide consistent functionality across targets.

I don't know why certain classes do not have corresponding TypeScript export class definition files. It should be identical to the list of exported symbols in Java.

@ericvergnaud
Copy link
Contributor

As a principle, I only export what is strictly required, excluding internal classes.
That can be hard to define until you bump into issues.
Can you provide an example of failing generated trgen code ?

@kaby76
Copy link
Contributor Author

kaby76 commented Feb 12, 2023

As a principle, I only export what is strictly required, excluding internal classes. That can be hard to define until you bump into issues. Can you provide an example of failing generated trgen code ?

I'm still going through with the creation of the template for TypeScript. So far, it's just Recognizer, which is easy to fix. But, I'll let you know what else may be missing, probably not much because the JavaScript target was working fine.

@kaby76
Copy link
Contributor Author

kaby76 commented Feb 12, 2023

Also missing is the CharStreams class. I use it in JavaScript (here, here, here), but there's no TypeScript decl for the class.

@ericvergnaud
Copy link
Contributor

CharStreams : There is and it is exported. Please check beta 5 ?

@kaby76
Copy link
Contributor Author

kaby76 commented Feb 12, 2023

CharStreams : There is and it is exported. Please check beta 5 ?

Thanks so much, again!!

@kaby76
Copy link
Contributor Author

kaby76 commented Feb 13, 2023

  • Version 4.12.0-beta.5 (and dev branch) doesn't have Recognizer exported.
  • CharStreams is not available even in 4.12.0-beta.5. In the JavaScript code, I could import and reference the static methods of CharStreams.js. I can't import CharStreams in typescript (i.e., import { CharStreams } from 'antlr4'; => compilation error), nor can I import the functions of CharStreams.js on the import statement, as I can do for CharStream and CommonTokenStream (i.e., import { fromPath } from 'antlr4'; => compilation error). CharStreams is not a class. How can I import the functions in CharStreams.d.ts in a TypeScript program? TypeScript has classes and static methods. I don't understand why these functions are wrapped in a class called "CharStreams" as in all the other targets.

@ericvergnaud
Copy link
Contributor

Let me check that

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Feb 13, 2023

In TypeScript, CharStreams is a class, and the methods are static methods.
It's a factory for actual streams, so you should write:

import { CharStreams } from 'antlr4';
const stream = CharStreams.fromPath(path);

I have added Recognizer, and fixed another issue in beta 6.

@ericvergnaud
Copy link
Contributor

If you're loading content from files you might want to upgrade to beta 8, I just added the encoding parameter to the FileStream API

@kaby76
Copy link
Contributor Author

kaby76 commented Feb 13, 2023

Excellent. Thank you.

Two other things that would be nice to have are: Lexer.reset() and ParserATNSimulator.trace_atn_sim. The later would be really nice since it allows for the parse to be compared across targets, which @parrt added recently. ParserATNSimulator.debug is available, but isn't useful because it outputs too much, so output can't be compared across targets as easily.

Otherwise, this works great. Already, the TypeScript port has shown a few problems with the JavaScript driver code I wrote. This is a great port. It's JavaScript finally done right.

@ericvergnaud
Copy link
Contributor

Done, check beta 9

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Feb 13, 2023

and thanks for the compliment

@kaby76
Copy link
Contributor Author

kaby76 commented Feb 13, 2023

and thanks for the compliment, I struggle with some contributors wanting me to reproduce the mistakes accumulated in the js API over 10 years...

You bet! This is a wonderful port. And we already have our first "customer" in grammars-v4.

@kaby76
Copy link
Contributor Author

kaby76 commented Feb 14, 2023

Looks like CommonToken is used in a number of grammar base classes, including the python/python3 grammar. That's somewhat easy even for me to fix.

import { Token } from "./Token";

export declare class CommonToken extends Token {
    constructor(source: number, type: number, channel: number, start: number, stop: number);
    clone(): CommonToken;
    cloneWithType(type: number): CommonToken;
    toString(): string;
}

The python/python3 grammar base class tries to set _token, but it can't because emitToken() is not available in the TypeScript code. We're also going to need skip(), more(), mode(d) declared in Lexer.d.ts.

@ericvergnaud
Copy link
Contributor

ericvergnaud commented Feb 14, 2023

Can you submit a PR ? That would avoid me adding stuff in the wrong place...

kaby76 added a commit to kaby76/antlr4 that referenced this issue Feb 16, 2023
…ethods from Lexer. These are needed for TypeScript ports of various grammars in grammars-v4. Issue antlr#4110.
parrt pushed a commit that referenced this issue Feb 16, 2023
…ethods from Lexer. These are needed for TypeScript ports of various grammars in grammars-v4. Issue #4110. (#4119)
@ericvergnaud
Copy link
Contributor

@kaby76 I think we can close this ?

@kaby76 kaby76 closed this as completed Feb 26, 2023
@RobEin
Copy link
Contributor

RobEin commented Jan 18, 2024

Excuse me.
How can I get the Token.DEFAULT_CHANNEL constant with the TypeScript target?

@ericvergnaud
Copy link
Contributor

Aargh... it's missing. Can you propose a PR to fix that ?
Until it gets released, you can safely assume its value is 0.

ericvergnaud pushed a commit that referenced this issue Jan 21, 2024
* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

based on:
https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L59

https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L65
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

#4110 (comment)
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

---------

Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>
ncordon pushed a commit to neo4j/cypher-language-support that referenced this issue Feb 1, 2024
* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

based on:
https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L59

https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L65
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

antlr/antlr4#4110 (comment)
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

---------

Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>
ncordon pushed a commit to neo4j/cypher-language-support that referenced this issue Feb 1, 2024
* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

based on:
https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L59

https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L65
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

antlr/antlr4#4110 (comment)
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

---------

Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>
ncordon pushed a commit to neo4j/cypher-language-support that referenced this issue Feb 2, 2024
* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

based on:
https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L59

https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L65
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

antlr/antlr4#4110 (comment)
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

---------

Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>
@RobEin
Copy link
Contributor

RobEin commented Feb 4, 2024

I would need additional features that are missing from the TypeScript runtime:

@ericvergnaud
Copy link
Contributor

Hi. Sure. Can you submit a PR ?

ncordon added a commit to neo4j/cypher-language-support that referenced this issue Feb 5, 2024
* fix: TypeScript: incorrect type in CommonTokenStream

Signed-off-by: Johannes Heesterman <johannes@elfsquad.io>

* added Token.DEFAULT_CHANNEL and Token.HIDDEN_CHANNEL (#4516)

* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

based on:
https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L59

https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L65
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

antlr/antlr4#4110 (comment)
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

---------

Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

---------

Signed-off-by: Johannes Heesterman <johannes@elfsquad.io>
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>
Co-authored-by: Johannes Heesterman <johannes@elfsquad.io>
Co-authored-by: Robert Einhorn <robert.einhorn.hu@gmail.com>
ncordon added a commit to neo4j/cypher-language-support that referenced this issue Feb 5, 2024
* fix: TypeScript: incorrect type in CommonTokenStream

Signed-off-by: Johannes Heesterman <johannes@elfsquad.io>

* added Token.DEFAULT_CHANNEL and Token.HIDDEN_CHANNEL (#4516)

* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

based on:
https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L59

https://github.com/antlr/antlr4/blob/ebb511a04a60ae5a605aba65471c07dd854e9303/runtime/JavaScript/src/antlr4/Token.js#L65
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

* added DEFAULT_CHANNEL and HIDDEN_CHANNEL constants

antlr/antlr4#4110 (comment)
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

---------

Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>

---------

Signed-off-by: Johannes Heesterman <johannes@elfsquad.io>
Signed-off-by: Robert Einhorn <robert.einhorn.hu@gmail.com>
Co-authored-by: Johannes Heesterman <johannes@elfsquad.io>
Co-authored-by: Robert Einhorn <robert.einhorn.hu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants