Merge pull request #145 from jmandel/check-arg-names

Check for valid field arg names

Author: leebyron

src/type/__tests__/validation.js

@@ -296,6 +296,17 @@ describe('Type System: Objects must have fields', () => {
     );
   });
 
+  it('rejects an Object type with incorrectly named fields', () => {
+    expect(
+      () => schemaWithFieldType(new GraphQLObjectType({
+        name: 'SomeObject',
+        fields: { 'bad-name-with-dashes': { type: GraphQLString } }
+      }))
+    ).to.throw(
+      'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "bad-name-with-dashes" does not.'
+    );
+  });
+
   it('rejects an Object type with incorrectly typed fields', () => {
     expect(
       () => schemaWithFieldType(new GraphQLObjectType({
@@ -353,6 +364,47 @@ describe('Type System: Objects must have fields', () => {
 });
 
 
+describe('Type System: Fields args must be properly named', () => {
+
+  it('accepts field args with valid names', () => {
+    expect(
+      () => schemaWithFieldType(new GraphQLObjectType({
+        name: 'SomeObject',
+        fields: {
+          goodField: {
+            type: GraphQLString,
+            args: {
+              goodArg: { type: GraphQLString }
+            }
+          }
+        }
+      }))
+    ).not.to.throw();
+  });
+
+  it('rejects field arg with invalid names', () => {
+    expect(
+      () => {
+        var QueryType = new GraphQLObjectType({
+          name: 'SomeObject',
+          fields: {
+            badField: {
+              type: GraphQLString,
+              args: {
+                'bad-name-with-dashes': { type: GraphQLString }
+              }
+            }
+          }
+        });
+        return new GraphQLSchema({ query: QueryType });
+      }).to.throw(
+      'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "bad-name-with-dashes" does not.'
+    );
+  });
+
+});
+
+
 describe('Type System: Fields args must be objects', () => {
 
   it('accepts an Object type with field args', () => {

src/type/schema.js

@@ -147,8 +147,13 @@ function typeMapReducer(map: TypeMap, type: ?GraphQLType): TypeMap {
     var fieldMap = type.getFields();
     Object.keys(fieldMap).forEach(fieldName => {
       var field = fieldMap[fieldName];
+      assertValidName(fieldName);
+
       if (field.args) {
         var fieldArgTypes = field.args.map(arg => arg.type);
+
+        field.args.forEach(arg => assertValidName(arg.name));
+
         reducedMap = fieldArgTypes.reduce(typeMapReducer, reducedMap);
       }
       reducedMap = typeMapReducer(reducedMap, field.type);
@@ -158,6 +163,13 @@ function typeMapReducer(map: TypeMap, type: ?GraphQLType): TypeMap {
   return reducedMap;
 }
 
+function assertValidName(name: string): void {
+  invariant(
+  /^[_a-zA-Z][_a-zA-Z0-9]*$/.test(name),
+  `Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ ` +
+  `but "${name}" does not.`);
+}
+
 function assertObjectImplementsInterface(
   object: GraphQLObjectType,
   iface: GraphQLInterfaceType