Parser: re-order identifier-enum keyword ordering

Prior to this commit:

  args:
    name string
    enum name ["alice", "bob", "charlie"]

after:

  args:
    name string
    name enum ["alice", "bob", "charlie"]

A couple of reasons:

1) I think, if you group lots of constraints together (as we add more
   types), it will be easier to read whats going on because, if they
   all start with the identifier, then the next keyword lines up
   visually.

2) It will arguably flow more nicely into some future keywords e.g.

  first_name requires last_name

although it does conflict with some planned syntax like

  exactly 2 arg1 arg2 arg3

where the keyword does come first.

I'm not 100% confident about this change, I just want to try it out and
then can re-evaluate later and perhaps revert.

core/parser.go

@@ -169,16 +169,16 @@ func (p *Parser) argStatement() ArgStmt {
 		panic(NOT_IMPLEMENTED)
 	}
 
-	if p.matchKeyword(ARGS_BLOCK_KEYWORDS, ENUM) {
-		return p.argEnumConstraint()
-	}
+	identifier := p.consume(IDENTIFIER, "Expected identifier or keyword")
 
-	identifier := p.consume(IDENTIFIER, "Expected Identifier or keyword")
+	if p.peekKeyword(ARGS_BLOCK_KEYWORDS, ENUM) {
+		return p.argEnumConstraint(identifier)
+	}
 
-	if p.peekType(STRING_LITERAL) ||
+	if p.peekType(STRING_LITERAL) || // for renames
 		p.peekType(IDENTIFIER) ||
 		p.peekType(STRING) ||
-		p.peekType(INT_LITERAL) ||
+		p.peekType(INT_LITERAL) || // to allow shorthand flags that are e.g. -1
 		p.peekType(BOOL) {
 
 		return p.argDeclaration(identifier)
@@ -192,9 +192,8 @@ func (p *Parser) argStatement() ArgStmt {
 	panic(NOT_IMPLEMENTED)
 }
 
-func (p *Parser) argEnumConstraint() ArgStmt {
-	enumTkn := p.previous()
-	identifier := p.consume(IDENTIFIER, "Expected identifier after 'enum'")
+func (p *Parser) argEnumConstraint(identifier Token) ArgStmt {
+	enumTkn := p.consume(IDENTIFIER, "Bug! Expected enum identifier")
 	t := ArgStringT
 	values := p.mixedArrayLiteral(&t)
 

core/testing/constraint_enum_test.go

@@ -6,7 +6,7 @@ func Test_Constraint_Enum_Valid(t *testing.T) {
 	rsl := `
 args:
 	name string
-	enum name ["alice", "bob", "charlie"]
+	name enum ["alice", "bob", "charlie"]
 print("Hi", name)
 `
 	setupAndRunCode(t, rsl, "alice")
@@ -19,7 +19,7 @@ func Test_Constraint_Enum_ErrorsOnInvalid(t *testing.T) {
 	rsl := `
 args:
 	name string
-	enum name ["alice", "bob", "charlie"]
+	name enum ["alice", "bob", "charlie"]
 print("Hi", name)
 `
 	setupAndRunCode(t, rsl, "david", "--NO-COLOR")
@@ -39,10 +39,23 @@ func Test_Constraint_Enum_ErrorsIfNonStringEnum(t *testing.T) {
 	rsl := `
 args:
 	name string
-	enum name ["alice", 2]
+	name enum ["alice", 2]
 print("Hi", name)
 `
 	setupAndRunCode(t, rsl, "david", "--NO-COLOR")
 	assertError(t, 1, "RslError at L4/23 on '2': Expected string literal, got int\n")
 	resetTestState()
 }
+
+func Test_Constraint_Enum_CanHaveArgNamedEnum(t *testing.T) {
+	rsl := `
+args:
+	enum string
+	enum enum ["alice", "bob", "charlie"]
+print("Hi", enum)
+`
+	setupAndRunCode(t, rsl, "alice")
+	assertOnlyOutput(t, stdOutBuffer, "Hi alice\n")
+	assertNoErrors(t)
+	resetTestState()
+}

docs-web/docs/reference/args.md

@@ -38,10 +38,12 @@ script Charlie 30 -e --friends David,Eve -h 1.86
 
 ## Constraint Statements
 
+### Enum
+
 ```rsl
 args:
     name string
-    enum name ["alice", "bob", "charlie"]
+    name enum ["alice", "bob", "charlie"]
 ```
 
 ```

docs/revisit.md

@@ -4,4 +4,5 @@
 - `quiet` being how we suppress announcements for shell commands
 - body should be a named arg in http methods?
 - should the int/float etc parsing funcs be prefixed with `parse_`?
-- one `http` func for all methods instead of some having their own? 
+- one `http` func for all methods instead of some having their own?
+- `enum name` vs. `name enum` ordering for arg constraints