diff --git a/src/configuration.js b/src/configuration.js index a6465e68..6d019409 100644 --- a/src/configuration.js +++ b/src/configuration.js @@ -11,12 +11,14 @@ export class Configuration { - format: (required) `text` | `json` - rules: [string array] whitelist rules - customRulePaths: [string array] path to additional custom rules to be loaded + - commentDescriptions: [boolean] use old way of defining descriptions in GraphQL SDL - oldImplementsSyntax: [boolean] use old way of defining implemented interfaces in GraphQL SDL */ constructor(schema, options = {}) { const defaultOptions = { format: 'text', customRulePaths: [], + commentDescriptions: false, oldImplementsSyntax: false, }; @@ -27,6 +29,10 @@ export class Configuration { this.rulePaths = this.options.customRulePaths.concat(this.builtInRulePaths); } + getCommentDescriptions() { + return this.options.commentDescriptions; + } + getOldImplementsSyntax() { return this.options.oldImplementsSyntax; } @@ -55,7 +61,7 @@ export class Configuration { let specifiedRules; if (this.options.rules && this.options.rules.length > 0) { specifiedRules = this.options.rules.map(toUpperCamelCase); - rules = this.getAllRules().filter((rule) => { + rules = this.getAllRules().filter(rule => { return specifiedRules.indexOf(rule.name) >= 0; }); } @@ -63,7 +69,7 @@ export class Configuration { // DEPRECATED - This code should be removed in v1.0.0. if (this.options.only && this.options.only.length > 0) { specifiedRules = this.options.only.map(toUpperCamelCase); - rules = this.getAllRules().filter((rule) => { + rules = this.getAllRules().filter(rule => { return specifiedRules.indexOf(rule.name) >= 0; }); } @@ -71,7 +77,7 @@ export class Configuration { // DEPRECATED - This code should be removed in v1.0.0. if (this.options.except && this.options.except.length > 0) { specifiedRules = this.options.except.map(toUpperCamelCase); - rules = this.getAllRules().filter((rule) => { + rules = this.getAllRules().filter(rule => { return specifiedRules.indexOf(rule.name) == -1; }); } @@ -93,9 +99,9 @@ export class Configuration { const expandedPaths = expandPaths(rulePaths); const rules = new Set([]); - expandedPaths.map((rulePath) => { + expandedPaths.map(rulePath => { let ruleMap = require(rulePath); - Object.keys(ruleMap).forEach((k) => rules.add(ruleMap[k])); + Object.keys(ruleMap).forEach(k => rules.add(ruleMap[k])); }); return Array.from(rules); @@ -128,7 +134,7 @@ export class Configuration { } } - const ruleNames = rules.map((rule) => rule.name); + const ruleNames = rules.map(rule => rule.name); let misConfiguredRuleNames = [] .concat( @@ -137,7 +143,7 @@ export class Configuration { this.options.rules || [] ) .map(toUpperCamelCase) - .filter((name) => ruleNames.indexOf(name) == -1); + .filter(name => ruleNames.indexOf(name) == -1); if (this.getFormatter() == null) { issues.push({ @@ -164,6 +170,6 @@ export class Configuration { function toUpperCamelCase(string) { return string .split('-') - .map((part) => part[0].toUpperCase() + part.slice(1)) + .map(part => part[0].toUpperCase() + part.slice(1)) .join(''); } diff --git a/src/rules/arguments_have_descriptions.js b/src/rules/arguments_have_descriptions.js index 5930fe5c..65ab822a 100644 --- a/src/rules/arguments_have_descriptions.js +++ b/src/rules/arguments_have_descriptions.js @@ -1,4 +1,4 @@ -import { getDescription } from 'graphql/utilities/buildASTSchema'; +import { getDescription } from 'graphql/utilities/extendSchema'; import { ValidationError } from '../validation_error'; export function ArgumentsHaveDescriptions(configuration, context) { @@ -7,19 +7,21 @@ export function ArgumentsHaveDescriptions(configuration, context) { const fieldName = node.name.value; for (const arg of node.arguments || []) { - if (arg.description && arg.description.value != '') { - continue; - } + const description = getDescription(arg, { + commentDescriptions: configuration.getCommentDescriptions(), + }); - const argName = arg.name.value; + if (typeof description !== 'string' || description.length === 0) { + const argName = arg.name.value; - context.reportError( - new ValidationError( - 'arguments-have-descriptions', - `The \`${argName}\` argument of \`${fieldName}\` is missing a description.`, - [arg] - ) - ); + context.reportError( + new ValidationError( + 'arguments-have-descriptions', + `The \`${argName}\` argument of \`${fieldName}\` is missing a description.`, + [arg] + ) + ); + } } }, }; diff --git a/src/rules/descriptions_are_capitalized.js b/src/rules/descriptions_are_capitalized.js index 91c31edd..155a8323 100644 --- a/src/rules/descriptions_are_capitalized.js +++ b/src/rules/descriptions_are_capitalized.js @@ -1,16 +1,16 @@ -import { getDescription } from 'graphql/utilities/buildASTSchema'; +import { getDescription } from 'graphql/utilities/extendSchema'; import { ValidationError } from '../validation_error'; export function DescriptionsAreCapitalized(configuration, context) { return { FieldDefinition(node, key, parent, path, ancestors) { + const description = getDescription(node, { + commentDescriptions: configuration.getCommentDescriptions(), + }); + // Rule should pass if there's an empty/missing string description. If empty // strings aren't wanted, the `*_have_descriptions` rules can be used. - if (!node.description || node.description.value == '') { - return; - } - - const description = node.description.value; + if (typeof description !== 'string' || description.length === 0) return; // It's possible there could be some markdown characters that do not // pass this test. If we discover some examples of this, we can improve. diff --git a/src/rules/enum_values_have_descriptions.js b/src/rules/enum_values_have_descriptions.js index 8b4a5e87..20e41b0b 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 { getDescription } from 'graphql/utilities/extendSchema'; import { ValidationError } from '../validation_error'; export function EnumValuesHaveDescriptions(configuration, context) { return { EnumValueDefinition(node, key, parent, path, ancestors) { - if (node.description && node.description.value != '') { + if ( + getDescription(node, { + commentDescriptions: configuration.getCommentDescriptions(), + }) + ) { return; } diff --git a/src/rules/fields_have_descriptions.js b/src/rules/fields_have_descriptions.js index c70e0f6c..223382ae 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 { getDescription } from 'graphql/utilities/extendSchema'; import { ValidationError } from '../validation_error'; export function FieldsHaveDescriptions(configuration, context) { return { FieldDefinition(node, key, parent, path, ancestors) { - if (node.description && node.description.value != '') { + 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 b5061eff..30b53c01 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 { getDescription } from 'graphql/utilities/extendSchema'; import { ValidationError } from '../validation_error'; export function InputObjectValuesHaveDescriptions(configuration, context) { return { InputValueDefinition(node, key, parent, path, ancestors) { - if (node.description && node.description.value != '') { + if ( + getDescription(node, { + commentDescriptions: configuration.getCommentDescriptions(), + }) + ) { return; } diff --git a/src/rules/types_have_descriptions.js b/src/rules/types_have_descriptions.js index 34e81cac..83774f18 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 { getDescription } from 'graphql/utilities/extendSchema'; import { ValidationError } from '../validation_error'; function validateTypeHasDescription(configuration, context, node, typeKind) { - if (node.description && node.description.value != '') { + if ( + getDescription(node, { + commentDescriptions: configuration.getCommentDescriptions(), + }) + ) { return; } diff --git a/src/runner.js b/src/runner.js index 463fc094..0c305c9c 100644 --- a/src/runner.js +++ b/src/runner.js @@ -30,6 +30,10 @@ export async 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' + ) .option( '--old-implements-syntax', 'use old way of defining implemented interfaces in GraphQL SDL' @@ -77,7 +81,7 @@ export async function run(stdout, stdin, stderr, argv) { const issues = configuration.validate(); - issues.map((issue) => { + issues.map(issue => { var prefix; if (issue.type == 'error') { prefix = `${chalk.red(figures.cross)} Error`; @@ -89,7 +93,7 @@ export async function run(stdout, stdin, stderr, argv) { ); }); - if (issues.some((issue) => issue.type == 'error')) { + if (issues.some(issue => issue.type == 'error')) { return 2; } @@ -152,6 +156,10 @@ function getOptionsFromCommander(commander) { options.customRulePaths = commander.customRulePaths.split(','); } + if (commander.commentDescriptions) { + options.commentDescriptions = commander.commentDescriptions; + } + if (commander.oldImplementsSyntax) { options.oldImplementsSyntax = commander.oldImplementsSyntax; } diff --git a/src/validator.js b/src/validator.js index d761e487..b96c22f3 100644 --- a/src/validator.js +++ b/src/validator.js @@ -33,7 +33,7 @@ export function validateSchemaDefinition( let schemaErrors = validateSDL(ast); if (schemaErrors.length > 0) { return sortErrors( - schemaErrors.map((error) => { + schemaErrors.map(error => { return new ValidationError( 'invalid-graphql-schema', error.message, @@ -44,6 +44,7 @@ export function validateSchemaDefinition( } const schema = buildASTSchema(ast, { + commentDescriptions: configuration.getCommentDescriptions(), assumeValidSDL: true, assumeValid: true, }); @@ -52,7 +53,7 @@ export function validateSchemaDefinition( schemaErrors = validateSchema(schema); if (schemaErrors.length > 0) { return sortErrors( - schemaErrors.map((error) => { + schemaErrors.map(error => { return new ValidationError( 'invalid-graphql-schema', error.message, @@ -62,7 +63,7 @@ export function validateSchemaDefinition( ); } - const rulesWithConfiguration = rules.map((rule) => { + const rulesWithConfiguration = rules.map(rule => { return ruleWithConfiguration(rule, configuration); }); @@ -80,7 +81,7 @@ function sortErrors(errors) { function ruleWithConfiguration(rule, configuration) { if (rule.length == 2) { - return function (context) { + return function(context) { return rule(configuration, context); }; } else { diff --git a/test/configuration.js b/test/configuration.js index e76e971a..8b05dd59 100644 --- a/test/configuration.js +++ b/test/configuration.js @@ -43,13 +43,13 @@ describe('Configuration', () => { assert.equal(rules.length, 2); assert.equal( rules[0], - configuration.getAllRules().find((rule) => { + configuration.getAllRules().find(rule => { return rule.name == 'FieldsHaveDescriptions'; }) ); assert.equal( rules[1], - configuration.getAllRules().find((rule) => { + configuration.getAllRules().find(rule => { return rule.name == 'TypesHaveDescriptions'; }) ); @@ -65,7 +65,7 @@ describe('Configuration', () => { assert.equal(rules.length, configuration.getAllRules().length - 2); assert.equal( 0, - rules.filter((rule) => { + rules.filter(rule => { return ( rule.name == 'FieldsHaveDescriptions' || rule.name == 'TypesHaveDescriptions' @@ -84,13 +84,13 @@ describe('Configuration', () => { assert.equal(rules.length, 2); assert.equal( rules[0], - configuration.getAllRules().find((rule) => { + configuration.getAllRules().find(rule => { return rule.name == 'FieldsHaveDescriptions'; }) ); assert.equal( rules[1], - configuration.getAllRules().find((rule) => { + configuration.getAllRules().find(rule => { return rule.name == 'TypesHaveDescriptions'; }) ); @@ -106,7 +106,7 @@ describe('Configuration', () => { assert.equal(rules.length, configuration.getAllRules().length - 2); assert.equal( 0, - rules.filter((rule) => { + rules.filter(rule => { return ( rule.name == 'FieldsHaveDescriptions' || rule.name == 'TypesHaveDescriptions' @@ -127,7 +127,7 @@ describe('Configuration', () => { assert.equal( 2, - rules.filter((rule) => { + rules.filter(rule => { return ( rule.name == 'EnumNameCannotContainEnum' || rule.name == 'TypeNameCannotContainType' @@ -145,7 +145,7 @@ describe('Configuration', () => { assert.equal( 4, - rules.filter((rule) => { + rules.filter(rule => { return ( rule.name == 'SomeRule' || rule.name == 'AnotherRule' || @@ -167,7 +167,7 @@ describe('Configuration', () => { assert.equal( 1, - rules.filter((rule) => { + rules.filter(rule => { return rule.name == 'TypeNameCannotContainType'; }).length ); @@ -238,6 +238,20 @@ describe('Configuration', () => { }); }); + describe('getCommentDescriptions', () => { + it('defaults to false', () => { + const configuration = new Configuration(emptySchema, {}); + assert.equal(configuration.getCommentDescriptions(), false); + }); + + it('returns specified value', () => { + const configuration = new Configuration(emptySchema, { + commentDescriptions: true, + }); + assert.equal(configuration.getCommentDescriptions(), true); + }); + }); + describe('getOldImplementsSyntax', () => { it('defaults to false', () => { const configuration = new Configuration(emptySchema, {}); diff --git a/test/rules/arguments_have_descriptions.js b/test/rules/arguments_have_descriptions.js index 8b02579b..9154a772 100644 --- a/test/rules/arguments_have_descriptions.js +++ b/test/rules/arguments_have_descriptions.js @@ -49,4 +49,22 @@ describe('ArgumentsHaveDescriptions rule', () => { ] ); }); + + it('gets descriptions correctly with commentDescriptions option', () => { + expectPassesRuleWithConfiguration( + ArgumentsHaveDescriptions, + ` + type Box { + widget( + "Widget ID" + id: Int + + # Widget type + type: String + ): String! + } + `, + { commentDescriptions: true } + ); + }); }); diff --git a/test/rules/descriptions_are_capitalized.js b/test/rules/descriptions_are_capitalized.js index 572fd12e..e4cf81a3 100644 --- a/test/rules/descriptions_are_capitalized.js +++ b/test/rules/descriptions_are_capitalized.js @@ -28,6 +28,29 @@ describe('DescriptionsAreCapitalized rule', () => { ); }); + it('detects lowercase field descriptions with commentDescriptions option', () => { + expectFailsRuleWithConfiguration( + DescriptionsAreCapitalized, + ` + type Widget { + # widget name + name: String! + + # Valid description + other: Int + } + `, + { commentDescriptions: true }, + [ + { + message: + 'The description for field `Widget.name` should be capitalized.', + locations: [{ line: 4, column: 9 }], + }, + ] + ); + }); + it('does not err on an empty description', () => { expectPassesRule( DescriptionsAreCapitalized, diff --git a/test/rules/enum_values_have_descriptions.js b/test/rules/enum_values_have_descriptions.js index 7ac06d04..1cdccc22 100644 --- a/test/rules/enum_values_have_descriptions.js +++ b/test/rules/enum_values_have_descriptions.js @@ -31,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 8b991e19..0caa9b2e 100644 --- a/test/rules/fields_have_descriptions.js +++ b/test/rules/fields_have_descriptions.js @@ -30,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 fcf4e7f8..baaffe7d 100644 --- a/test/rules/input_object_values_have_descriptions.js +++ b/test/rules/input_object_values_have_descriptions.js @@ -41,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 bb4177b3..9713e18a 100644 --- a/test/rules/types_have_descriptions.js +++ b/test/rules/types_have_descriptions.js @@ -161,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 2b1a47ed..055048a3 100644 --- a/test/runner.js +++ b/test/runner.js @@ -7,14 +7,14 @@ import { stripAnsi } from './strip_ansi.js'; describe('Runner', () => { var stdout; var mockStdout = { - write: (text) => { + write: text => { stdout = stdout + text; }, }; var stderr; var mockStderr = { - write: (text) => { + write: text => { stderr = stderr + text; }, }; @@ -107,6 +107,29 @@ describe('Runner', () => { assert.equal(0, exitCode); }); + it('allows setting descriptions using comments in GraphQL SDL', async () => { + const argv = [ + 'node', + 'lib/cli.js', + '--format', + 'text', + '--comment-descriptions', + '--rules', + 'fields-have-descriptions', + `${__dirname}/fixtures/schema.comment-descriptions.graphql`, + ]; + + await 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('allows using old `implements` syntax in GraphQL SDL', async () => { const argv = [ 'node',