Repository: ChristopheBougere/asl-validator (ts)

Issue:
detect duplicate field names in payload templates resulting from substitution
**Is your feature request related to a problem? Please describe.**
Need a new rule to detect duplicate field names in a payload template following the rename that occurs after evaluation.

**Describe the solution you'd like**
Add a new validator that detects names that will collide. 

invalid:
```json
"Parameters" : {
    "static": "foo",
   "static.$": "$.path.to.value"  
}
```

The above is valid JSON at the time of deploy but will be invalid after evaluation since `static.$` is renamed to `static` which collides with the existing field.



Pull request:
feat: detect duplicate fields in payload templates post eval
closes #92 

Patch:
diff --git a/src/checks/duplicate-payload-template-fields.ts b/src/checks/duplicate-payload-template-fields.ts
new file mode 100644
index 0000000..cf58d19
--- /dev/null
+++ b/src/checks/duplicate-payload-template-fields.ts
@@ -0,0 +1,41 @@
+import {JSONPath} from 'jsonpath-plus';
+import {StateMachine, StateMachineError, StateMachineErrorCode} from '../types';
+
+// From the ASL spec:
+// > A JSON object MUST NOT have duplicate field names after fields
+// > ending with the characters ".$" are renamed to strip the ".$" suffix.
+export const mustNotHaveDuplicateFieldNamesAfterEvaluation = (definition: StateMachine): StateMachineError[] => {
+    // find each `Parameters` or `ResultSelector` field
+    // for each one, examine each node's children for field names
+    // that will be in conflict after evaluation.
+    const errorMessages: StateMachineError[] = [];
+    JSONPath<Record<string, unknown>[]>({
+        json: definition,
+        // this uses some extensions to the original JSONPath spec supported by the lib
+        //
+        // extension 1: @property.match enables matching on the matched property name.
+        //              This finds and selects all nodes with a name ending in .$
+        // extension 2: ^ operator twice at the end selects the parent node instead of the value for the node
+        //
+        // also, don't miss the array offset [0] before selecting the grandparent.
+        // There may be multiple fields within a node that could be in conflict.
+        // We're checking all the keys in the parent node, so we only need a single
+        // result for that parent.
+        path: `$..[Parameters,ResultSelector]..*[?(@property.match(/^.+\\.\\$$/))][0]^^`
+    })
+        .forEach((parent) => {
+            const keys: Record<string, number> = {};
+            Object.keys(parent).forEach((key) => {
+                const keyPostEval = key.endsWith(".$") ? key.substring(0, key.length - 2) : key;
+                const current = keys[keyPostEval];
+                if (current) {
+                    errorMessages.push({
+                        'Error code': StateMachineErrorCode.DuplicateFieldName,
+                        Message: `A duplicate field will exist after renaming to strip the ".$" suffix from: ${key}.$`,
+                    });
+                }
+                keys[keyPostEval] = current ? current + 1 : 1;
+            });
+        });
+    return errorMessages;
+}
diff --git a/src/types.ts b/src/types.ts
index 5e9de2c..3a3fccd 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -6,6 +6,7 @@ export type StateMachine = {
 export enum StateMachineErrorCode {
   BranchOutboundTransitionTarget = 'BRANCH_OUTBOUND_TRANSITION_TARGET',
   DuplicateStateNames = 'DUPLICATE_STATE_NAMES',
+  DuplicateFieldName = 'DUPLICATE_FIELD_NAME',
   InvalidJsonPath = 'INVALID_JSON_PATH',
   MapOutboundTransitionTarget = 'MAP_OUTBOUND_TRANSITION_TARGET',
   MissingTerminalState = 'MISSING_TERMINAL_STATE',
diff --git a/src/validator.ts b/src/validator.ts
index a5bbd3b..267393e 100644
--- a/src/validator.ts
+++ b/src/validator.ts
@@ -6,6 +6,7 @@ import stateTransitionsErrors from  './checks/state-transitions-errors';
 import duplicateStateNamesErrors from './checks/duplicate-state-names-errors';
 import missingTerminalStateErrors from './checks/missing-terminal-state-errors';
 import { StateMachine, StateMachineError } from './types';
+import {mustNotHaveDuplicateFieldNamesAfterEvaluation} from "./checks/duplicate-payload-template-fields";
 
 export default function validator(definition: StateMachine): {
   isValid: boolean;
@@ -19,6 +20,7 @@ export default function validator(definition: StateMachine): {
     errors.push(...stateTransitionsErrors(definition));
     errors.push(...duplicateStateNamesErrors(definition));
     errors.push(...missingTerminalStateErrors(definition));
+    errors.push(...mustNotHaveDuplicateFieldNamesAfterEvaluation(definition))
   }
 
   return {


Test patch:
diff --git a/src/__tests__/definitions/invalid-dupe-fields.asl.json b/src/__tests__/definitions/invalid-dupe-fields.asl.json
new file mode 100644
index 0000000..0608222
--- /dev/null
+++ b/src/__tests__/definitions/invalid-dupe-fields.asl.json
@@ -0,0 +1,16 @@
+{
+  "Comment": "Duplicate field in Parameters after the .$ suffix is removed during evaluation",
+  "StartAt": "PassState",
+  "States": {
+    "PassState": {
+      "Type": "Pass",
+      "Parameters": {
+        "dynamic": {
+          "conflict": "value",
+          "conflict.$": "$.value"
+        }
+      },
+      "End": true
+    }
+  }
+}