UserNobody14 / tree-sitter-dart

Attempt to make a tree-sitter grammar for dart
MIT License
59 stars 36 forks source link

Global Mutable Variables #6

Closed TimWhiting closed 4 years ago

TimWhiting commented 4 years ago

See the global_mutable_variables branch for a test case and an attempted failed fix for this.

The issue is that the parser falls back on statements (because we allow statements at the top level for testing). Because of that we have a lot of conflicts of various kinds (specifically looking at the _top_level_definition conflicts). This causes it to think that the following is invalid.

List<String> animations = ['1', '2', '3', '4', 'Default'];
final String assetFile = "assets/myasset.flr";
class MyClass {

}

This is because it parses the List and final String as local variable definitions rather than top_level definitions. And since top level definitions must happen before statements, the class is unable to be parsed.

According to the spec (I'm looking at the informal spec page 188):

topLevelDeclaration ::= classDeclaration
| mixinDeclaration
| enumType
| typeAlias
| external? functionSignature ‘;’
| external? getterSignature ‘;’
| external? setterSignature ‘;’
| functionSignature functionBody
| getterSignature functionBody
| setterSignature functionBody
| (final | const) type? staticFinalDeclarationList ‘;’
| late final type? initializedIdentifierList ‘;’
| late? varOrType initializedIdentifierList ‘;’

Currently we don't support the last entry (that is global mutable variables). Ignoring late right now of course.

There are a few different solutions as I see it.

  1. Remove the ability to have statements in the top level program.
    • Pro: Removes a ton of issues / conflicts
    • Pro: Closest to actual language spec, and real parsed files
    • Con: Bloats the tests, because we have to wrap all statements in a void main(){}
  2. Make it so that it doesn't matter whether statements come before or after top_level_definitions (eg. move statement as last entry in top_level_definition and remove it from program).
    • Pro: Less bloating in the tests from void main(){}
    • Con: Allows invalid code, however this is already allowed somewhat.
  3. Try to resolve all of the conflicts we run into by figuring out proper precedence numbers for all the rules
    • Pro: Keeps things the way they are currently organized
    • Con: We have a lot of conflicts, and are sure to run into more, and it is hard & takes a lot of time to determine proper precedence

What are your opinions? @UserNobody14

TimWhiting commented 4 years ago

Another option would be to combine the definitions of local_variable_declaration, and the top level variable declarations. You don't capture as much about the restrictions in the global scope, but I'm not sure there are that many restrictions anyways. And it's pretty much impossible to distinguish them in the grammar, unless you know whether you are in the global scope.

TimWhiting commented 4 years ago

Fixed this. Turned out to be an issue with using _type_name rather than _type.