diff --git a/README.md b/README.md index cd971a6..2becc43 100644 --- a/README.md +++ b/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 to begin searching for config files diff --git a/package.json b/package.json index 6e83a4e..4954d43 100644 --- a/package.json +++ b/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" }, diff --git a/src/configuration.js b/src/configuration.js index d6ab735..341b90b 100644 --- a/src/configuration.js +++ b/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; diff --git a/src/rules/enum_values_have_descriptions.js b/src/rules/enum_values_have_descriptions.js index c8cf617..38867ac 100644 --- a/src/rules/enum_values_have_descriptions.js +++ b/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; } diff --git a/src/rules/fields_have_descriptions.js b/src/rules/fields_have_descriptions.js index ce4e0ba..eb45e8c 100644 --- a/src/rules/fields_have_descriptions.js +++ b/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; } diff --git a/src/rules/input_object_values_have_descriptions.js b/src/rules/input_object_values_have_descriptions.js index fbda698..0234ea7 100644 --- a/src/rules/input_object_values_have_descriptions.js +++ b/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; } diff --git a/src/rules/types_have_descriptions.js b/src/rules/types_have_descriptions.js index 87fa6a4..8acb1d0 100644 --- a/src/rules/types_have_descriptions.js +++ b/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'); }, }; } diff --git a/src/runner.js b/src/runner.js index 089edd0..84ae01c 100644 --- a/src/runner.js +++ b/src/runner.js @@ -28,6 +28,10 @@ export function run(stdout, stdin, stderr, argv) { '-p, --custom-rule-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 ', @@ -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; } diff --git a/src/validator.js b/src/validator.js index 83cce30..afcc9cc 100644 --- a/src/validator.js +++ b/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; + } +} diff --git a/test/assertions.js b/test/assertions.js index 2c12437..5b8d2a2 100644 --- a/test/assertions.js +++ b/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; +} diff --git a/test/configuration.js b/test/configuration.js index 9b3f1f9..c65407f 100644 --- a/test/configuration.js +++ b/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); + }); + }); }); diff --git a/test/fixtures/animal.graphql b/test/fixtures/animal.graphql index 63849c5..393dac9 100644 --- a/test/fixtures/animal.graphql +++ b/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 } diff --git a/test/fixtures/schema.comment-descriptions.graphql b/test/fixtures/schema.comment-descriptions.graphql new file mode 100644 index 0000000..48f5b57 --- /dev/null +++ b/test/fixtures/schema.comment-descriptions.graphql @@ -0,0 +1,7 @@ +# Query +type Query { + a: String + + # B + b: String +} diff --git a/test/fixtures/valid.graphql b/test/fixtures/valid.graphql index ea46408..1fc23ae 100644 --- a/test/fixtures/valid.graphql +++ b/test/fixtures/valid.graphql @@ -1,5 +1,5 @@ -# Query +"Query" type Query { - # a + "a" a: String! } diff --git a/test/rules/enum_values_have_descriptions.js b/test/rules/enum_values_have_descriptions.js index 170861b..1cdccc2 100644 --- a/test/rules/enum_values_have_descriptions.js +++ b/test/rules/enum_values_have_descriptions.js @@ -1,5 +1,8 @@ import { EnumValuesHaveDescriptions } from '../../src/rules/enum_values_have_descriptions'; -import { expectFailsRule } from '../assertions'; +import { + expectFailsRule, + expectPassesRuleWithConfiguration, +} from '../assertions'; describe('EnumValuesHaveDescriptions rule', () => { it('catches enum values that have no description', () => { @@ -9,7 +12,7 @@ describe('EnumValuesHaveDescriptions rule', () => { enum Status { DRAFT - # Hidden + "Hidden" HIDDEN PUBLISHED @@ -28,4 +31,17 @@ describe('EnumValuesHaveDescriptions rule', () => { ] ); }); + + it('get descriptions correctly with commentDescriptions option', () => { + expectPassesRuleWithConfiguration( + EnumValuesHaveDescriptions, + ` + enum Status { + # Hidden + HIDDEN + } + `, + { commentDescriptions: true } + ); + }); }); diff --git a/test/rules/fields_have_descriptions.js b/test/rules/fields_have_descriptions.js index a2c692d..0caa9b2 100644 --- a/test/rules/fields_have_descriptions.js +++ b/test/rules/fields_have_descriptions.js @@ -1,5 +1,8 @@ import { FieldsHaveDescriptions } from '../../src/rules/fields_have_descriptions'; -import { expectFailsRule } from '../assertions'; +import { + expectFailsRule, + expectPassesRuleWithConfiguration, +} from '../assertions'; describe('FieldsHaveDescriptions rule', () => { it('catches fields that have no description', () => { @@ -10,7 +13,7 @@ describe('FieldsHaveDescriptions rule', () => { withoutDescription: String withoutDescriptionAgain: String! - # Description + "Description" withDescription: String } `, @@ -27,4 +30,17 @@ describe('FieldsHaveDescriptions rule', () => { ] ); }); + + it('gets descriptions correctly with commentDescriptions option', () => { + expectPassesRuleWithConfiguration( + FieldsHaveDescriptions, + ` + type A { + "Description" + withDescription: String + } + `, + { commentDescriptions: true } + ); + }); }); diff --git a/test/rules/input_object_values_have_descriptions.js b/test/rules/input_object_values_have_descriptions.js index c2ca76e..baaffe7 100644 --- a/test/rules/input_object_values_have_descriptions.js +++ b/test/rules/input_object_values_have_descriptions.js @@ -4,7 +4,11 @@ import { validate } from 'graphql/validation'; import { buildASTSchema } from 'graphql/utilities/buildASTSchema'; import { InputObjectValuesHaveDescriptions } from '../../src/rules/input_object_values_have_descriptions'; -import { expectFailsRule, expectPassesRule } from '../assertions'; +import { + expectFailsRule, + expectPassesRule, + expectPassesRuleWithConfiguration, +} from '../assertions'; describe('InputObjectValuesHaveDescriptions rule', () => { it('catches input object type values that have no description', () => { @@ -14,7 +18,7 @@ describe('InputObjectValuesHaveDescriptions rule', () => { input User { username: String - # Description + "Description" withDescription: String } `, @@ -37,4 +41,17 @@ describe('InputObjectValuesHaveDescriptions rule', () => { ` ); }); + + it('gets descriptions correctly with commentDescriptions option', () => { + expectPassesRuleWithConfiguration( + InputObjectValuesHaveDescriptions, + ` + input F { + # F + f: String + } + `, + { commentDescriptions: true } + ); + }); }); diff --git a/test/rules/types_have_descriptions.js b/test/rules/types_have_descriptions.js index abfacdf..30df47e 100644 --- a/test/rules/types_have_descriptions.js +++ b/test/rules/types_have_descriptions.js @@ -1,12 +1,16 @@ import { TypesHaveDescriptions } from '../../src/rules/types_have_descriptions'; -import { expectFailsRule, expectPassesRule } from '../assertions'; +import { + expectFailsRule, + expectPassesRule, + expectPassesRuleWithConfiguration, +} from '../assertions'; describe('TypesHaveDescriptions rule', () => { it('catches enum types that have no description', () => { expectFailsRule( TypesHaveDescriptions, ` - # A + "A" enum A { A } @@ -30,7 +34,7 @@ describe('TypesHaveDescriptions rule', () => { expectFailsRule( TypesHaveDescriptions, ` - # A + "A" scalar A scalar DateTime @@ -52,7 +56,7 @@ describe('TypesHaveDescriptions rule', () => { a: String } - # B + "B" type B { b: String } @@ -74,7 +78,7 @@ describe('TypesHaveDescriptions rule', () => { id: ID! } - # RemoveStar + "RemoveStar" input RemoveStar { id: ID! } @@ -92,7 +96,7 @@ describe('TypesHaveDescriptions rule', () => { expectFailsRule( TypesHaveDescriptions, ` - # B + "B" interface B { B: String } @@ -114,19 +118,19 @@ describe('TypesHaveDescriptions rule', () => { expectFailsRule( TypesHaveDescriptions, ` - # A + "A" type A { a: String } - # B + "B" type B { b: String } union AB = A | B - # BA + "BA" union BA = B | A `, [ @@ -146,7 +150,7 @@ describe('TypesHaveDescriptions rule', () => { b: String } - # Interface + "Interface" interface Vehicle { make: String! } @@ -157,4 +161,38 @@ describe('TypesHaveDescriptions rule', () => { ` ); }); + + it('gets descriptions correctly with commentDescriptions option', () => { + expectPassesRuleWithConfiguration( + TypesHaveDescriptions, + ` + # A + scalar A + + # B + type B { + b: String + } + + # C + interface C { + c: String + } + + # D + union D = B + + # E + enum E { + A + } + + # F + input F { + f: String + } + `, + { commentDescriptions: true } + ); + }); }); diff --git a/test/runner.js b/test/runner.js index 3f467a7..2186c40 100644 --- a/test/runner.js +++ b/test/runner.js @@ -70,6 +70,27 @@ describe('Runner', () => { assert.equal(0, exitCode); }); + it('allows setting descriptions using comments in GraphQL SDL', () => { + const argv = [ + 'node', + 'lib/cli.js', + '--format', + 'text', + '--comment-descriptions', + `${__dirname}/fixtures/schema.comment-descriptions.graphql`, + ]; + + run(mockStdout, mockStdin, mockStderr, argv); + + const expected = + `${__dirname}/fixtures/schema.comment-descriptions.graphql\n` + + '3:3 The field `Query.a` is missing a description. fields-have-descriptions\n' + + '\n' + + '✖ 1 error detected\n'; + + assert.equal(expected, stripAnsi(stdout)); + }); + it('validates a single schema file and outputs in text', () => { const argv = [ 'node', @@ -108,7 +129,7 @@ describe('Runner', () => { const expected = `${__dirname}/fixtures/animal.graphql\n` + "18:3 The enum value `AnimalTypes.CAT_ENUM` cannot include the word 'enum'. enum-name-cannot-contain-enum\n" + - "20:3 The enum value `AnimalTypes.DOG_ENUM` cannot include the word 'enum'. enum-name-cannot-contain-enum\n" + + "21:3 The enum value `AnimalTypes.DOG_ENUM` cannot include the word 'enum'. enum-name-cannot-contain-enum\n" + '\n' + '✖ 2 errors detected\n'; diff --git a/test/validator.js b/test/validator.js index afcc230..6dd4b5e 100644 --- a/test/validator.js +++ b/test/validator.js @@ -12,7 +12,11 @@ describe('validateSchemaDefinition', () => { const schemaDefinition = configuration.getSchema(); const rules = [FieldsHaveDescriptions, DummyValidator]; - const errors = validateSchemaDefinition(schemaDefinition, rules); + const errors = validateSchemaDefinition( + schemaDefinition, + rules, + configuration + ); const errorLineNumbers = errors.map(error => { return error.locations[0].line; }); @@ -28,10 +32,40 @@ describe('validateSchemaDefinition', () => { const schemaDefinition = configuration.getSchema(); - const errors = validateSchemaDefinition(schemaDefinition, []); + const errors = validateSchemaDefinition( + schemaDefinition, + [], + configuration + ); assert.equal(1, errors.length); }); + + it('passes configuration to rules that require it', () => { + const schemaPath = `${__dirname}/fixtures/valid.graphql`; + const configuration = new Configuration({ schemaPaths: [schemaPath] }); + + const schemaDefinition = configuration.getSchema(); + + const ruleWithConfiguration = (config, context) => { + assert.equal(configuration, config); + assert.equal('ValidationContext', context.constructor.name); + return {}; + }; + + const ruleWithoutConfiguration = context => { + assert.equal('ValidationContext', context.constructor.name); + return {}; + }; + + const errors = validateSchemaDefinition( + schemaDefinition, + [ruleWithConfiguration, ruleWithoutConfiguration], + configuration + ); + + assert.equal(0, errors.length); + }); }); function DummyValidator(context) { diff --git a/yarn.lock b/yarn.lock index ef5c992..506d47c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1466,12 +1466,18 @@ graphql-request@^1.2.0: dependencies: isomorphic-fetch "^2.2.1" -"graphql@0.7.1 - 1.0.0", graphql@^0.10.1: +"graphql@0.7.1 - 1.0.0": version "0.10.5" resolved "https://registry.yarnpkg.com/graphql/-/graphql-0.10.5.tgz#c9be17ca2bdfdbd134077ffd9bbaa48b8becd298" dependencies: iterall "^1.1.0" +graphql@^0.13.0: + version "0.13.0" + resolved "https://registry.yarnpkg.com/graphql/-/graphql-0.13.0.tgz#d1b44a282279a9ce0a6ec1037329332f4c1079b6" + dependencies: + iterall "1.1.x" + growl@1.9.2: version "1.9.2" resolved "https://registry.yarnpkg.com/growl/-/growl-1.9.2.tgz#0ea7743715db8d8de2c5ede1775e1b45ac85c02f" @@ -1746,6 +1752,10 @@ isstream@~0.1.2: version "0.1.2" resolved "https://registry.yarnpkg.com/isstream/-/isstream-0.1.2.tgz#47e63f7af55afa6f92e1500e690eb8b8529c099a" +iterall@1.1.x: + version "1.1.4" + resolved "https://registry.yarnpkg.com/iterall/-/iterall-1.1.4.tgz#0db40d38fdcf53ae14dc8ec674e62ab190d52cfc" + iterall@^1.1.0: version "1.1.1" resolved "https://registry.yarnpkg.com/iterall/-/iterall-1.1.1.tgz#f7f0af11e9a04ec6426260f5019d9fcca4d50214"