add error message when not used on classes or methods, clean up tests

src/vm/wren_compiler.c

@@ -363,7 +363,13 @@ struct sCompiler
   
   ObjMap* constants;
 
-  // Attributes for the next class or method, or NULL if not applicable
+  // The number of attributes seen while parsing.
+  // We track this separately as compile time attributes 
+  // are not stored, so we can't rely on attributes->count
+  // to enforce an error message when attributes are used
+  // anywhere other than methods or classes.
+  int numAttributes;
+  // Attributes for the next class or method.
   ObjMap* attributes;
 };
 
@@ -392,6 +398,7 @@ typedef struct
 } Variable;
 
 // Forward declarations
+static void disallowAttributes(Compiler* compiler);
 static void addToAttributeGroup(Compiler* compiler, Value group, Value key, Value value);
 static void emitClassAttributes(Compiler* compiler, ClassInfo* classInfo);
 static void copyAttributes(Compiler* compiler, ObjMap* into);
@@ -568,6 +575,7 @@ static void initCompiler(Compiler* compiler, Parser* parser, Compiler* parent,
     compiler->scopeDepth = 0;
   }
   
+  compiler->numAttributes = 0;
   compiler->attributes = wrenNewMap(parser->vm);
   compiler->fn = wrenNewFunction(parser->vm, parser->module,
                                  compiler->numLocals);
@@ -3325,6 +3333,7 @@ static Value consumeLiteral(Compiler* compiler, const char* message)
 
 static bool matchAttribute(Compiler* compiler) {
   if(match(compiler, TOKEN_HASH)) {
+    compiler->numAttributes++;
     bool runtimeAccess = match(compiler, TOKEN_BANG);
     if(match(compiler, TOKEN_NAME)) {
       Value group = compiler->parser->previous.value;
@@ -3681,13 +3690,18 @@ void definition(Compiler* compiler)
   if (match(compiler, TOKEN_CLASS))
   {
     classDefinition(compiler, false);
+    return;
   }
   else if (match(compiler, TOKEN_FOREIGN))
   {
     consume(compiler, TOKEN_CLASS, "Expect 'class' after 'foreign'.");
     classDefinition(compiler, true);
+    return;
   }
-  else if (match(compiler, TOKEN_IMPORT))
+
+  disallowAttributes(compiler);
+
+  if (match(compiler, TOKEN_IMPORT))
   {
     import(compiler);
   }
@@ -3878,6 +3892,18 @@ void wrenMarkCompiler(WrenVM* vm, Compiler* compiler)
 
 // Helpers for Attributes
 
+// Throw an error if any attributes were found preceding, 
+// and clear the attributes so the error doesn't keep happening.
+static void disallowAttributes(Compiler* compiler)
+{
+  if (compiler->numAttributes > 0)
+  {
+    error(compiler, "Attributes can only specified before a class or a method");
+    wrenMapClear(compiler->parser->vm, compiler->attributes);
+    compiler->numAttributes = 0;
+  }
+}
+
 // Add an attribute to a given group in the compiler attribues map
 static void addToAttributeGroup(Compiler* compiler, Value group, Value key, Value value) 
 {
@@ -3995,8 +4021,11 @@ static void emitClassAttributes(Compiler* compiler, ClassInfo* classInfo)
 }
 
 // Copy the current attributes stored in the compiler into a destination map
+// This also resets the counter, since the intention is to consume the attributes
 static void copyAttributes(Compiler* compiler, ObjMap* into)
 {
+  compiler->numAttributes = 0;
+
   if(compiler->attributes->count == 0) return;
   if(into == NULL) return;
 
@@ -4014,10 +4043,13 @@ static void copyAttributes(Compiler* compiler, ObjMap* into)
 }
 
 // Copy the current attributes stored in the compiler into the method specific
-// attributes for the current enclosingClass
-static void copyMethodAttributes(Compiler* compiler, bool isForeign, 
+// attributes for the current enclosingClass.
+// This also resets the counter, since the intention is to consume the attributes
+static void copyMethodAttributes(Compiler* compiler, bool isForeign,
             bool isStatic, const char* fullSignature, int32_t length) 
 {
+  compiler->numAttributes = 0;
+
   if(compiler->attributes->count == 0) return;
 
   WrenVM* vm = compiler->parser->vm;

test/language/class/attributes.wren

@@ -1,120 +0,0 @@
-
-// No attributes should have no ClassAttributes allocated
-
-class Without {}
-System.print(Without.attributes == null)    // expect: true
-
-// Attributes without a ! shouldn't be 
-// passed to the runtime, they're compiled out
-
-#compileonly
-class WithNonRuntime {
-  #unused
-  method() {}
-}
-System.print(WithNonRuntime.attributes == null)    // expect: true
-
-// Test the basic states. Keys without a group
-// go into a group with null as the key
-
-#!key
-class Attr {}
-
-System.print(Attr.attributes != null)         // expect: true
-System.print(Attr.attributes.self != null)    // expect: true
-System.print(Attr.attributes.methods)         // expect: null
-
-var attr = Attr.attributes.self
-var nullGroup = attr[null]
-System.print(nullGroup != null)             // expect: true
-System.print(nullGroup.count)               // expect: 1
-System.print(nullGroup.containsKey("key"))  // expect: true
-
-var keyItems = nullGroup["key"]
-System.print(keyItems != null)              // expect: true
-System.print(keyItems is List)              // expect: true
-System.print(keyItems.count)                // expect: 1
-System.print(keyItems[0])                   // expect: null
-
-// Keys must be a name, and values can be any literal value
-
-#!name = name
-#!string = "string"
-#!integer = 32
-#!number = 2.5
-#!bool = true
-class Literals {}
-
-var literalGroup = Literals.attributes.self[null]
-
-System.print(literalGroup.count)                  // expect: 5
-System.print(literalGroup["string"][0] is String) // expect: true
-System.print(literalGroup["string"][0])           // expect: string
-System.print(literalGroup["integer"][0] is Num)   // expect: true
-System.print(literalGroup["integer"][0])          // expect: 32
-System.print(literalGroup["number"][0] is Num)    // expect: true
-System.print(literalGroup["number"][0])           // expect: 2.5
-System.print(literalGroup["bool"][0] is Bool)     // expect: true
-System.print(literalGroup["bool"][0])             // expect: true
-
-// Duplicate keys add multiple values to 
-// the attribute's key, in parse order 
-#!key
-#!key = value
-#!key=other
-class DuplicateKeys {}
-
-var dupeGroup = DuplicateKeys.attributes.self[null]
-System.print(dupeGroup.count)               // expect: 1
-System.print(dupeGroup["key"].count)        // expect: 3
-System.print(dupeGroup["key"])              // expect: [null, value, other]
-
-// Groups store attributes by named group
-
-#!key //not combined
-#!group(key=combined)
-#!group(key=value, key=2, key=false)
-class GroupedKeys {}
-
-var ungroupedKeys = GroupedKeys.attributes.self[null]
-var groupedKeys = GroupedKeys.attributes.self["group"]
-
-System.print(ungroupedKeys.count)               // expect: 1
-System.print(groupedKeys.count)                 // expect: 1
-System.print(ungroupedKeys.containsKey("key"))  // expect: true
-var groupedKey = groupedKeys["key"]
-System.print(groupedKey.count)                  // expect: 4
-System.print(groupedKey)                        // expect: [combined, value, 2, false]
-
-
-class Methods {
-
-  #!getter
-  method {}
-
-  method() {}
-
-  #!regular = 2
-  #!group(key, other=value, string="hello")
-  method(arg0, arg1) {}
-
-  #!is_static = true
-  static method() {}
-
-}
-
-var methodAttr = Methods.attributes.methods
-var getter = methodAttr["method"]
-var none = methodAttr["method()"]
-var regular = methodAttr["method(_,_)"]
-var aStatic = methodAttr["static method()"]
-
-// (Be wary of relying on map order)
-
-System.print(getter)                        // expect: {null: {getter: [null]}}
-System.print(none)                          // expect: null
-System.print(regular[null])                 // expect: {regular: [2]}
-System.print(regular["group"]["key"])       // expect: [null]
-System.print(regular["group"]["other"])     // expect: [value]
-System.print(regular["group"]["string"])    // expect: [hello]
-System.print(aStatic[null])                 // expect: {is_static: [true]}

test/language/class/attributes/attributes.wren

@@ -0,0 +1,21 @@
+// Test the basic states. Keys without a group
+// go into a group with null as the key
+
+#!key
+class Attr {}
+
+System.print(Attr.attributes != null)         // expect: true
+System.print(Attr.attributes.self != null)    // expect: true
+System.print(Attr.attributes.methods)         // expect: null
+
+var attr = Attr.attributes.self
+var nullGroup = attr[null]
+System.print(nullGroup != null)             // expect: true
+System.print(nullGroup.count)               // expect: 1
+System.print(nullGroup.containsKey("key"))  // expect: true
+
+var keyItems = nullGroup["key"]
+System.print(keyItems != null)              // expect: true
+System.print(keyItems is List)              // expect: true
+System.print(keyItems.count)                // expect: 1
+System.print(keyItems[0])                   // expect: null

test/language/class/attributes/compile_only.wren

@@ -0,0 +1,9 @@
+// Attributes without a ! shouldn't be 
+// passed to the runtime, they're compiled out
+
+#compileonly
+class WithNonRuntime {
+  #unused
+  method() {}
+}
+System.print(WithNonRuntime.attributes == null)    // expect: true

test/language/class/attributes/duplicate_keys.wren

@@ -0,0 +1,11 @@
+// Duplicate keys add multiple values to 
+// the attribute's key, in parse order 
+#!key
+#!key = value
+#!key=other
+class DuplicateKeys {}
+
+var dupeGroup = DuplicateKeys.attributes.self[null]
+System.print(dupeGroup.count)               // expect: 1
+System.print(dupeGroup["key"].count)        // expect: 3
+System.print(dupeGroup["key"])              // expect: [null, value, other]

test/language/class/attributes/groups.wren

@@ -0,0 +1,17 @@
+// Groups store attributes by named group
+
+#!key //not combined
+#!group(key=combined)
+#!group(key=value, key=2, key=false)
+class GroupedKeys {}
+
+var ungroupedKeys = GroupedKeys.attributes.self[null]
+var groupedKeys = GroupedKeys.attributes.self["group"]
+
+System.print(ungroupedKeys.count)               // expect: 1
+System.print(groupedKeys.count)                 // expect: 1
+System.print(ungroupedKeys.containsKey("key"))  // expect: true
+var groupedKey = groupedKeys["key"]
+System.print(groupedKey.count)                  // expect: 4
+System.print(groupedKey)                        // expect: [combined, value, 2, false]
+

test/language/class/attributes/invalid_expression.wren

@@ -0,0 +1,12 @@
+
+// When used in an expression location, 
+// the error remains Error at '#': Expected expression
+
+#valid
+class Example {
+
+  #valid
+  method() {
+    return #invalid 1 // expect error
+  }
+}

test/language/class/attributes/invalid_scope.wren

@@ -0,0 +1,11 @@
+
+
+#valid
+class Example {
+
+  #valid
+  method() {
+    #invalid    // expect error
+    var a = 3
+  }
+}

test/language/class/attributes/invalid_toplevel.wren

@@ -0,0 +1,4 @@
+
+
+#meta     // expect error
+var A = 3 

test/language/class/attributes/literals.wren

@@ -0,0 +1,20 @@
+// Keys must be a name, and values can be any literal value
+
+#!name = name
+#!string = "string"
+#!integer = 32
+#!number = 2.5
+#!bool = true
+class Literals {}
+
+var literalGroup = Literals.attributes.self[null]
+
+System.print(literalGroup.count)                  // expect: 5
+System.print(literalGroup["string"][0] is String) // expect: true
+System.print(literalGroup["string"][0])           // expect: string
+System.print(literalGroup["integer"][0] is Num)   // expect: true
+System.print(literalGroup["integer"][0])          // expect: 32
+System.print(literalGroup["number"][0] is Num)    // expect: true
+System.print(literalGroup["number"][0])           // expect: 2.5
+System.print(literalGroup["bool"][0] is Bool)     // expect: true
+System.print(literalGroup["bool"][0])             // expect: true

test/language/class/attributes/methods.wren

@@ -0,0 +1,32 @@
+
+class Methods {
+
+  #!getter
+  method {}
+
+  method() {}
+
+  #!regular = 2
+  #!group(key, other=value, string="hello")
+  method(arg0, arg1) {}
+
+  #!is_static = true
+  static method() {}
+
+}
+
+var methodAttr = Methods.attributes.methods
+var getter = methodAttr["method"]
+var none = methodAttr["method()"]
+var regular = methodAttr["method(_,_)"]
+var aStatic = methodAttr["static method()"]
+
+// (Be wary of relying on map order)
+
+System.print(getter)                        // expect: {null: {getter: [null]}}
+System.print(none)                          // expect: null
+System.print(regular[null])                 // expect: {regular: [2]}
+System.print(regular["group"]["key"])       // expect: [null]
+System.print(regular["group"]["other"])     // expect: [value]
+System.print(regular["group"]["string"])    // expect: [hello]
+System.print(aStatic[null])                 // expect: {is_static: [true]}

test/language/class/attributes/without.wren

@@ -0,0 +1,4 @@
+// With no attributes defined, no ClassAttributes should be allocated
+
+class Without {}
+System.print(Without.attributes == null)    // expect: true