Detect pin name clashes

packages/xod-arduino/src/transpiler.js

@@ -121,7 +121,7 @@ const convertPatchToTPatch = def(
           ),
         })
       ),
-      XP.normalizePinLabels,
+      XP.normalizeEmptyPinLabels,
       XP.listOutputPins
     )(patch);
 
@@ -133,7 +133,7 @@ const convertPatchToTPatch = def(
           isDirtyable,
         })
       ),
-      XP.normalizePinLabels,
+      XP.normalizeEmptyPinLabels,
       XP.listInputPins
     )(patch);
 
@@ -266,7 +266,7 @@ const getNodePinsUnsafe = def(
 
 const getNodePinLabels = def(
   'getNodePinLabels :: Node -> Project -> Map PinKey PinLabel',
-  R.compose(getPinLabelsMap, XP.normalizePinLabels, getNodePinsUnsafe)
+  R.compose(getPinLabelsMap, XP.normalizeEmptyPinLabels, getNodePinsUnsafe)
 );
 
 // TODO: Remove it when `Project.getBoundValue` will return default values

packages/xod-client/src/editor/components/PatchDocs.jsx

@@ -124,7 +124,7 @@ const PatchDocs = ({ patch, minimal }) => {
 
   const [inputPins, outputPins] = R.compose(
     R.partition(XP.isInputPin),
-    XP.normalizePinLabels,
+    XP.normalizeEmptyPinLabels,
     R.map(pin =>
       R.assoc('isVariadic', R.contains(XP.getPinKey(pin), variadicPinKeys), pin)
     ),

packages/xod-client/src/project/selectors.js

@@ -153,12 +153,11 @@ const mergePinDataFromPatch = R.curry((project, curPatch, node) =>
   )(node, curPatch, project)
 );
 
+const errorsLens = R.lens(R.propOr([], 'errors'), R.assoc('errors'));
+
 // :: Error -> RenderableNode|RenderableLink -> RenderableNode|RenderableLink
 const addError = R.curry((error, renderableEntity) =>
-  R.compose(
-    R.over(R.lensProp('errors'), R.append(error)),
-    R.unless(R.has('errors'), R.assoc('errors', []))
-  )(renderableEntity)
+  R.over(errorsLens, R.append(error), renderableEntity)
 );
 
 // :: Project -> RenderableNode -> RenderableNode
@@ -182,34 +181,6 @@ const addDeadRefErrors = R.curry((project, renderableNode) =>
   )(renderableNode)
 );
 
-const addMarkerErrors = (predicate, validator) =>
-  R.curry((patch, renderableNode) =>
-    R.when(predicate, node =>
-      R.compose(
-        foldEither(err => addError(err, node), R.always(node)),
-        validator
-      )(patch)
-    )(renderableNode)
-  );
-
-// :: Patch -> RenderableNode -> RenderableNode
-const addVariadicErrors = addMarkerErrors(
-  R.pipe(XP.getNodeType, XP.isVariadicPath),
-  XP.validatePatchForVariadics
-);
-
-// :: Patch -> RenderableNode -> RenderableNode
-const addAbstractPatchErrors = addMarkerErrors(
-  R.pipe(XP.getNodeType, R.equals(XP.ABSTRACT_MARKER_PATH)),
-  XP.validateAbstractPatch
-);
-
-// :: Patch -> RenderableNode -> RenderableNode
-const addConstructorPatchErrors = addMarkerErrors(
-  R.pipe(XP.getNodeType, R.equals(XP.OUTPUT_SELF_PATH)),
-  XP.validateConstructorPatch
-);
-
 /**
  * Adds `isVariadic` flag and `arityStep` prop.
  */
@@ -319,9 +290,6 @@ export const getRenderableNode = R.curry(
       addSpecializationsList(project),
       markDeprecatedNodes(project),
       addInvalidLiteralErrors(project, currentPatch),
-      addConstructorPatchErrors(currentPatch),
-      addAbstractPatchErrors(currentPatch),
-      addVariadicErrors(currentPatch),
       addVariadicProps(project),
       addDeadRefErrors(project),
       addNodePositioning,
@@ -331,6 +299,71 @@ export const getRenderableNode = R.curry(
     )(node)
 );
 
+const getMarkerNodesErrorMap = (predicate, validator) => patch => {
+  const markerNodeIds = R.compose(
+    R.map(XP.getNodeId),
+    R.filter(predicate),
+    XP.listNodes
+  )(patch);
+
+  if (R.isEmpty(markerNodeIds)) return {};
+
+  return foldEither(
+    err => R.compose(R.fromPairs, R.map(R.pair(R.__, err)))(markerNodeIds),
+    R.always({}),
+    validator(patch)
+  );
+};
+
+// :: Patch -> Map NodeId Error
+const getVariadicMarkersErrorMap = getMarkerNodesErrorMap(
+  R.pipe(XP.getNodeType, XP.isVariadicPath),
+  XP.validatePatchForVariadics
+);
+
+// :: Patch -> Map NodeId Error
+const getAbstractMarkersErrorMap = getMarkerNodesErrorMap(
+  R.pipe(XP.getNodeType, R.equals(XP.ABSTRACT_MARKER_PATH)),
+  XP.validateAbstractPatch
+);
+
+// :: Patch -> Map NodeId Error
+const getConstructorMarkersErrorMap = getMarkerNodesErrorMap(
+  R.pipe(XP.getNodeType, R.equals(XP.OUTPUT_SELF_PATH)),
+  XP.validateConstructorPatch
+);
+
+// :: Patch -> Map NodeId Error
+const getTerminalsErrorMap = R.compose(
+  foldEither(
+    err =>
+      R.compose(
+        R.fromPairs,
+        R.map(R.pair(R.__, err)),
+        R.path(['payload', 'pinKeys']) // those are affected terminal node ids
+      )(err),
+    R.always({})
+  ),
+  XP.validatePinLabels
+);
+
+const markNodesCausingErrors = R.curry((currentPatch, nodes) => {
+  // :: Map NodeId Error
+  const errorsMap = R.mergeAll([
+    getTerminalsErrorMap(currentPatch),
+    getVariadicMarkersErrorMap(currentPatch),
+    getAbstractMarkersErrorMap(currentPatch),
+    getConstructorMarkersErrorMap(currentPatch),
+  ]);
+
+  if (R.isEmpty(errorsMap)) return nodes;
+
+  return R.map(node => {
+    const nodeId = XP.getNodeId(node);
+    return R.has(nodeId, errorsMap) ? addError(errorsMap[nodeId], node) : node;
+  }, nodes);
+});
+
 // :: State -> StrMap RenderableNode
 export const getRenderableNodes = createMemoizedSelector(
   [
@@ -351,16 +384,18 @@ export const getRenderableNodes = createMemoizedSelector(
     foldMaybe(
       {},
       currentPatch =>
-        R.map(
-          getRenderableNode(
-            R.__,
-            currentPatch,
-            connectedPins,
-            deducedPinTypes,
-            project
-          ),
-          currentPatchNodes
-        ),
+        R.compose(
+          markNodesCausingErrors(currentPatch),
+          R.map(
+            getRenderableNode(
+              R.__,
+              currentPatch,
+              connectedPins,
+              deducedPinTypes,
+              project
+            )
+          )
+        )(currentPatchNodes),
       maybeCurrentPatch
     )
 );

packages/xod-client/src/project/utils.js

@@ -82,49 +82,51 @@ export const getInitialPatchOffset = R.compose(
 );
 
 // extract information from Patch that is required to render it with Node component
-export const patchToNodeProps = R.curry((shouldNormalizePinLabels, patch) => {
-  const pins = XP.listPins(patch);
-  const size = calcutaleNodeSizeFromPins(pins);
-  const type = XP.getPatchPath(patch);
-  const isVariadic = XP.isVariadicPatch(patch);
-  const arityStep = foldMaybe(0, R.identity, XP.getArityStepFromPatch(patch));
-
-  return {
-    id: type,
-    type,
-    label: '',
-    position: { x: 0, y: 0 },
-    size,
-    isVariadic,
-    pins: R.compose(
-      R.when(
-        () => isVariadic,
-        renderablePins =>
-          R.compose(
-            R.merge(renderablePins),
-            R.indexBy(R.prop('keyName')),
-            R.map(R.assoc('isLastVariadicGroup', true)),
-            R.takeLast(arityStep),
-            R.sortBy(XP.getPinOrder),
-            R.filter(XP.isInputPin),
-            R.values
-          )(renderablePins)
-      ),
-      R.indexBy(R.prop('keyName')),
-      R.map(
-        R.applySpec({
-          key: XP.getPinKey,
-          keyName: XP.getPinKey,
-          type: XP.getPinType,
-          direction: XP.getPinDirection,
-          label: XP.getPinLabel,
-          position: calculatePinPosition(size),
-        })
-      ),
-      shouldNormalizePinLabels ? XP.normalizePinLabels : R.identity
-    )(pins),
-  };
-});
+export const patchToNodeProps = R.curry(
+  (shouldnormalizeEmptyPinLabels, patch) => {
+    const pins = XP.listPins(patch);
+    const size = calcutaleNodeSizeFromPins(pins);
+    const type = XP.getPatchPath(patch);
+    const isVariadic = XP.isVariadicPatch(patch);
+    const arityStep = foldMaybe(0, R.identity, XP.getArityStepFromPatch(patch));
+
+    return {
+      id: type,
+      type,
+      label: '',
+      position: { x: 0, y: 0 },
+      size,
+      isVariadic,
+      pins: R.compose(
+        R.when(
+          () => isVariadic,
+          renderablePins =>
+            R.compose(
+              R.merge(renderablePins),
+              R.indexBy(R.prop('keyName')),
+              R.map(R.assoc('isLastVariadicGroup', true)),
+              R.takeLast(arityStep),
+              R.sortBy(XP.getPinOrder),
+              R.filter(XP.isInputPin),
+              R.values
+            )(renderablePins)
+        ),
+        R.indexBy(R.prop('keyName')),
+        R.map(
+          R.applySpec({
+            key: XP.getPinKey,
+            keyName: XP.getPinKey,
+            type: XP.getPinType,
+            direction: XP.getPinDirection,
+            label: XP.getPinLabel,
+            position: calculatePinPosition(size),
+          })
+        ),
+        shouldnormalizeEmptyPinLabels ? XP.normalizeEmptyPinLabels : R.identity
+      )(pins),
+    };
+  }
+);
 
 export const isPatchDeadTerminal = R.compose(
   R.ifElse(

packages/xod-project/src/index.js

@@ -22,6 +22,7 @@ export {
   listInputPins,
   listOutputPins,
   isTerminalPatch,
+  validatePinLabels,
   listLinks,
   linkIdEquals,
   getLinkById,
@@ -92,7 +93,7 @@ export {
   isInputPin,
   isOutputPin,
   isTerminalPin,
-  normalizePinLabels,
+  normalizeEmptyPinLabels,
   isPinBindable,
   isPulsePin,
 } from './pin';

packages/xod-project/src/messages.js

@@ -235,6 +235,12 @@ export default {
     note: `Can't find the Pin "${pinKey}" in the patch with path "${patchPath}"`,
     trace,
   }),
+  CLASHING_PIN_LABELS: ({ label, pinKeys, trace }) => ({
+    title: 'Clashing pin names',
+    note: `${pinKeys.length} pins are named ${label}`,
+    solution: 'Give the pins unique names',
+    trace,
+  }),
   DEAD_REFERENCE__PATCH_FOR_NODE_NOT_FOUND: ({ nodeType, trace }) => ({
     title: 'Dead reference error',
     note: `Patch "${nodeType}" is not found in the project`,

packages/xod-project/src/patch.js

@@ -481,6 +481,32 @@ export const isTerminalPatch = def(
   R.compose(isTerminalPatchPath, getPatchPath)
 );
 
+// detects clashing pin labels
+export const validatePinLabels = def(
+  'validatePinLabels :: Patch -> Either Error Patch',
+  patch =>
+    R.compose(
+      R.ifElse(
+        R.isEmpty,
+        R.always(Either.of(patch)),
+        R.compose(
+          ([label, pins]) =>
+            fail('CLASHING_PIN_LABELS', {
+              label,
+              pinKeys: R.map(Pin.getPinKey, pins),
+              trace: [getPatchPath(patch)],
+            }),
+          R.head,
+          R.toPairs
+        )
+      ),
+      R.filter(groupedPins => groupedPins.length > 1),
+      R.groupBy(Pin.getPinLabel),
+      Pin.normalizeEmptyPinLabels,
+      listPins
+    )(patch)
+);
+
 // =============================================================================
 //
 // Links
@@ -948,7 +974,7 @@ export const getNondeadNodePins = def(
           R.mergeWith(R.merge, R.__, patchPins),
           R.map(R.compose(R.objOf('normalizedLabel'), Pin.getPinLabel)),
           R.indexBy(Pin.getPinKey),
-          Pin.normalizePinLabels,
+          Pin.normalizeEmptyPinLabels,
           R.values
         )(patchPins),
       R.indexBy(Pin.getPinKey),