Merge pull request #1149 from xodio/refactor-retain-more-info-about-conflicting-types

Retain information about conflicting data types in type resolution results
2026-03-15 05:06:59 +01:00 · 2018-04-11 18:02:03 +03:00
parent 8a36d7fd44 c13cabcac4
commit 8038ae168d
7 changed files with 95 additions and 56 deletions
--- a/packages/xod-client/src/project/components/Pin.jsx
+++ b/packages/xod-client/src/project/components/Pin.jsx
@@ -1,6 +1,8 @@
+import R from 'ramda';
 import React from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
+import { foldEither } from 'xod-func-tools';
 import { PIN_DIRECTION } from 'xod-project';

 import {
@@ -62,7 +64,7 @@ const Pin = props => {
          className={classNames(
            'symbol',
            'is-connected',
-            props.deducedType.getOrElse('conflicting')
+            foldEither(R.always('conflicting'), R.identity, props.deducedType)
          )}
          {...pinCircleCenter}
          r={PIN_INNER_RADIUS}
--- a/packages/xod-client/src/project/utils.js
+++ b/packages/xod-client/src/project/utils.js
@@ -1,7 +1,7 @@
 import * as R from 'ramda';
 import { Maybe } from 'ramda-fantasy';
 import * as XP from 'xod-project';
-import { foldMaybe } from 'xod-func-tools';
+import { foldMaybe, foldEither } from 'xod-func-tools';

 import {
  getOptimalPanningOffset,
@@ -145,6 +145,6 @@ export const getRenderablePinType = pin => {
  const { deducedType } = pin;

  return deducedType
-    ? deducedType.getOrElse('conflicting')
+    ? foldEither(R.always('conflicting'), R.identity, deducedType)
    : XP.getPinType(pin);
 };
--- a/packages/xod-project/src/constants.js
+++ b/packages/xod-project/src/constants.js
@@ -37,7 +37,8 @@ export const ERROR = {
    'Input node of the link does not exist in this patch',
  LINK_OUTPUT_NODE_NOT_FOUND:
    'Output node of the link does not exist in this patch',
-  LINK_CAUSES_TYPE_CONFLICT: 'Link causes type conflict',
+  LINK_CAUSES_TYPE_CONFLICT:
+    'Link causes type conflict between {conflictingTypes}',
  // comments
  COMMENT_NOT_FOUND:
    'Can\'t find the Comment "{commentId}" in the patch with path "{patchPath}"',
--- a/packages/xod-project/src/project.js
+++ b/packages/xod-project/src/project.js
@@ -3,9 +3,11 @@ import { Either, Maybe } from 'ramda-fantasy';
 import {
  foldMaybe,
  explodeMaybe,
+  explodeEither,
  notEmpty,
  isAmong,
  notNil,
+  noop,
 } from 'xod-func-tools';
 import { BUILT_IN_PATCH_PATHS } from './builtInPatches';

@@ -19,6 +21,7 @@ import * as PatchPathUtils from './patchPathUtils';
 import { def } from './types';
 import * as Utils from './utils';
 import { deducePinTypes } from './typeDeduction';
+import { foldEither } from '../../xod-func-tools/dist/monads';

 /**
 * Root of a project’s state tree
@@ -421,18 +424,28 @@ const checkPinsCompatibility = def(
            )(outputPinType, inputPinType),
          R.map(
            ({ original, deduced }) =>
-              deduced
-                ? explodeMaybe(
-                    'Impossible error: We already checked for unresolvable types earlier',
-                    deduced
-                  )
-                : original
+              deduced ? explodeEither(deduced) : original
          )
        )
      ),
-      Tools.errOnFalse(
-        CONST.ERROR.LINK_CAUSES_TYPE_CONFLICT,
-        R.none(R.propSatisfies(R.both(notNil, Maybe.isNothing), 'deduced'))
+      R.ifElse(
+        R.none(R.propSatisfies(R.both(notNil, Either.isLeft), 'deduced')),
+        Either.of,
+        R.compose(
+          foldEither(
+            conflictingTypes =>
+              Either.Left(
+                new Error(
+                  Utils.formatString(CONST.ERROR.LINK_CAUSES_TYPE_CONFLICT, {
+                    conflictingTypes: conflictingTypes.join(', '),
+                  })
+                )
+              ),
+            noop
+          ),
+          R.find(R.both(notNil, Either.isLeft)),
+          R.map(R.prop('deduced'))
+        )
      )
    )(pinTypes);
  }
--- a/packages/xod-project/src/typeDeduction.js
+++ b/packages/xod-project/src/typeDeduction.js
@@ -1,5 +1,5 @@
 import * as R from 'ramda';
-import { Maybe } from 'ramda-fantasy';
+import { Maybe, Either } from 'ramda-fantasy';
 import { foldEither, catMaybies } from 'xod-func-tools';

 import * as Link from './link';
@@ -16,7 +16,10 @@ const maybeGetTypeFromPreviouslyDeduced = R.curry(
    R.ifElse(
      Utils.isGenericType,
      () =>
-        R.pathOr(Maybe.Nothing(), [nodeId, pinKey], previouslyDeducedPinTypes),
+        R.compose(
+          foldEither(Maybe.Nothing, Maybe.of),
+          R.pathOr(Either.Left([]), [nodeId, pinKey])
+        )(previouslyDeducedPinTypes),
      Maybe.of
    )
 );
@@ -61,7 +64,7 @@ const getPinKeysByGenericType = patch =>
    Patch.listPins
  )(patch);

-// returns Map NodeId (Map PinKey (Maybe DataType))
+// returns Map NodeId (Map PinKey (Either [DataType] DataType))
 const deducePinTypesForNode = (
  abstractNodeId,
  getPatchByNodeId,
@@ -72,7 +75,7 @@ const deducePinTypesForNode = (
  // :: Map DataType [PinKey]
  const pinKeysByGenericType = getPinKeysByGenericType(abstractPatch);

-  // :: Map PinKey (Maybe DataType)
+  // :: Map PinKey (Either [DataType] DataType)
  const previouslyDeducedPinTypesForNode = R.propOr(
    {},
    abstractNodeId,
@@ -81,14 +84,34 @@ const deducePinTypesForNode = (

  return R.compose(
    R.reduce(
-      R.mergeWith(
-        // Nothing means there are are contradictions
-        (a, b) => (a.equals(b) ? a : Maybe.Nothing())
+      R.mergeWith((eitherA, eitherB) =>
+        foldEither(
+          contradictingAs =>
+            foldEither(
+              contradictingBs =>
+                R.compose(Either.Left, R.uniq, R.concat)(
+                  contradictingAs,
+                  contradictingBs
+                ),
+              okB =>
+                R.compose(Either.Left, R.uniq, R.append)(okB, contradictingAs),
+              eitherB
+            ),
+          okA =>
+            foldEither(
+              contradictingBs =>
+                R.compose(Either.Left, R.uniq, R.append)(okA, contradictingBs),
+              okB =>
+                okA === okB ? Either.Right(okA) : Either.Left([okA, okB]),
+              eitherB
+            ),
+          eitherA
+        )
      ),
      {}
    ),
    R.append(previouslyDeducedPinTypesForNode),
-    R.map(R.map(Maybe.of)),
+    R.map(R.map(Either.of)),
    // convert from [(PinKey, DataType)] to [Map PinKey DataType]
    // and propagate detected types to all pins with the same generic type
    catMaybies,
@@ -117,7 +140,7 @@ const deducePinTypesForNode = (

 // Filters out intermediate(outputs for strong resolution, inputs for weak) ambiguous pins,
 // which we needed only during resolving process.
-// :: Map NodeId (Map PinKey (Maybe DataType)) -> Map NodeId (Map PinKey (Maybe DataType))
+// :: Map NodeId (Map PinKey (Either [DataType] DataType)) -> Map NodeId (Map PinKey (Either [DataType] DataType))
 const omitIntermediateAmbiguousPins = (
  getPatchByNodeId,
  listIntermediatePins
@@ -126,7 +149,7 @@ const omitIntermediateAmbiguousPins = (
    R.compose(
      R.omit(R.__, pinTypes),
      R.filter(
-        R.pipe(R.propOr(Maybe.Nothing(), R.__, pinTypes), Maybe.isNothing)
+        R.pipe(R.propOr(Either.Left([]), R.__, pinTypes), Either.isLeft)
      ),
      R.map(Pin.getPinKey),
      listIntermediatePins,
@@ -163,8 +186,8 @@ export const deducePinTypes = def(
      Patch.listLinks
    )(patch);

-    // :: Map NodeId (Map PinKey (Maybe DataType))
-    // Maybe DataType is Just when type is resolved, and Nothing if it can not be decided.
+    // :: Map NodeId (Map PinKey (Either [DataType] DataType))
+    // Either [DataType] DataType is Right when type is resolved, and Left if there are conflicts.
    const stronglyResolvedTypes = R.compose(
      R.reject(R.isEmpty),
      R.reduce((previouslyDeducedPinTypes, abstractNodeId) => {
@@ -180,7 +203,7 @@ export const deducePinTypes = def(
          linksToNode
        );

-        // :: Map PinKey (Maybe DataType)
+        // :: Map PinKey (Either [DataType] DataType)
        const deducedPinTypesForNode = deducePinTypesForNode(
          abstractNodeId,
          getPatchByNodeId,
@@ -226,7 +249,7 @@ export const deducePinTypes = def(
          linksFromNode
        );

-        // :: Map PinKey (Maybe DataType)
+        // :: Map PinKey (Either [DataType] DataType)
        const deducedPinTypesForNode = deducePinTypesForNode(
          abstractNodeId,
          getPatchByNodeId,
--- a/packages/xod-project/src/types.js
+++ b/packages/xod-project/src/types.js
@@ -159,7 +159,7 @@ export const PinOrKey = OneOfType('PinOrKey', [PinKey, ObjectWithKey]);

 export const DeducedPinTypes = AliasType(
  'DeducedPinTypes',
-  $.StrMap($.StrMap(XF.$Maybe(DataType)))
+  $.StrMap($.StrMap(XF.$Either($.Array(DataType), DataType)))
 );

 //-----------------------------------------------------------------------------
--- a/packages/xod-project/test/typeDeduction.spec.js
+++ b/packages/xod-project/test/typeDeduction.spec.js
@@ -1,5 +1,5 @@
 import { assert } from 'chai';
-import { Maybe } from 'ramda-fantasy';
+import { Either } from 'ramda-fantasy';

 import * as Helper from './helpers';

@@ -18,20 +18,20 @@ describe('deducePinTypes', () => {

    const expected = {
      gen1_1to1: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
      gen2_ptp: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
      gen3_1to1: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
    };

-    assert.deepEqual(expected, deduced);
+    assert.deepEqual(deduced, expected);
  });

  it('deduces pin types from concrete nodes linked to generic inputs', () => {
@@ -42,20 +42,20 @@ describe('deducePinTypes', () => {

    const expected = {
      gen1_1to1: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
      gen2_ptp: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
      gen3_1to1: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
    };

-    assert.deepEqual(expected, deduced);
+    assert.deepEqual(deduced, expected);
  });

  it('detects conflicts when several outputs with different types are linked to inputs of the same generic type', () => {
@@ -66,20 +66,20 @@ describe('deducePinTypes', () => {

    const expected = {
      gen1_healthy: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
      gen2_broken: {
-        inT1_1: Maybe.Nothing(),
-        inT1_2: Maybe.Nothing(),
+        inT1_1: Either.Left([PIN_TYPE.NUMBER, PIN_TYPE.STRING]),
+        inT1_2: Either.Left([PIN_TYPE.NUMBER, PIN_TYPE.STRING]),
      },
      gen4_unaffected_healthy: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
    };

-    assert.deepEqual(expected, deduced);
+    assert.deepEqual(deduced, expected);
  });

  it('detects conflicts when several inputs with different types are linked to outputs of the same generic type', () => {
@@ -90,18 +90,18 @@ describe('deducePinTypes', () => {

    const expected = {
      gen2_broken: {
-        outT1: Maybe.Nothing(),
+        outT1: Either.Left([PIN_TYPE.NUMBER, PIN_TYPE.STRING]),
      },
      gen4_broken: {
-        outT1_1: Maybe.Nothing(),
-        outT1_2: Maybe.Nothing(),
+        outT1_1: Either.Left([PIN_TYPE.STRING, PIN_TYPE.NUMBER]),
+        outT1_2: Either.Left([PIN_TYPE.STRING, PIN_TYPE.NUMBER]),
      },
      gen5_unaffected: {
-        inT1: Maybe.Just(PIN_TYPE.NUMBER),
-        outT1: Maybe.Just(PIN_TYPE.NUMBER),
+        inT1: Either.Right(PIN_TYPE.NUMBER),
+        outT1: Either.Right(PIN_TYPE.NUMBER),
      },
    };

-    assert.deepEqual(expected, deduced);
+    assert.deepEqual(deduced, expected);
  });
 });