In the parser, Assign should not be an Expression

[jbdeboer]

Currently, assignments (the Assign class) is a subclass of Expression. But this implies that you can watch assignments, but you can't.

Assignments should not be allows in watches (e.g. only in places where we pass them to Scope.apply()).

[IgorMinar]

Allowing assignments makes many things, including dirty checking much harder because an expression that was expected to be idempotent would now have side-effects which we are not ready to handle.

[chalin]

From looking at the code, a little while back, I came up with a grammar; here is an excerpt from here that is relevant to this issue:

expressions:  expression (';' expression)?
expression: 
    literal
  | id args?                        # variable or function
  | expression '.' id args?         # member
  | expression '|' id filterArg*    # filter
  | expression '[' expression ']'
  | preOp expression
  | expression binOp expression
  | expression '?' expression ':' expression
  | expression '=' expression           # assignment

In the source code, the expressions non-terminal was referred to as "expression chain". Maybe "expression chain" should be renamed to "statements" so that the grammar might look like this:

statements:  statement (';' statements)?
statement: 
    expression                          # expression statement
  | expression '=' expression           # assignment
expression: ... same as above but without "assignment" 

Is this what you have in mind?

[mhevery]

Sounds reasonable. I am a little concerned that such a change will complicate the parser.

It is true that you can't watch assignments, but you will get an error about that during AST conversion.