Repository: stoplightio/spectral
Pull request #157 (2019-04-19 19:55:10)

Feature/SO-106/add config file support
Work in progress PR adding config file support.
Open for suggestions.


mallachari: Thanks for reviews! 
I have just added final ruleset support for config files. It can go either as a string or an array of paths.
Please take a look and let me know if you notice anything to correct.

Patch:
diff --git a/src/cli/commands/lint.ts b/src/cli/commands/lint.ts
index c6f27f30..8823c35b 100644
--- a/src/cli/commands/lint.ts
+++ b/src/cli/commands/lint.ts
@@ -2,9 +2,11 @@ import { Command, flags as flagHelpers } from '@oclif/command';
 import { IParserResult } from '@stoplight/types';
 import { getLocationForJsonPath } from '@stoplight/yaml';
 import { writeFile } from 'fs';
+import { isNil, omitBy } from 'lodash';
 import { resolve } from 'path';
 import { promisify } from 'util';
 import { IRuleResult } from '../..';
+import { createEmptyConfig, getDefaultConfigFile, load as loadConfig } from '../../config/configLoader';
 import { json, stylish } from '../../formatters';
 import { readParsable } from '../../fs/reader';
 import { oas2Functions, oas2Rules } from '../../rulesets/oas2';
@@ -12,6 +14,7 @@ import { oas3Functions, oas3Rules } from '../../rulesets/oas3';
 import { readRulesets } from '../../rulesets/reader';
 import { Spectral } from '../../spectral';
 import { IParsedResult, RuleCollection } from '../../types';
+import { ConfigCommand, IConfig, ILintConfig } from '../../types/config';
 
 const writeFileAsync = promisify(writeFile);
 export default class Lint extends Command {
@@ -27,12 +30,10 @@ linting ./openapi.yaml
     help: flagHelpers.help({ char: 'h' }),
     encoding: flagHelpers.string({
       char: 'e',
-      default: 'utf8',
       description: 'text encoding to use',
     }),
     format: flagHelpers.string({
       char: 'f',
-      default: 'stylish',
       description: 'formatter to use for outputting results',
       options: ['json', 'stylish'],
     }),
@@ -48,6 +49,10 @@ linting ./openapi.yaml
       char: 'v',
       description: 'increase verbosity',
     }),
+    config: flagHelpers.string({
+      char: 'c',
+      description: 'path to a config file',
+    }),
     ruleset: flagHelpers.string({
       char: 'r',
       description: 'path to a ruleset file (supports remote files)',
@@ -59,7 +64,20 @@ linting ./openapi.yaml
 
   public async run() {
     const { args, flags } = this.parse(Lint);
-    const { ruleset } = flags;
+    const { config: configFileFlag } = flags;
+
+    let config: ILintConfig = mergeConfig(createEmptyConfig(), flags);
+
+    const configFile = configFileFlag || getDefaultConfigFile(process.cwd()) || null;
+    if (configFile) {
+      try {
+        const loadedConfig = await loadConfig(configFile, ConfigCommand.LINT);
+        config = mergeConfig(loadedConfig, flags);
+      } catch (ex) {
+        this.error(`Cannot load provided config file. ${ex.message}.`);
+      }
+    }
+    const { ruleset } = config;
     let rules;
 
     if (ruleset) {
@@ -72,7 +90,7 @@ linting ./openapi.yaml
 
     if (args.source) {
       try {
-        await lint(args.source, flags, this, rules);
+        await lint(args.source, config, this, rules);
       } catch (ex) {
         this.error(ex.message);
       }
@@ -147,3 +165,20 @@ export async function writeOutput(outputStr: string, flags: any, command: Lint)
 
   command.log(outputStr);
 }
+
+function mergeConfig(config: IConfig, flags: any): ILintConfig {
+  return {
+    ...config.lint,
+    ...omitBy<ILintConfig>(
+      {
+        encoding: flags.encoding,
+        format: flags.format,
+        output: flags.output,
+        maxResults: flags.maxResults,
+        verbose: flags.verbose,
+        ruleset: flags.ruleset,
+      },
+      isNil
+    ),
+  };
+}
diff --git a/src/config/configLoader.ts b/src/config/configLoader.ts
new file mode 100644
index 00000000..0b9fd8e3
--- /dev/null
+++ b/src/config/configLoader.ts
@@ -0,0 +1,59 @@
+import { IParserResult } from '@stoplight/types';
+import { existsSync } from 'fs-extra';
+import { isEmpty, merge } from 'lodash';
+import * as path from 'path';
+import { readParsable } from '../fs/reader';
+import { ConfigCommand, ConfigFormat, IConfig } from '../types/config';
+
+const DEFAULT_CONFIG_FILE = 'spectral.yml';
+
+const loadConfig = async (filePath: any): Promise<IConfig> => {
+  switch (path.extname(filePath)) {
+    case '.json':
+    case '.yaml':
+    case '.yml':
+      const { data, diagnostics }: IParserResult = await readParsable(filePath, 'utf8');
+      if (!isEmpty(diagnostics)) {
+        throw new Error('Parsing exception');
+      }
+      return data;
+    default:
+      throw new Error('Unknown file');
+  }
+};
+
+export const load = async (filePath: string, command: ConfigCommand): Promise<IConfig> => {
+  const config: IConfig = await loadConfig(filePath);
+  validate(config, command);
+  normalize(config, command);
+  return merge(createEmptyConfig(), config);
+};
+
+export const validate = (config: IConfig, command: ConfigCommand) => {
+  if (!config) throw new Error('Missing config');
+  if (!config[command]) throw new Error(`Missing ${command} command in config file`);
+};
+
+export const normalize = (config: IConfig, command: ConfigCommand) => {
+  switch (command) {
+    case ConfigCommand.LINT:
+      if (config.lint.ruleset) {
+        const { ruleset } = config.lint;
+        config.lint.ruleset = Array.isArray(ruleset) ? ruleset : [ruleset];
+      }
+      break;
+  }
+};
+
+export const createEmptyConfig = (): IConfig => ({
+  lint: {
+    encoding: 'utf8',
+    format: ConfigFormat.STYLISH,
+    verbose: false,
+  },
+});
+
+export const getDefaultConfigFile = (directory: string) => {
+  const filePath = path.join(directory, DEFAULT_CONFIG_FILE);
+  return existsSync(filePath) ? filePath : null;
+};
diff --git a/src/types/config.ts b/src/types/config.ts
new file mode 100644
index 00000000..e6085dd1
--- /dev/null
+++ b/src/types/config.ts
@@ -0,0 +1,22 @@
+export enum ConfigFormat {
+  JSON = 'json',
+  STYLISH = 'stylish',
+}
+
+export enum ConfigCommand {
+  LINT = 'lint',
+}
+
+export interface ILintConfig {
+  encoding: string;
+  format: ConfigFormat;
+  maxResults?: number;
+  output?: string;
+  verbose: boolean;
+  ruleset?: string[];
+}
+
+export interface IConfig {
+  extends?: string;
+  lint: ILintConfig;
+}


Test patch:
diff --git a/src/cli/commands/__tests__/__fixtures__/config.output.yml b/src/cli/commands/__tests__/__fixtures__/config.output.yml
new file mode 100644
index 00000000..8f14af88
--- /dev/null
+++ b/src/cli/commands/__tests__/__fixtures__/config.output.yml
@@ -0,0 +1,3 @@
+---
+lint:
+  output: results.json
diff --git a/src/cli/commands/__tests__/__fixtures__/config.ruleset.invalid.yml b/src/cli/commands/__tests__/__fixtures__/config.ruleset.invalid.yml
new file mode 100644
index 00000000..8b1a75f9
--- /dev/null
+++ b/src/cli/commands/__tests__/__fixtures__/config.ruleset.invalid.yml
@@ -0,0 +1,4 @@
+---
+lint:
+  ruleset:
+    - src/cli/commands/__tests__/__fixtures__/ruleset-invalid.yaml
diff --git a/src/cli/commands/__tests__/__fixtures__/config.ruleset.yml b/src/cli/commands/__tests__/__fixtures__/config.ruleset.yml
new file mode 100644
index 00000000..82d933c3
--- /dev/null
+++ b/src/cli/commands/__tests__/__fixtures__/config.ruleset.yml
@@ -0,0 +1,4 @@
+---
+lint:
+  ruleset:
+    - src/cli/commands/__tests__/__fixtures__/ruleset-valid.yaml
diff --git a/src/cli/commands/__tests__/__fixtures__/config.yml b/src/cli/commands/__tests__/__fixtures__/config.yml
new file mode 100644
index 00000000..c408dcec
--- /dev/null
+++ b/src/cli/commands/__tests__/__fixtures__/config.yml
@@ -0,0 +1,6 @@
+---
+lint:
+  encoding: utf8
+  format: json
+  maxResults: 5
+  verbose: true
diff --git a/src/cli/commands/__tests__/__fixtures__/spectral.yml b/src/cli/commands/__tests__/__fixtures__/spectral.yml
new file mode 100644
index 00000000..9d07456d
--- /dev/null
+++ b/src/cli/commands/__tests__/__fixtures__/spectral.yml
@@ -0,0 +1,5 @@
+---
+lint:
+  encoding: utf8
+  format: json
+  maxResults: 1
diff --git a/src/cli/commands/__tests__/lint.test.ts b/src/cli/commands/__tests__/lint.test.ts
index 083cf13a..235f8a7d 100644
--- a/src/cli/commands/__tests__/lint.test.ts
+++ b/src/cli/commands/__tests__/lint.test.ts
@@ -1,14 +1,19 @@
 import { test } from '@oclif/test';
 import * as fs from 'fs';
 import { resolve } from 'path';
+import SpyInstance = jest.SpyInstance;
 
 const invalidSpecPath = resolve(__dirname, '__fixtures__/openapi-3.0-no-contact.yaml');
 const validSpecPath = resolve(__dirname, '__fixtures__/openapi-3.0-valid.yaml');
+const validConfigPath = resolve(__dirname, '__fixtures__/config.yml');
+const outputConfigPath = resolve(__dirname, '__fixtures__/config.output.yml');
 const validCustomSpecPath = resolve(__dirname, '__fixtures__/openapi-3.0-valid-custom.yaml');
 const invalidRulesetPath = resolve(__dirname, '__fixtures__/ruleset-invalid.yaml');
 const validRulesetPath = resolve(__dirname, '__fixtures__/ruleset-valid.yaml');
 const validNestedRulesetPath = resolve(__dirname, '__fixtures__/ruleset-extends-valid.yaml');
 const invalidNestedRulesetPath = resolve(__dirname, '__fixtures__/ruleset-extends-invalid.yaml');
+const validRulesetConfigPath = resolve(__dirname, '__fixtures__/config.ruleset.yml');
+const invalidRulesetConfigPath = resolve(__dirname, '__fixtures__/config.ruleset.invalid.yml');
 
 /*
  * These tests currently do not assert stderr because it doesn't seem to be
@@ -201,4 +206,78 @@ describe('lint', () => {
         expect(ctx.stdout).toContain('Info object should contain `contact` object');
       });
   });
+
+  describe('when using config file', () => {
+    test
+      .stdout()
+      .command(['lint', invalidSpecPath, '-c', validConfigPath])
+      .it('outputs warnings in json format', ctx => {
+        expect(ctx.stdout).toContain('"info.contact is not truthy"');
+        expect(ctx.stdout).toContain('"info.description is not truthy"');
+        expect(ctx.stdout).toContain('"servers does not exist"');
+      });
+
+    test
+      .stdout()
+      .command(['lint', invalidSpecPath, '-c', outputConfigPath])
+      .it('saves results to a file', () => {
+        expect(fs.writeFile).toHaveBeenCalledWith(
+          'results.json',
+          // there are more errors listed
+          expect.stringContaining('Info object should contain `contact` object'),
+          expect.any(Function) // callback, util.promisify handles it for us
+        );
+      });
+
+    test
+      .stdout()
+      .command(['lint', validSpecPath, '-c', validRulesetConfigPath])
+      .it('outputs invalid ruleset error when invalid ruleset provided', ctx => {
+        expect(ctx.stdout).toContain(`5:10  warning  info-matches-stoplight  Info must contain Stoplight`);
+      });
+
+    test
+      .stdout()
+      .command(['lint', validSpecPath, '-c', invalidRulesetConfigPath])
+      .exit(2)
+      .it('outputs invalid ruleset error when invalid ruleset provided', ctx => {
+        expect(ctx.stdout).toContain(`/rules/rule-without-given-nor-them 	 should have required property 'given'`);
+      });
+  });
+
+  describe('when using config file and command args', () => {
+    test
+      .stdout()
+      .command(['lint', invalidSpecPath, '-c', validConfigPath, '-m', '1'])
+      .it('given maxResults set to 1 outputs warnings in json format', ctx => {
+        expect(ctx.stdout).toContain('"info.contact is not truthy"');
+        expect(ctx.stdout).not.toContain('"info.description is not truthy"');
+        expect(ctx.stdout).not.toContain('"servers does not exist"');
+      });
+  });
+
+  describe('when not using config nor default config file', () => {
+    test
+      .stdout()
+      .command(['lint', invalidSpecPath])
+      .it('outputs warnings in default format', ctx => {
+        expect(ctx.stdout).toContain('3:6  warning  info-contact      Info object should contain `contact` object');
+      });
+  });
+
+  describe('when using default config file', () => {
+    let spy: SpyInstance;
+    beforeAll(() => {
+      spy = jest.spyOn(process, 'cwd').mockReturnValue(resolve(__dirname, '__fixtures__'));
+    });
+    afterAll(() => {
+      spy.mockClear();
+    });
+    test
+      .stdout()
+      .command(['lint', invalidSpecPath])
+      .it('outputs data in format from default config file', ctx => {
+        expect(ctx.stdout).toContain('"info.contact is not truthy"');
+      });
+  });
 });
diff --git a/src/config/__tests__/__fixtures__/config.invalid.empty.yaml b/src/config/__tests__/__fixtures__/config.invalid.empty.yaml
new file mode 100644
index 00000000..ed97d539
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/config.invalid.empty.yaml
@@ -0,0 +1,1 @@
+---
diff --git a/src/config/__tests__/__fixtures__/config.invalid.json b/src/config/__tests__/__fixtures__/config.invalid.json
new file mode 100644
index 00000000..dcb513df
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/config.invalid.json
@@ -0,0 +1,7 @@
+  "lint": {
+    "encoding": "utf8",
+    "format": "json",
+    "maxResults": 5,
+    "verbose": true
+  }
+}
diff --git a/src/config/__tests__/__fixtures__/config.invalid.nolint.yaml b/src/config/__tests__/__fixtures__/config.invalid.nolint.yaml
new file mode 100644
index 00000000..c8b76882
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/config.invalid.nolint.yaml
@@ -0,0 +1,5 @@
+---
+encoding: utf8
+format: json
+maxResults: 5
+verbose: true
diff --git a/src/config/__tests__/__fixtures__/config.invalid.yaml b/src/config/__tests__/__fixtures__/config.invalid.yaml
new file mode 100644
index 00000000..9fd62835
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/config.invalid.yaml
@@ -0,0 +1,6 @@
+---
+lint:
+  encoding: utf8
+  format: json
+  maxResults: 5
+  verbose true
diff --git a/src/config/__tests__/__fixtures__/config.json b/src/config/__tests__/__fixtures__/config.json
new file mode 100644
index 00000000..70cabfc2
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/config.json
@@ -0,0 +1,8 @@
+{
+  "lint": {
+    "encoding": "utf8",
+    "format": "json",
+    "maxResults": 5,
+    "verbose": true
+  }
+}
diff --git a/src/config/__tests__/__fixtures__/config.yaml b/src/config/__tests__/__fixtures__/config.yaml
new file mode 100644
index 00000000..c408dcec
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/config.yaml
@@ -0,0 +1,6 @@
+---
+lint:
+  encoding: utf8
+  format: json
+  maxResults: 5
+  verbose: true
diff --git a/src/config/__tests__/__fixtures__/config.yml b/src/config/__tests__/__fixtures__/config.yml
new file mode 100644
index 00000000..c408dcec
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/config.yml
@@ -0,0 +1,6 @@
+---
+lint:
+  encoding: utf8
+  format: json
+  maxResults: 5
+  verbose: true
diff --git a/src/config/__tests__/__fixtures__/ruleset/config.ruleset.string.yml b/src/config/__tests__/__fixtures__/ruleset/config.ruleset.string.yml
new file mode 100644
index 00000000..8c6ca297
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/ruleset/config.ruleset.string.yml
@@ -0,0 +1,3 @@
+---
+lint:
+  ruleset: src/cli/commands/__tests__/__fixtures__/ruleset-invalid.yaml
diff --git a/src/config/__tests__/__fixtures__/ruleset/config.ruleset.yml b/src/config/__tests__/__fixtures__/ruleset/config.ruleset.yml
new file mode 100644
index 00000000..8b1a75f9
--- /dev/null
+++ b/src/config/__tests__/__fixtures__/ruleset/config.ruleset.yml
@@ -0,0 +1,4 @@
+---
+lint:
+  ruleset:
+    - src/cli/commands/__tests__/__fixtures__/ruleset-invalid.yaml
diff --git a/src/config/__tests__/config.test.ts b/src/config/__tests__/config.test.ts
new file mode 100644
index 00000000..9c1588fb
--- /dev/null
+++ b/src/config/__tests__/config.test.ts
@@ -0,0 +1,73 @@
+import * as path from 'path';
+import { ConfigCommand } from '../../types/config';
+import { load } from '../configLoader';
+
+const getFixture = (filePath: string) => path.resolve(__dirname, './__fixtures__', filePath);
+
+describe('config loading', () => {
+  let exampleConfig: any;
+  beforeEach(() => {
+    exampleConfig = {
+      lint: {
+        encoding: 'utf8',
+        format: 'json',
+        verbose: true,
+        maxResults: 5,
+      },
+    };
+  });
+  test('should load JSON config', async () => {
+    const configPath = getFixture('config.json');
+    const config = await load(configPath, ConfigCommand.LINT);
+    expect(config).toEqual(exampleConfig);
+  });
+
+  test('should load YAML config', async () => {
+    const configPath = getFixture('config.yaml');
+    const config = await load(configPath, ConfigCommand.LINT);
+    expect(config).toEqual(exampleConfig);
+  });
+
+  test('should load YML config', async () => {
+    const configPath = getFixture('config.yml');
+    const config = await load(configPath, ConfigCommand.LINT);
+    expect(config).toEqual(exampleConfig);
+  });
+
+  test('should throw error if file does not exist', async () => {
+    const configPath = getFixture('config.ghost.json');
+    await expect(load(configPath, ConfigCommand.LINT)).rejects.toThrowError(/does not exist/);
+  });
+
+  test('should throw error if JSON file is invalid', async () => {
+    const configPath = getFixture('config.invalid.json');
+    await expect(load(configPath, ConfigCommand.LINT)).rejects.toThrowError();
+  });
+
+  test('should throw error if YAML file is invalid', async () => {
+    const configPath = getFixture('config.invalid.yaml');
+    await expect(load(configPath, ConfigCommand.LINT)).rejects.toThrowError();
+  });
+
+  test('should load config with ruleset', async () => {
+    const configPath = getFixture('./ruleset/config.ruleset.yml');
+    const config = await load(configPath, ConfigCommand.LINT);
+    expect(config.lint.ruleset).toEqual(['src/cli/commands/__tests__/__fixtures__/ruleset-invalid.yaml']);
+  });
+
+  test('should load config with string ruleset', async () => {
+    const configPath = getFixture('./ruleset/config.ruleset.string.yml');
+    const config = await load(configPath, ConfigCommand.LINT);
+    expect(config.lint.ruleset).toEqual(['src/cli/commands/__tests__/__fixtures__/ruleset-invalid.yaml']);
+  });
+
+  test('should throw error if there is no lint specified', async () => {
+    const configPath = getFixture('config.invalid.nolint.yaml');
+    await expect(load(configPath, ConfigCommand.LINT)).rejects.toThrowError('Missing lint command in config file');
+  });
+
+  test('should throw error if config is empty', async () => {
+    const configPath = getFixture('config.invalid.empty.yaml');
+    await expect(load(configPath, ConfigCommand.LINT)).rejects.toThrowError('Missing config');
+  });
+});