[dart] Add support to dart's sound null safety

[canastro]

Since dart's SDK version 2.12.0, Sound null safety was added.
The SDK needs to be updated and the usage of nulls should be reviewed.

[canastro]

Im not familiar with the code, and the change requires some deep knowledge of the codebase, so unfortunately I'm not able to do this myself.

[jnhyatt]

I have experience with Dart's sound null safety, and I'd be happy to take a stab at it if someone could point me in the direction of the Dart code generation.

[renancaraujo]

I've started to migrate but there is a lot of work to do. 246 errors to go:
https://github.com/antlr/antlr4/compare/master...renancaraujo:nnbd?expand=1

[renancaraujo]

We have migrated both the runtime and the generated code to nndb on #3116. We can just figure out the test runner, which is weird.

[renancaraujo]

I guess @lingyv-li coukd help us with that?

[canastro]

Although I replaced all occurrences of dart 2.8.4 to 2.12.1, I still get the following error when I run the runtime test suite:

Error: Null safety features are disabled for this library.
Try removing the package language version or setting the language version to 2.10 or higher.

Couldn't really figure out whats going on here.

[lingyv-li]

I did try to add sound nullsafety support too earlier, but it's a bit more tricky than I thought.

1. First of all it's a large codebase. Running the migrator leaves a lot of nullable because it uses a lot of imperative programming following the Java target.
2. The migration tool doesn't run on the codegen StringTemplate files. So it will be a much more manual process that requires expertise and experience on nullsafety.
3. From my understanding, projects not upgraded to nullsafety won't be able to use generated code with nullsafety. We might need to generate two sets of dart code if we want to support that.

I'm glad to see contributions happening, ping me wherever I can help.

[stefanschaller]

Hey guys, I just wanted to create the same issue. I'm in the bad situation that I need to use ANTLR with nullsafety because my company started to develop a super awesome interactive game in Flutter/Dart.

If you guys need any additional support in flutter or the dart nullsafety, you could contact me here or on Twitter https://twitter.com/StefanSchalle10 .

[canastro]

@stefanschaller Our PR is now passing all the Dart related tests, my project is now using antlr's version from our branch and is also working as expected. So, hopefully, we're getting closer.

@lingyv-li

From my understanding, projects not upgraded to nullsafety won't be able to use generated code with nullsafety. We might need to generate two sets of dart code if we want to support that.

I think that from this version onwards we should only support null-safety, otherwise we would also need two versions of the runtime, right? Since the whole ecosystem is moving to NNBD, i think its safe to say that from the current version onwards we only support a certain version of the dart SDK.

Can you help us out with a review, the PR is huge and we (me an @renancaraujo) made some assumptions that might require some deeper knowledge on antlr's codebase.

[stefanschaller]

@canastro Thanks, I will try it out.
For everyone that would also check out ANTLR4 with null safety, you need to add the following code to your pubspec.yaml:

  antlr4:
    git:
      url: git@github.com:renancaraujo/antlr4.git
      ref: nnbd
      path: runtime/Dart/

[stefanschaller]

@canastro In my case, it generates code that is not compilable.
e.g.:

  static final List<String> _LITERAL_NAMES = [
      null, null, null, "'<<'", null, null, null, "'|'", null, "'>>'", "']'", 
      null, null, "'||'", "'&&'", null, null, null, null, null, null, "'+'", 
      "'-'", "'*'", "'/'", "'%'", "'^'", "'!'", "';'", null, "'('", "')'", 
      "'{'", "'}'", "','", "'true'", "'false'", "'if'", "'elseif'", "'else'", 
      "'/if'", "'set'", "'textbox'", "'listbox'", "'option'", "'/listbox'", 
      null, null, null, "'either'"
  ];

or:

class ParseContext extends ParserRuleContext {
  BlockContext block() => getRuleContext<BlockContext>(0);
  TerminalNode EOF() => getToken(SugarCubeParser.TOKEN_EOF, 0);
  ParseContext([ParserRuleContext parent, int invokingState]) : super(parent, invokingState); 
}

1. getRuleContext returns a nullable type.
2. ([ParserRuleContext parent, int invokingState]) from the constructor is not marked as nullable.

Should I take care about something to fix this issue?
Or why do it behave like that.

Thanks.

[canastro]

@stefanschaller I think you're using the correct runtime but not the correct cli to run the generator.

You might need to clone the branch locally, build and add this (or something similar) to your bashrc file:

export CLASSPATH=".:/Users/username/.m2/repository/org/antlr/antlr4/4.9.2-SNAPSHOT/antlr4-4.9.2-SNAPSHOT-complete.jar:$CLASSPATH"
alias antlr4='java -Xmx500M -cp "/Users/username/.m2/repository/org/antlr/antlr4/4.9.2-SNAPSHOT/antlr4-4.9.2-SNAPSHOT-complete.jar:$CLASSPATH" org.antlr.v4.Tool'

[stefanschaller]

@canastro I don't get it, sorry.
I'm using antlr-runtime-4.9.2.jar from https://www.antlr.org/download.html.

You mean, I should clone your repo? But is there a cli tool checked in, inside the repo? Thanks for your help.

[canastro]

We also made changes to the generator, so you’ll need to use antlr by building our branch and using the jar that gets generated. The one you downloaded is a stable version that is already in master.

ps: I’m not close to a computer, so I can’t give better instructions rn

[lingyv-li]

@stefanschaller you can follow this https://github.com/antlr/antlr4/blob/master/doc/building-antlr.md to build and generate the parser from the nnbd branch.

I also think it's ok for us to drop support for older dart version, as the Dart target hasn't been around for too long. If anyone think otherwise, please comment. @canastro in this case can you please add some documentation mentioning the breaking change?

Regarding the PR, it looks good to me at first glance. To be honest I don't understand all details of ANTLR, so it would be great if more people can try and test it out.

[stefanschaller]

1. I cloned the repo from here: https://github.com/renancaraujo/antlr4/tree/nnbd
2. I did add steps from there: https://github.com/antlr/antlr4/blob/master/doc/building-antlr.md
   • $ mvn -DskipTests install
3. I took the generated jar from there: /Users/ma-name/.m2/repository/org/antlr/antlr4/4.9.3-SNAPSHOT/antlr4-4.9.3-SNAPSHOT.jar
4. Moved the generated jar to my repo
5. Added the following script to my repo: (The generated jar is part of my repo)

export CLASSPATH="antlr4-4.9.3-SNAPSHOT-complete.jar:$CLASSPATH"
alias antlr4='java -Xmx500M -cp "antlr4-4.9.2-SNAPSHOT-complete.jar:$CLASSPATH" org.antlr.v4.Tool'

antlr4 -Dlanguage=Dart SugarCubeLexer.g4 -o ./lib/generated
antlr4 -Dlanguage=Dart SugarCubeParser.g4 -o ./lib/generated

And it's still not working. The generated code is the same and there are still null values etc.
I'm really feeling like a programming beginner.

[canastro]

@lingyv-li : added the comment 👍🏽

@stefanschaller
Your alias doesn't seem to be pointing to your snapshot.
After fixing that, run which antlr4 to be sure its using the correct version.

This is a piece of the generated code from my lexer:

  static final List<String?> _LITERAL_NAMES = [
      null, null, null, null, null, null, null, "'`'", "'='", "'%'", "'{'", 
      "'}'", "'['", "']'", "'('", "')'", "','", "';'", "'+'", "'-'", "'*'", 
      "'/'", "'^'", "'<'", "'>'", "'<='", "'>='", "'<>'", "':'", "'!'", 
      "'&'", null, null, "'null'", null, null, "'#REF!'"
  ];

We can continue on twitter DMs.

[stefanschaller]

@canastro Thank you so much for your support!
I just tested the branch and Dart and the null safety in my project.

Looks good and the code is working properly.

Good job to everyone that is working on the branch. :D

[canastro]

@lingyv-li can you point us who we should tag in the PR to make a thorough review? Or is there any maintainers chat that you can ping someone to help with the review?

[lingyv-li]

@ericvergnaud should be able to help when your PR is no longer WIP. (Since he reviewed the Dart target PR.)

There might be a mailing list but not sure.

[renancaraujo]

Just removed the WIP tag on #3116, should be good to go!

[stefanschaller]

@lingyv-li This issue can be closed after we published a new version. The PR is already merged.

[obeobe]

May I ask when the new v4.9.3 will be released and also updated in https://pub.dev/packages/antlr4/versions ?

Thanks everybody for your hard work on this!

[KvanTTT]

@parrt please close