fix(inspector): correct filepath cross-references in aggregate messag…

…es; closes #18

- do not hardcode `spec` in `.mocharc.js`; it's impossible to execute a single test file if it's there
- add sourcemaps to Rollup config for debugging
- fix "way too many subscriptions" to `Observable<Report>` instances
- use `_.coerceToArray()` in a couple places
- put `NO_FILEPATH` and `MULTIPLE_FILEPATHS` constants where they should be
- import `gte` and `__` (placeholder) from `lodash/fp`
- sort _before_ instantiating the `Report` object, not after (it will be moved anyway; see #19)
- don't use hardcoded JSON in JSON formatter test
- add some debugs
- fix bad debug namespace in `rule-config.js`
- refactor some logic into a new `Message` class in the inspector
- rename `context` to `report` anywhere NOT in a Rule impl
- remove unused `createReportWithFilepath()`
- don't refuse to instantiate a `Report` from a `Report`
- actually create a `Report` when inspecting (test harness)
- remove dupe of `library-mismatch.spec.js`

.mocharc.js

@@ -5,6 +5,5 @@ module.exports = {
   'experimental-report': true,
   'forbid-only': Boolean(process.env.CI),
   'no-warnings': true,
-  require: ['esm', require.resolve('./packages/common/test/setup.js')],
-  spec: './packages/*/test/**/*.spec.js'
+  require: ['esm', require.resolve('./packages/common/test/setup.js')]
 };

package.json

@@ -22,7 +22,7 @@
     "lint": "eslint .",
     "prerelease": "cross-env NPM_CONFIG_LOGLEVEL=silent npm run clean && npm run build",
     "pretest": "cross-env NPM_CONFIG_LOGLEVEL=silent npm run build",
-    "test": "mocha",
+    "test": "mocha \"./packages/*/test/**/*.spec.js\"",
     "posttest": "cross-env NPM_CONFIG_LOGLEVEL=silent npm run lint"
   },
   "husky": {

packages/cli/src/commands/inspect.js

@@ -11,7 +11,7 @@ import {colors, fail, toFormattedString} from '../console-utils.js';
 import {GROUPS, OPTIONS} from './common.js';
 
 const {ERROR, INFO, WARNING} = constants;
-const {fromAny} = observable;
+const {fromAny, share} = observable;
 const {toInspection, toReportFromObject, toRuleConfig} = stream;
 
 const BUILTIN_RULES_DIR = join(
@@ -26,7 +26,7 @@ export const desc = 'Inspect diagnostic report JSON against rules';
 export const builder = yargs =>
   yargs
     .positional('file', {
-      coerce: v => (_.isArray(v) ? v : [v]),
+      coerce: _.coerceToArray,
       type: 'array'
     })
     .options({
@@ -53,7 +53,8 @@ export const handler = argv => {
   } = argv;
   const reports = fromAny(filepaths).pipe(
     toObjectFromFilepath({...config.inspect}),
-    toReportFromObject({...config.inspect, showSecretsUnsafe})
+    toReportFromObject({...config.inspect, showSecretsUnsafe}),
+    share()
   );
   fromSearchpathToRuleDefinition(BUILTIN_RULES_DIR)
     .pipe(

packages/common/src/constants.js

@@ -2,6 +2,8 @@ export const ERROR = 'error';
 export const WARNING = 'warning';
 export const INFO = 'info';
 export const NAMESPACE = 'gnostic';
+export const NO_FILEPATH = '(no filepath)';
+export const MULTIPLE_FILEPATHS = '(multiple files)';
 
 export const DEFAULT_DIFF_OPTIONS = Object.freeze({
   properties: Object.freeze([

packages/common/src/util.js

@@ -13,6 +13,7 @@ import flip from 'lodash/fp/flip.js';
 import forEach from 'lodash/fp/forEach.js';
 import get from 'lodash/fp/get.js';
 import getOr from 'lodash/fp/getOr.js';
+import gte from 'lodash/fp/gte.js';
 import has from 'lodash/fp/has.js';
 import identity from 'lodash/fp/identity.js';
 import includes from 'lodash/fp/includes.js';
@@ -40,6 +41,7 @@ import overEvery from 'lodash/fp/overEvery.js';
 import overSome from 'lodash/fp/overSome.js';
 import pick from 'lodash/fp/pick.js';
 import pipe from 'lodash/fp/pipe.js';
+import __ from 'lodash/fp/placeholder.js';
 import reduce from 'lodash/fp/reduce.js';
 import reverse from 'lodash/fp/reverse.js';
 import size from 'lodash/fp/size.js';
@@ -57,6 +59,7 @@ import traverse from 'traverse';
 export const coerceToArray = value => (isArray(value) ? value : [value]);
 
 export const _ = {
+  __,
   clamp,
   concat,
   constant,
@@ -70,6 +73,7 @@ export const _ = {
   forEach,
   get,
   getOr,
+  gte,
   has,
   identity,
   includes,

packages/core/src/stream.js

@@ -22,8 +22,8 @@ const {
   mergeMap,
   of,
   pipeIf,
-  share,
   sort,
+  switchMapTo,
   take,
   throwGnosticError,
   toArray,
@@ -62,34 +62,20 @@ export const diff = (reports, opts = {}) => {
   );
 };
 
-export const toInspection = (reports, opts = {}) => {
-  const {
-    disableSort,
-    severity,
-    showSecretsUnsafe,
-    sortDirection,
-    sortField
-  } = _.defaults(DEFAULT_LOAD_REPORT_OPTIONS, opts);
-  const multicastReports = defer(() =>
-    reports.pipe(
-      toReport({
-        disableSort,
-        showSecretsUnsafe,
-        sortDirection,
-        sortField
-      }),
-      share()
-    )
-  );
-  return ruleConfigs =>
-    defer(() =>
-      isObservable(reports)
-        ? ruleConfigs.pipe(inspectReports(multicastReports, {severity}))
-        : throwGnosticError(
-            GNOSTIC_ERR_INVALID_PARAMETER,
-            'inspect() requires an Observable of reports'
+export const toInspection = (reports, opts = DEFAULT_LOAD_REPORT_OPTIONS) => {
+  const {severity} = opts;
+
+  return isObservable(reports)
+    ? ruleConfigs => ruleConfigs.pipe(inspectReports(reports, {severity}))
+    : ruleConfigs =>
+        ruleConfigs.pipe(
+          switchMapTo(
+            throwGnosticError(
+              GNOSTIC_ERR_INVALID_PARAMETER,
+              'Parameter to toInspection() must be of type Observable<Report>'
+            )
           )
-    );
+        );
 };
 
 export const inspect = (reports, rules, config, {severity} = {}) =>
@@ -113,8 +99,8 @@ export const toReportFromObject = (opts = {}) => {
             : {rawReport: redact(obj)}
         )
       ),
-      map(({filepath, rawReport}) => createReport(rawReport, filepath)),
-      pipeIf(!disableSort, sort(`rawReport.${sortField}`, sortDirection))
+      pipeIf(!disableSort, sort(`rawReport.${sortField}`, sortDirection)),
+      map(({filepath, rawReport}) => createReport(rawReport, filepath))
     );
 };
 

packages/formatters/test/json.spec.js

@@ -7,31 +7,66 @@ describe('@gnostic/formatters:json', function() {
     describe('toJson()', function() {
       it('should parse a JS object and return JSON', function() {
         return expect(
-          of({bar: 2, baz: 3, foo: 1}, {bar: 5, baz: 6, foo: 4}).pipe(toJson()),
+          of(
+            {
+              bar: 2,
+              baz: 3,
+              foo: 1
+            },
+            {
+              bar: 5,
+              baz: 6,
+              foo: 4
+            }
+          ).pipe(toJson()),
           'to complete with value',
-          '[{"foo":1,"bar":2,"baz":3},{"foo":4,"bar":5,"baz":6}]'
+          JSON.stringify([
+            {
+              bar: 2,
+              baz: 3,
+              foo: 1
+            },
+            {
+              bar: 5,
+              baz: 6,
+              foo: 4
+            }
+          ])
         );
       });
 
       describe('when "pretty" option enabled', function() {
         it('should parse a JS object and return pretty JSON', function() {
           return expect(
-            of({bar: 2, baz: 3, foo: 1}, {bar: 5, baz: 6, foo: 4}).pipe(
-              toJson({pretty: true})
-            ),
+            of(
+              {
+                bar: 2,
+                baz: 3,
+                foo: 1
+              },
+              {
+                bar: 5,
+                baz: 6,
+                foo: 4
+              }
+            ).pipe(toJson({pretty: true})),
             'to complete with value',
-            `[
-  {
-    "foo": 1,
-    "bar": 2,
-    "baz": 3
-  },
-  {
-    "foo": 4,
-    "bar": 5,
-    "baz": 6
-  }
-]`
+            JSON.stringify(
+              [
+                {
+                  bar: 2,
+                  baz: 3,
+                  foo: 1
+                },
+                {
+                  bar: 5,
+                  baz: 6,
+                  foo: 4
+                }
+              ],
+              null,
+              2
+            )
           );
         });
       });

packages/fs/src/fs-report-loader.js

@@ -1,15 +1,18 @@
-import {observable} from '@gnostic/common';
+import {createDebugPipe, observable} from '@gnostic/common';
 import {readFile as readFileNodeback} from 'fs';
 const {bindNodeCallback, map, mergeMap, toObjectFromJSON} = observable;
 
 const readFile = bindNodeCallback(readFileNodeback);
 
+const debug = createDebugPipe('fs', 'report-loader');
+
 export const toObjectFromFilepath = () => observable =>
   observable.pipe(
     mergeMap(filepath =>
       readFile(filepath, 'utf8').pipe(
         toObjectFromJSON(),
-        map(rawReport => ({filepath, rawReport}))
+        map(rawReport => ({filepath, rawReport})),
+        debug(({filepath}) => `parsed raw report from ${filepath}`)
       )
     )
   );

packages/inspector/src/index.js

@@ -1,7 +1,8 @@
-import {_, constants, observable} from '@gnostic/common';
+import {_, constants, createDebugPipe, observable} from '@gnostic/common';
 
 const {ERROR, INFO, WARNING} = constants;
-const {filter, map, mergeMap} = observable;
+const {filter, mergeMap} = observable;
+const debug = createDebugPipe('inspector');
 
 const SEVERITIES = {
   [ERROR]: 30,
@@ -24,11 +25,13 @@ export const inspectReports = (
 ) => ruleConfigs =>
   ruleConfigs.pipe(
     mergeMap(ruleConfig => ruleConfig.inspect(reports)),
-    map(message =>
-      _.has('severity', message) ? message : {...message, severity: ERROR}
-    ),
+    debug(msg => `received message ${JSON.stringify(msg)}`),
     filter(
-      message => _.get(message.severity, SEVERITIES) >= SEVERITIES[severity]
+      _.pipe(
+        _.get('severity'),
+        _.get(_.__, SEVERITIES),
+        _.gte(_.__, SEVERITIES[severity])
+      )
     )
   );
 

packages/inspector/src/message.js

@@ -0,0 +1,64 @@
+import {_, constants} from '@gnostic/common';
+const {ERROR, MULTIPLE_FILEPATHS} = constants;
+
+export class Message {
+  constructor(opts = {}) {
+    const {
+      data,
+      filepath,
+      id,
+      isAggregate,
+      message,
+      originalError,
+      config,
+      severity = ERROR
+    } = _.omitBy(
+      _.overEvery([_.negate(_.isError), _.negate(_.isBoolean), _.isEmpty]),
+      opts
+    );
+
+    this.message = message.toString();
+    this.severity = severity;
+    this.id = id;
+
+    if (config) {
+      this.config = config;
+    }
+
+    if (isAggregate) {
+      this.filepath = MULTIPLE_FILEPATHS;
+    } else if (filepath) {
+      this.filepath = filepath;
+    }
+
+    if (originalError) {
+      this.originalError = originalError;
+    }
+
+    if (data) {
+      this.data = data;
+    }
+  }
+
+  isNonEmpty() {
+    return Boolean(this.message.trim());
+  }
+
+  toString() {
+    return this.message;
+  }
+}
+
+export const createMessage = (
+  message,
+  {filepath, id, isAggregate = false, config = {}}
+) => {
+  message = _.isString(message) ? {message} : message;
+  return new Message({
+    ...message,
+    config,
+    filepath,
+    id,
+    isAggregate
+  });
+};

packages/inspector/src/rule-config.js

@@ -1,6 +1,6 @@
 import {_, createDebugger} from '@gnostic/common';
 
-const debug = createDebugger(module);
+const debug = createDebugger('inspector', 'rule-config');
 const ruleMap = new WeakMap();
 
 export class RuleConfig {
@@ -45,8 +45,8 @@ export class RuleConfig {
     return config;
   }
 
-  inspect(contexts) {
-    return this.rule.inspect({config: this.config, contexts});
+  inspect(reports) {
+    return this.rule.inspect({config: this.config, reports});
   }
 
   static create(rule, rawConfig) {