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

Mavo.Script.getVariables(ast) function to extract expression variables #975

Closed
Tracked by #967
LeaVerou opened this issue Oct 3, 2023 · 6 comments
Closed
Tracked by #967
Assignees

Comments

@LeaVerou
Copy link
Member

LeaVerou commented Oct 3, 2023

This is part of #967 and depends on #974

What would this function do?

This function would extract variables, i.e. non-literal operands, from a Mavo expression and return them as an array of AST nodes (object literals with a type attribute).

Requirements:

  • AST nodes returned will be either Identifier or MemberExpression or (more rarely) a ThisExpression. However, not all nodes of these types will be part of a variable.
  • For MemberExpression nodes (e.g. foo.bar.baz) we want the topmost MemberExpression node.
  • Variables cannot be function calls. While in the future we may have some cool functionality where authors can create custom functions (e.g. see Mavo functions #161), don't worry about that right now.

Step 1: Map generalized AST structures to separate getVariables() functions

For each AST structure identified in #974, write a function that maps it to an array of its variables.

For example, for the type of expression in Step 2 of #974, there is only one operand, which is returned by ast.arguments[0],
whereas for an expression of the form foo(bar + 1), ast.arguments[0] would be a BinaryExpression — there we'd get the variable via ast.arguments[0].left.

You will probably notice a lot of patterns here, and will already start consolidating similar AST structures and coming up with a prototype algorithm. This is great!

Step 2: Create automated tests

Create a Mavo.Script.getVariables(ast) function, with the signature described above, but do not implement it yet.

Add a page in test.mavo.io, pick one expression from each AAST category, and create tests for the function.

Step 5: Write algorithm to extract variables

Hopefully, after Step 1, this will be an easy task.
Use the tests you wrote in Step 2 to verify the algorithm is correct.

@LeaVerou
Copy link
Member Author

LeaVerou commented Oct 3, 2023

Implementation sketch, entirely untested, may or may not be correct:

function getVariables(node) {
	let args;
	switch (node.type):
		case "LiteralExpression": 
			return [];
		case "UnaryExpression":
			return getVariables(node.argument);
		case "CallExpression": 
			args = node.arguments; break;
		case "Compound": 
			args = node.body; break;
		case "BinaryExpression":
			args = [node.left, node.right]; break;
		case "ArrayExpression":
			args = node.elements; break;
		case "ConditionalExpression":
			args = [node.test, node.consequent, node.alternate]; break;
		case "ThisExpression":
		case "Identifier":
		case "MemberExpression":
		default:
			return [node];
	}

	if (args) {
		return args.flatMap(arg => getVariables(arg));
	}
}

@adamjanicki2
Copy link
Collaborator

@LeaVerou Follow-up on MemberExpression nodes:

An example: user.info.name gets parsed into the AST:

{
  "type": "MemberExpression",
  "computed": false,
  "object": {
    "type": "MemberExpression",
    "computed": false,
    "object": {
      "type": "Identifier",
      "name": "user"
    },
    "property": {
      "type": "Identifier",
      "name": "info"
    }
  },
  "property": {
    "type": "Identifier",
    "name": "name"
  }
}

In the implementation for getVariables for this AST, would we want to return the whole AST, or just the user node, i.e.

{
      "type": "Identifier",
      "name": "user"
}

@adamjanicki2
Copy link
Collaborator

I guess a better way to phrase it would be are foo.bar and foo.baz their own variables, or is the top level foo the only variable

@LeaVerou
Copy link
Member Author

LeaVerou commented Oct 4, 2023

Think of the problem we're solving. Why are we writing this function? What do we aim to do with it? If you haven't read it yet, #967 will provide some essential context.
If we return a simplified node like {type: "Identifier", name: "user"} will that make it easier or harder to satisfy that use case compared to returning the actual AST node?

@LeaVerou
Copy link
Member Author

LeaVerou commented Oct 27, 2023

Pasting my Slack message here for posterity:

Thinking more about getVariables(), I think it should return all, including globals, and the surrounding code should deal with globals etc. Especially since if we rewrite Mavo apps to use Vue, we’d want to know which globals are being used so we can expose them. That also makes it more portable and more general purpose.

In fact, I wonder if it should return functions as well, just separately. I.e. a data structure like {"variables": [], "functions": []}. Then we can utilize it for the $fn rewriting as well, that is currently happening in a sort of ad hoc fashion. And of course then it would have a more generic name than getVariables()

@adamjanicki2
Copy link
Collaborator

Implemented in vastly, marking this as closed

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

2 participants