Merge pull request #95 from cjoudrey/descriptions-as-strings

Descriptions as strings in GraphQL SDL

README.md

@@ -44,6 +44,10 @@ Options:
 
     schema definition will be read from STDIN instead of specified file
 
+  --comment-descriptions
+
+    use old way of defining descriptions in GraphQL SDL
+
   -c, --config-direction <path>
 
     path to begin searching for config files

package.json

@@ -41,7 +41,7 @@
     "cosmiconfig": "^3.1.0",
     "figures": "^2.0.0",
     "glob": "^7.1.2",
-    "graphql": "^0.10.1",
+    "graphql": "^0.13.0",
     "graphql-config": "^1.0.0",
     "lodash": "^4.17.4"
   },

src/configuration.js

@@ -16,9 +16,14 @@ export class Configuration {
       - schemaPaths: [string array] file(s) to read schema from
       - customRulePaths: [string array] path to additional custom rules to be loaded
       - stdin: [boolean] pass schema via stdin?
+      - commentDescriptions: [boolean] use old way of defining descriptions in GraphQL SDL
   */
   constructor(options = {}, stdinFd = null) {
-    const defaultOptions = { format: 'text', customRulePaths: [] };
+    const defaultOptions = {
+      format: 'text',
+      customRulePaths: [],
+      commentDescriptions: false,
+    };
     const configOptions = loadOptionsFromConfig(options.configDirectory);
 
     // TODO Get configs from .graphqlconfig file
@@ -33,6 +38,10 @@ export class Configuration {
     );
   }
 
+  getCommentDescriptions() {
+    return this.options.commentDescriptions;
+  }
+
   getSchema() {
     if (this.schema) {
       return this.schema;

src/rules/enum_values_have_descriptions.js

@@ -1,10 +1,14 @@
 import { getDescription } from 'graphql/utilities/buildASTSchema';
 import { ValidationError } from '../validation_error';
 
-export function EnumValuesHaveDescriptions(context) {
+export function EnumValuesHaveDescriptions(configuration, context) {
   return {
     EnumValueDefinition(node, key, parent, path, ancestors) {
-      if (getDescription(node)) {
+      if (
+        getDescription(node, {
+          commentDescriptions: configuration.getCommentDescriptions(),
+        })
+      ) {
         return;
       }
 

src/rules/fields_have_descriptions.js

@@ -1,10 +1,14 @@
 import { getDescription } from 'graphql/utilities/buildASTSchema';
 import { ValidationError } from '../validation_error';
 
-export function FieldsHaveDescriptions(context) {
+export function FieldsHaveDescriptions(configuration, context) {
   return {
     FieldDefinition(node, key, parent, path, ancestors) {
-      if (getDescription(node)) {
+      if (
+        getDescription(node, {
+          commentDescriptions: configuration.getCommentDescriptions(),
+        })
+      ) {
         return;
       }
 

src/rules/input_object_values_have_descriptions.js

@@ -1,10 +1,14 @@
 import { getDescription } from 'graphql/utilities/buildASTSchema';
 import { ValidationError } from '../validation_error';
 
-export function InputObjectValuesHaveDescriptions(context) {
+export function InputObjectValuesHaveDescriptions(configuration, context) {
   return {
     InputValueDefinition(node, key, parent, path, ancestors) {
-      if (getDescription(node)) {
+      if (
+        getDescription(node, {
+          commentDescriptions: configuration.getCommentDescriptions(),
+        })
+      ) {
         return;
       }
 

src/rules/types_have_descriptions.js

@@ -1,8 +1,12 @@
 import { getDescription } from 'graphql/utilities/buildASTSchema';
 import { ValidationError } from '../validation_error';
 
-function validateTypeHasDescription(context, node, typeKind) {
-  if (getDescription(node)) {
+function validateTypeHasDescription(configuration, context, node, typeKind) {
+  if (
+    getDescription(node, {
+      commentDescriptions: configuration.getCommentDescriptions(),
+    })
+  ) {
     return;
   }
 
@@ -17,34 +21,34 @@ function validateTypeHasDescription(context, node, typeKind) {
   );
 }
 
-export function TypesHaveDescriptions(context) {
+export function TypesHaveDescriptions(configuration, context) {
   return {
     TypeExtensionDefinition(node) {
       return false;
     },
 
     ScalarTypeDefinition(node) {
-      validateTypeHasDescription(context, node, 'scalar');
+      validateTypeHasDescription(configuration, context, node, 'scalar');
     },
 
     ObjectTypeDefinition(node) {
-      validateTypeHasDescription(context, node, 'object');
+      validateTypeHasDescription(configuration, context, node, 'object');
     },
 
     InterfaceTypeDefinition(node) {
-      validateTypeHasDescription(context, node, 'interface');
+      validateTypeHasDescription(configuration, context, node, 'interface');
     },
 
     UnionTypeDefinition(node) {
-      validateTypeHasDescription(context, node, 'union');
+      validateTypeHasDescription(configuration, context, node, 'union');
     },
 
     EnumTypeDefinition(node) {
-      validateTypeHasDescription(context, node, 'enum');
+      validateTypeHasDescription(configuration, context, node, 'enum');
     },
 
     InputObjectTypeDefinition(node) {
-      validateTypeHasDescription(context, node, 'input object');
+      validateTypeHasDescription(configuration, context, node, 'input object');
     },
   };
 }

src/runner.js

@@ -28,6 +28,10 @@ export function run(stdout, stdin, stderr, argv) {
       '-p, --custom-rule-paths <paths>',
       'path to additional custom rules to be loaded. Example: rules/*.js'
     )
+    .option(
+      '--comment-descriptions',
+      'use old way of defining descriptions in GraphQL SDL'
+    )
     // DEPRECATED - This code should be removed in v1.0.0.
     .option(
       '-o, --only <rules>',
@@ -84,8 +88,7 @@ export function run(stdout, stdin, stderr, argv) {
   const rules = configuration.getRules();
   const schemaSourceMap = configuration.getSchemaSourceMap();
 
-  const errors = validateSchemaDefinition(schema, rules);
-
+  const errors = validateSchemaDefinition(schema, rules, configuration);
   const groupedErrors = groupErrorsBySchemaFilePath(errors, schemaSourceMap);
 
   stdout.write(formatter(groupedErrors));
@@ -137,6 +140,10 @@ function getOptionsFromCommander(commander) {
     options.customRulePaths = commander.customRulePaths.split(',');
   }
 
+  if (commander.commentDescriptions) {
+    options.commentDescriptions = commander.commentDescriptions;
+  }
+
   if (commander.args && commander.args.length) {
     options.schemaPaths = commander.args;
   }

src/validator.js

@@ -3,7 +3,11 @@ import { validate } from 'graphql/validation';
 import { buildASTSchema } from 'graphql/utilities/buildASTSchema';
 import { GraphQLError } from 'graphql/error';
 
-export function validateSchemaDefinition(schemaDefinition, rules) {
+export function validateSchemaDefinition(
+  schemaDefinition,
+  rules,
+  configuration
+) {
   let ast;
   try {
     ast = parse(schemaDefinition);
@@ -14,8 +18,15 @@ export function validateSchemaDefinition(schemaDefinition, rules) {
       throw e;
     }
   }
-  const schema = buildASTSchema(ast);
-  const errors = validate(schema, ast, rules);
+  const schema = buildASTSchema(ast, {
+    commentDescriptions: configuration.getCommentDescriptions(),
+  });
+
+  const rulesWithConfiguration = rules.map(rule => {
+    return ruleWithConfiguration(rule, configuration);
+  });
+
+  const errors = validate(schema, ast, rulesWithConfiguration);
   const sortedErrors = sortErrors(errors);
 
   return sortedErrors;
@@ -26,3 +37,13 @@ function sortErrors(errors) {
     return a.locations[0].line - b.locations[0].line;
   });
 }
+
+function ruleWithConfiguration(rule, configuration) {
+  if (rule.length == 2) {
+    return function(context) {
+      return rule(configuration, context);
+    };
+  } else {
+    return rule;
+  }
+}

test/assertions.js

@@ -3,19 +3,28 @@ import { parse } from 'graphql';
 import { validate } from 'graphql/validation';
 import { buildASTSchema } from 'graphql/utilities/buildASTSchema';
 import { kebabCase } from 'lodash';
+import { validateSchemaDefinition } from '../src/validator.js';
+import { Configuration } from '../src/configuration.js';
 
 const DefaultSchema = `
-  # Query root
+  "Query root"
   type Query {
-    # Field
+    "Field"
     a: String
   }
 `;
 
 export function expectFailsRule(rule, schemaSDL, expectedErrors = []) {
-  const ast = parse(`${schemaSDL}${DefaultSchema}`);
-  const schema = buildASTSchema(ast);
-  const errors = validate(schema, ast, [rule]);
+  return expectFailsRuleWithConfiguration(rule, schemaSDL, {}, expectedErrors);
+}
+
+export function expectFailsRuleWithConfiguration(
+  rule,
+  schemaSDL,
+  configurationOptions,
+  expectedErrors = []
+) {
+  const errors = validateSchemaWithRule(rule, schemaSDL, configurationOptions);
 
   assert(errors.length > 0, "Expected rule to fail but didn't");
 
@@ -30,13 +39,28 @@ export function expectFailsRule(rule, schemaSDL, expectedErrors = []) {
   );
 }
 
-export function expectPassesRule(rule, schemaSDL, expectedErrors = []) {
-  const ast = parse(`${schemaSDL}${DefaultSchema}`);
-  const schema = buildASTSchema(ast);
-  const errors = validate(schema, ast, [rule]);
+export function expectPassesRule(rule, schemaSDL) {
+  expectPassesRuleWithConfiguration(rule, schemaSDL, {});
+}
+
+export function expectPassesRuleWithConfiguration(
+  rule,
+  schemaSDL,
+  configurationOptions
+) {
+  const errors = validateSchemaWithRule(rule, schemaSDL, configurationOptions);
 
   assert(
     errors.length == 0,
     `Expected rule to pass but didn't got these errors:\n\n${errors.join('\n')}`
   );
 }
+
+function validateSchemaWithRule(rule, schemaSDL, configurationOptions) {
+  const fullSchemaSDL = `${schemaSDL}${DefaultSchema}`;
+  const rules = [rule];
+  const configuration = new Configuration(configurationOptions, null);
+  const errors = validateSchemaDefinition(fullSchemaSDL, rules, configuration);
+
+  return errors;
+}

test/configuration.js

@@ -262,4 +262,16 @@ extend type Query {
       assert.equal(issues.length, 2);
     });
   });
+
+  describe('getCommentDescriptions', () => {
+    it('defaults to false', () => {
+      const configuration = new Configuration({});
+      assert.equal(configuration.getCommentDescriptions(), false);
+    });
+
+    it('returns specified value', () => {
+      const configuration = new Configuration({ commentDescriptions: true });
+      assert.equal(configuration.getCommentDescriptions(), true);
+    });
+  });
 });

test/fixtures/animal.graphql

@@ -1,21 +1,23 @@
-# Base query
+"Base query"
 type Query {
-  # Animal Viewing
+  "Animal Viewing"
   viewer: Animal!
 }
 
-# Animal
+"Animal"
 type Animal {
-  # name
+  "name"
   name: String!
-  # type
+
+  "type"
   types: [AnimalTypes]
 }
 
-# Animal type enum
+"Animal type enum"
 enum AnimalTypes {
-  # meow
+  "meow"
   CAT_ENUM
-  # woof
+
+  "woof"
   DOG_ENUM
 }

test/fixtures/schema.comment-descriptions.graphql

@@ -0,0 +1,7 @@
+# Query
+type Query {
+  a: String
+
+  # B
+  b: String
+}

test/fixtures/valid.graphql

@@ -1,5 +1,5 @@
-# Query
+"Query"
 type Query {
-  # a
+  "a"
   a: String!
 }