fix: spaces in FnSub literals are causing value to not be a literal

src/parser/private/sub.ts

@@ -4,31 +4,33 @@ export function analyzeSubPattern(pattern: string): SubFragment[] {
 
   let ph0 = pattern.indexOf('${', start);
   while (ph0 > -1) {
-    if (pattern[ph0 + 2] === '!') {
-      // "${!" means "don't actually substitute"
-      ret.push({ type: 'literal', content: pattern.substring(start, ph0 + 3) });
-      start = ph0 + 3;
-      ph0 = pattern.indexOf('${', start);
-      continue;
-    }
-
     const ph1 = pattern.indexOf('}', ph0 + 2);
+    const placeholder = pattern.substring(ph0 + 2, ph1);
+
     if (ph1 === -1) {
       break;
     }
-    const placeholder = pattern.substring(ph0 + 2, ph1);
 
     if (ph0 > start) {
       ret.push({ type: 'literal', content: pattern.substring(start, ph0) });
     }
+
+    if (placeholder.trim()[0] === '!') {
+      // "${!" means "don't actually substitute"
+      ret.push({ type: 'literal', content: pattern.substring(ph0, ph1 + 1) });
+      start = ph1 + 1;
+      ph0 = pattern.indexOf('${', start);
+      continue;
+    }
+
     if (placeholder.includes('.')) {
       const logicalId = placeholder.split('.')[0];
       // Because split('.', 2) doesn't do what you think it does
       const attr = placeholder.substring(logicalId.length + 1);
 
       ret.push({ type: 'getatt', logicalId: logicalId!, attr: attr! });
     } else {
-      ret.push({ type: 'ref', logicalId: placeholder });
+      ret.push({ type: 'ref', logicalId: placeholder.trim() });
     }
 
     start = ph1 + 1;

test/evaluate/fixtures.test.ts

@@ -19,8 +19,6 @@ describe('Cloudformation templates', () => {
     'cloudformation/condition-same-name-as-resource.json', // There is already a Construct with name 'AlwaysTrue' in DeclarativeStack [Test]
     'cloudformation/find-in-map-for-boolean-property.json', // Expected string or list of strings, got: true
     'cloudformation/find-in-map-with-dynamic-mapping.json', // Expected string, got: {"Ref":"Stage"}
-    'cloudformation/fn-sub-escaping.json', //  No resource or parameter with name:  ! DoesNotExist
-    'cloudformation/fn-sub-parsing-edges.json', // TypeError: Cannot read properties of null (reading 'Resources')
     'cloudformation/fn-sub-shadow-attribute.json', //  No resource or parameter with name: AnotherBucket
     'cloudformation/functions-and-conditions.json', // Expected list of length 3, got 2
     'cloudformation/hook-code-deploy-blue-green-ecs.json', // Expected valid template, got error(s): -  is not allowed to have the additional property "Hooks"
@@ -29,7 +27,6 @@ describe('Cloudformation templates', () => {
     'cloudformation/parameter-references.json', // Expected string or list of strings, got: {"Name":"AWS::Include","Parameters":{"Location":{"Ref":"MyParam"}}}
     'cloudformation/resource-attribute-creation-policy.json', // Expected number, got: {"Ref":"CountParameter"}
     'cloudformation/resource-attribute-update-policy.json', // Expected boolean, got: {"Fn::Equals":["true",{"Ref":"WaitOnResourceSignals"}]}
-    'cloudformation/short-form-fnsub-string.yaml', // No resource or parameter with name:  ! AWS::Region
   ];
 
   testTemplateFixtures(

test/parser/sub.test.ts

@@ -9,14 +9,6 @@ const subFormatString = fc.stringOf(
   { maxLength: 10 }
 );
 
-test('test analyzesubpattern', () => {
-  const parts = analyzeSubPattern('${AWS::StackName}-LogBucket');
-  expect(parts).toEqual([
-    { type: 'ref', logicalId: 'AWS::StackName' },
-    { type: 'literal', content: '-LogBucket' },
-  ] as SubFragment[]);
-});
-
 test('parsing and reconstituting are each others inverse', () => {
   fc.assert(
     fc.property(subFormatString, fc.context(), (s, ctx) => {
@@ -27,6 +19,51 @@ test('parsing and reconstituting are each others inverse', () => {
   );
 });
 
+test('references are extracted', () => {
+  const parts = analyzeSubPattern('${AWS::StackName}-LogBucket');
+  expect(parts).toEqual([
+    { type: 'ref', logicalId: 'AWS::StackName' },
+    { type: 'literal', content: '-LogBucket' },
+  ] as SubFragment[]);
+});
+
+test('references can have spaces', () => {
+  const parts = analyzeSubPattern('${ AWS::StackName }-LogBucket');
+  expect(parts).toEqual([
+    { type: 'ref', logicalId: 'AWS::StackName' },
+    { type: 'literal', content: '-LogBucket' },
+  ] as SubFragment[]);
+});
+
+test('literals are preserved without !', () => {
+  const parts = analyzeSubPattern('before-${!AWS::Region}-LogBucket');
+  expect(parts).toEqual([
+    { type: 'literal', content: 'before-' },
+    { type: 'literal', content: '${!AWS::Region}' },
+    { type: 'literal', content: '-LogBucket' },
+  ] as SubFragment[]);
+});
+
+test('unclosed ${ is preserved', () => {
+  const parts = analyzeSubPattern('start-${ end');
+  expect(parts).toEqual([
+    { type: 'literal', content: 'start-${ end' },
+  ] as SubFragment[]);
+});
+
+test('literal pattern can contain spaces around !', () => {
+  const parts = analyzeSubPattern(
+    '${!NoSpace}${ ! BothSpace}${! AfterSpace}${ !BeforeSpace}${!EndSpace }'
+  );
+  expect(parts).toEqual([
+    { type: 'literal', content: '${!NoSpace}' },
+    { type: 'literal', content: '${ ! BothSpace}' },
+    { type: 'literal', content: '${! AfterSpace}' },
+    { type: 'literal', content: '${ !BeforeSpace}' },
+    { type: 'literal', content: '${!EndSpace }' },
+  ] as SubFragment[]);
+});
+
 /**
  * Reconstitute a sub-string from a list of patterns
  */