There. I said it.

Defining task sequence is a mess, even when we only have a really simple build process. Plus we have a million dependencies just gulp-related that only end up running some basic console command anyway, and most of the tasks are just there so we can run them in sequence instead of in parallel...

I need to rewire the build process so the WRE is properly compiled and all tests are run against /dist and I've been postponing it for days because I know it's going to be ugly.

I would be so much happier if we could replace the whole gulp mambo-jambo with a couple of small yarn tasks. I understand there are CI reasons why we need gulp, but I'll be glad to investigate if there is some other way.

What do you think, @javierfernandes?

Not really sure why this happens since I saw there are tests for this asserting for Catch() but if I parse this

console.log(JSON.stringify(parser.parse(`
      program p {
        try {
          var age = 23
          age = age + 1
        } catch e {
        }
      }
    `), null, 2))

I get this (important part almost at the end)

{
  "nodeType": "File",
  "content": [
    {
      "nodeType": "Program",
      "name": "p",
      "sentences": {
        "nodeType": "Block",
        "sentences": [
          {
            "nodeType": "Try",
            "sentences": {
              "nodeType": "Block",
              "sentences": [
                {
                  "nodeType": "VariableDeclaration",
                  "variable": {
                    "nodeType": "Variable",
                    "name": "age"
                  },
                  "writeable": true,
                  "value": {
                    "nodeType": "Literal",
                    "value": 23
                  }
                },
                {
                  "nodeType": "Assignment",
                  "variable": {
                    "nodeType": "Variable",
                    "name": "age"
                  },
                  "value": {
                    "nodeType": "BinaryOp",
                    "op": "+",
                    "left": {
                      "nodeType": "Variable",
                      "name": "age"
                    },
                    "right": {
                      "nodeType": "Literal",
                      "value": 1
                    }
                  }
                }
              ]
            },
            "catches": [],
            "always": {
              "nodeType": "Block",
              "sentences": []
            }
          },
          {
            "nodeType": "Variable",                <<<<<<<< HERE !!! SHOULD BE CATCH !
            "name": "catch"
          },
          {
            "nodeType": "Variable",
            "name": "e"
          },
          {
            "nodeType": "Closure",
            "parameters": [],
            "sentences": {
              "nodeType": "Block",
              "sentences": []
            }
          }
        ]
      }
    }
  ]
}

Tried to parse

program p {
        try {
          var age = 23
          age = age + 1
        } catch (e) {
          // nothing to see here
        }
      }

And got ...

SyntaxError: Expected "!", "#{", "'", "(", "+", "-", "0X", "0x", "=>", "[", "\"", "^", "const", "false", "if", "new", "not", "null", "object", "return", "self", "super", "throw", "true", "try", "var", "{", "}", [ \t\r\n], [0-9], or [a-zA-Z_] but "/" found.

See here

catch = _ 'catch' __ variable:variable _ type:(':' _ qualifiedName)? _ handler:blockOrSentence { return Catch(variable,type?type[2]:undefined)(...handler) }

I think that there is a mistake as the catch doesn't reference a variable but declare one, just like a closure parameter or method parameters..

Currently the linker fails with this

try {

}
catch e {
}

With Cannot resolve reference to 'e' at ???
Because as it is a "Variable" it assumes is a reference and not a declaration.

I think that this should be changed in the grammar.
@nscarcella am I right ?

Currently the last step of the linker is to check if there was any unresolved reference and then throw an exception with the first one (for backward compatibility because the initial impl failed with the first unresolved reference).

We are moving to a more "functional" implementation where the linker is now a serie of functions pipeline folding the AST over it.

So, the last step should just return the AST with:

• the scoping information
• the links done
• any error in the nodes themselves.

For example

Node: {
   type: 'Variable'
   name: 'edad'
   errors: [
       { type: 'Linkage' }   // message can be generated once you have  all the data here
   ]
}

AST => linker => AST(+errors, +links, +scopes)

So, I've been reading a lot about how babel handles AST transformations and what possible improvements may come from migrating some parts of the code to Typescript.

As far as Typescript concerns, since there is no way to check types in runtime, I don't think it would imply a functional differences (besides some eager error detection).

But...

I tried to pay close attention to the way babel handles paths and node modifications and the general way it does things (particularly the ones we are already kind of copying...) and I'm not sure I like it.

I'm cool with the idea of avoiding state modification while traversing the ASTs (I think traditional OOP's visitors can get quite upsetting and hard to follow) but, even if that seems to be the general idea they manage, the implementation does not seem to agree.

If you check the babel handbook, particularly the section where avoiding state handling is described you will notice a couple of odd things:

• You are supposed to define visitors within visitors to properly handle recursive changes.
  So what's the point of having a visitor if you are going to handle propagation yourself?
• The visitors contract have enter and exit methods for each node type.
  That may look like a good idea at first, but the only reason to need to handle the exit event is to deal with state changes since entering.
• Even if paths prevent you from modifying the nodes, they still expose a very effect-cause based interface
  Take this code for example:

BinaryExpression(path) {
  path.parentPath.remove();
}

Sure, there is no effect on the node, which allows you to remove the parent, even if it was already visited; but that doesn't change the fact that this is no transformation function: You are not returning the node you want, you are causing a state change and still need to orchestrate it's impact in all the other handlers.

So, due to these factors I propose we change the way the linker and compiler are implemented (and the future validations and transformations) so we conform to the following rules:

• Each handler returns the next node instead of modifying the given node or working with a separate (yet effect oriented implementation) like paths. We could augment the nodes interface to have useful checks and copying logic, though.
• Each iteration over a tree is handled by a function instead of a visitor. This would not be such a big change from what we have, we just leave the "enter"/"exit" model and allow each function to handle the propagation as each case seems fit, so there would be no default propagation contract to follow.
• We could make this functions behave differently based on a configuration. Here is a quick example of how I imagine this could work:

// This functions can be used as a constructor for "visitor functions"
const traverse = configuration => Object.Assign(model => {
    const applicableTxs = Object.keys(configuration).filter(key =>
        key.split(',').some(e => model.type === e)
    )

    if (applicableTxs.length < 1) throw new Error // No config key was good
    return configuration[applicableTxs[0]](model)
}, configuration)

//Then we could build functions like this:
const compile = traverse({
  [Assignment]: ({variable, value}) => `${variable.name} = compile(value)`, // recursion is still manually handled, yet dynamically dispatched
  [Expression]: (s) => s.is(Variable) ? ... compile.Variable(s) ... : ...
  [File]: (content) => traverse({ ... })   // We can always nest traverse functions if necessary
   ...
})

// Only a single key matching the node type will it be applied. If more than one matches, we could apply the first one only or just fail, to avoid complex, hard to track scenarios.
// Notice that keys are not strings, but the actual builder functions for the node (we can change their toString to return it's name instead of the pseudocode). This allow us to use other things, such as arrays of node builders as keys to, so we could define Expression like this in the model:
const Expression = [Variable, Operation, etc...]
// This way, we can model relations between node types, generalize common handlers and extend the node behavior so it can do things like:

Variable('a').is(Variable) // true
Variable('a').is(Expression) // true
Variable('a').is(Sentence) // false

So, for example, we could write the createScope linker step like this:

const scopeFor = traverse({
  [File]: (node) => {}
  [Scopable]: (node) => {...scopeFor(node.parent), ...node.is(Referenciable) ? {[node.name]: node} : {} }
 })
 // This code may not even work, is just to give a general idea

One last thing that might be good about this is that we could define generic lifting functions, like this:

const nMap = (f) => traverse({
  [File]: ({content}) => File(...content.map(nMap(f)))
  [Variable]: v => f(v)
  ... // repeat for each node
 })

const nFilter = (f) => traverse({
  [File]: ({content}) => File(...content.filter(c => nFilter(f)(c) != undefined ) )
  [Variable]: v => f(v) ? v : undefined
  ... // repeat for each node
 })

  // These can now be used as a more classic visitor that might take other traverse functions as parameters!

addScopeToTree = nMap( n => ({ ...n, scopeFor(n) }) )

I don't know... It feels better. What do you think @javierfernandes ?

Hey @nscarcella .. I'm not sure if it came from wollok xtext grammar, if so then that is also wrong,
but I think that wollok doesn't currently support "instance of" operator, and it will never do so :)

So I think that we can remove this parsing rule

export const InstanceOf = (left, right) => Node(InstanceOf, location)({ left, right })