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

Internalize ANTLR-generated parser sources #1446

Closed
alancai98 opened this issue Apr 25, 2024 · 2 comments · Fixed by #1452
Closed

Internalize ANTLR-generated parser sources #1446

alancai98 opened this issue Apr 25, 2024 · 2 comments · Fixed by #1452

Comments

@alancai98
Copy link
Member

As of v0.14.5, all the ANTLR-generated parser sources in the partiql-parser subproject are marked as public. See the docs -- https://javadoc.io/doc/org.partiql/partiql-parser/0.14.5/partiql-parser/org.partiql.parser.antlr/index.html. We currently do not intend for these to be implemented outside of other packages within our codebase (ideally just partiql-parser).

We should try to do one or a combination of the following:

  1. Make the ANTLR-generated parser package-private. It's currently Java code and not Kotlin code so package-private rather than internal.
  2. Add a comment that the ANTLR-generated sources are intended to be implemented only by other partiql packages (similar to what we had done for sprout-generated interfaces -- Add warning to not implement sprout-generated visitor interfaces #1413).
  3. Move the ANTLR-generated sources behind an internal namespace and add a comment that we do not intend for APIs under the internal namespace are not intended to be implemented.
@alancai98
Copy link
Member Author

Likely can't do the first point (make package-private) until we deprecate and remove the PIG-parser from partiql-ast.

@alancai98
Copy link
Member Author

#1452 was merged to v1 branch. Change will be part of the next major version release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant