Represent rest parameters as properties on Parameter (#2454)

This simplifies the code and will likely make it easier for users to
consume and manipulate parameters.

Author: nex3

pkg/sass-parser/lib/src/__snapshots__/parameter-list.test.ts.snap

@@ -11,9 +11,9 @@ exports[`a parameter list toJSON 1`] = `
   ],
   "nodes": [
     <$foo>,
+    <$bar...>,
   ],
   "raws": {},
-  "restParameter": "bar",
   "sassType": "parameter-list",
   "source": <1:12-1:27 in 0>,
 }

pkg/sass-parser/lib/src/__snapshots__/parameter.test.ts.snap

@@ -12,6 +12,7 @@ exports[`a parameter toJSON with a default 1`] = `
   ],
   "name": "baz",
   "raws": {},
+  "rest": false,
   "sassType": "parameter",
   "source": <1:13-1:24 in 0>,
 }
@@ -28,7 +29,18 @@ exports[`a parameter toJSON with no default 1`] = `
   ],
   "name": "baz",
   "raws": {},
+  "rest": false,
   "sassType": "parameter",
   "source": <1:13-1:17 in 0>,
 }
 `;
+
+exports[`a parameter toJSON with rest = true 1`] = `
+{
+  "inputs": [],
+  "name": "baz",
+  "raws": {},
+  "rest": true,
+  "sassType": "parameter",
+}
+`;

pkg/sass-parser/lib/src/parameter-list.test.ts

@@ -20,9 +20,6 @@ describe('a parameter list', () => {
           expect(node.sassType).toBe('parameter-list'));
 
         it('has no nodes', () => expect(node.nodes).toHaveLength(0));
-
-        it('has no rest parameter', () =>
-          expect(node.restParameter).toBeUndefined());
       });
     }
 
@@ -80,9 +77,6 @@ describe('a parameter list', () => {
           expect(node.nodes[0].defaultValue).toBeUndefined();
           expect(node.nodes[0].parent).toBe(node);
         });
-
-        it('has no rest parameter', () =>
-          expect(node.restParameter).toBeUndefined());
       });
     }
 
@@ -165,9 +159,6 @@ describe('a parameter list', () => {
           expect(node.nodes[0]).toHaveStringExpression('defaultValue', 'bar');
           expect(node.nodes[0]).toHaveProperty('parent', node);
         });
-
-        it('has no rest parameter', () =>
-          expect(node.restParameter).toBeUndefined());
       });
     }
 
@@ -253,58 +244,6 @@ describe('a parameter list', () => {
     });
   });
 
-  describe('with a rest parameter', () => {
-    function describeNode(
-      description: string,
-      create: () => ParameterList,
-    ): void {
-      describe(description, () => {
-        beforeEach(() => void (node = create()));
-
-        it('has a sassType', () =>
-          expect(node.sassType).toBe('parameter-list'));
-
-        it('has no nodes', () => expect(node.nodes).toHaveLength(0));
-
-        it('has a rest parameter', () =>
-          expect(node.restParameter).toBe('foo'));
-      });
-    }
-
-    describeNode(
-      'parsed as SCSS',
-      () =>
-        (scss.parse('@function x($foo...) {}').nodes[0] as FunctionRule)
-          .parameters,
-    );
-
-    describeNode(
-      'parsed as Sass',
-      () =>
-        (sass.parse('@function x($foo...)').nodes[0] as FunctionRule)
-          .parameters,
-    );
-
-    describeNode(
-      'constructed manually',
-      () => new ParameterList({restParameter: 'foo'}),
-    );
-
-    describeNode(
-      'constructed from properties',
-      () =>
-        new FunctionRule({
-          functionName: 'x',
-          parameters: {restParameter: 'foo'},
-        }).parameters,
-    );
-  });
-
-  it('assigned a new rest parameter', () => {
-    node.restParameter = 'qux';
-    expect(node.restParameter).toBe('qux');
-  });
-
   describe('can add', () => {
     beforeEach(() => void (node = new ParameterList()));
 
@@ -683,17 +622,10 @@ describe('a parameter list', () => {
   });
 
   describe('stringifies', () => {
-    describe('with no nodes or rest parameter', () => {
+    describe('with no nodes', () => {
       it('with default raws', () =>
         expect(new ParameterList().toString()).toBe('()'));
 
-      it('ignores restParameter', () =>
-        expect(
-          new ParameterList({
-            raws: {restParameter: {value: 'foo', raw: 'foo'}},
-          }).toString(),
-        ).toBe('()'));
-
       it('ignores comma', () =>
         expect(new ParameterList({raws: {comma: true}}).toString()).toBe('()'));
 
@@ -709,22 +641,6 @@ describe('a parameter list', () => {
           '($foo, $bar, $baz)',
         ));
 
-      it('ignores beforeRestParameter', () =>
-        expect(
-          new ParameterList({
-            nodes: ['foo', 'bar', 'baz'],
-            raws: {beforeRestParameter: '/**/'},
-          }).toString(),
-        ).toBe('($foo, $bar, $baz)'));
-
-      it('ignores restParameter', () =>
-        expect(
-          new ParameterList({
-            nodes: ['foo', 'bar', 'baz'],
-            raws: {restParameter: {value: 'foo', raw: 'foo'}},
-          }).toString(),
-        ).toBe('($foo, $bar, $baz)'));
-
       it('with comma: true', () =>
         expect(
           new ParameterList({
@@ -788,77 +704,6 @@ describe('a parameter list', () => {
           ).toBe('($foo, $bar, $baz  ,)'));
       });
     });
-
-    describe('with restParameter', () => {
-      it('with default raws', () =>
-        expect(new ParameterList({restParameter: 'foo'}).toString()).toBe(
-          '($foo...)',
-        ));
-
-      it("that's not an identifier", () =>
-        expect(new ParameterList({restParameter: 'f o'}).toString()).toBe(
-          '($f\\20o...)',
-        ));
-
-      it('with parameters', () =>
-        expect(
-          new ParameterList({
-            nodes: ['foo', 'bar'],
-            restParameter: 'baz',
-          }).toString(),
-        ).toBe('($foo, $bar, $baz...)'));
-
-      describe('with beforeRestParameter', () => {
-        it('with no parameters', () =>
-          expect(
-            new ParameterList({
-              restParameter: 'foo',
-              raws: {beforeRestParameter: '/**/'},
-            }).toString(),
-          ).toBe('(/**/$foo...)'));
-
-        it('with parameters', () =>
-          expect(
-            new ParameterList({
-              nodes: ['foo', 'bar'],
-              restParameter: 'baz',
-              raws: {beforeRestParameter: '/**/'},
-            }).toString(),
-          ).toBe('($foo, $bar,/**/$baz...)'));
-      });
-
-      it('with matching restParameter', () =>
-        expect(
-          new ParameterList({
-            restParameter: 'foo',
-            raws: {restParameter: {value: 'foo', raw: 'f\\6fo'}},
-          }).toString(),
-        ).toBe('($f\\6fo...)'));
-
-      it('with non-matching restParameter', () =>
-        expect(
-          new ParameterList({
-            restParameter: 'foo',
-            raws: {restParameter: {value: 'bar', raw: 'b\\61r'}},
-          }).toString(),
-        ).toBe('($foo...)'));
-
-      it('ignores comma', () =>
-        expect(
-          new ParameterList({
-            restParameter: 'foo',
-            raws: {comma: true},
-          }).toString(),
-        ).toBe('($foo...)'));
-
-      it('with after', () =>
-        expect(
-          new ParameterList({
-            restParameter: 'foo',
-            raws: {after: '/**/'},
-          }).toString(),
-        ).toBe('($foo.../**/)'));
-    });
   });
 
   describe('clone', () => {
@@ -867,7 +712,6 @@ describe('a parameter list', () => {
       () =>
         void (original = new ParameterList({
           nodes: ['foo', 'bar'],
-          restParameter: 'baz',
           raws: {after: '  '},
         })),
     );
@@ -882,11 +726,8 @@ describe('a parameter list', () => {
           expect(clone.nodes[0].parent).toBe(clone);
           expect(clone.nodes[1].name).toBe('bar');
           expect(clone.nodes[1].parent).toBe(clone);
-          expect(clone.restParameter).toBe('baz');
         });
 
-        it('restParameter', () => expect(clone.restParameter).toBe('baz'));
-
         it('raws', () => expect(clone.raws).toEqual({after: '  '}));
 
         it('source', () => expect(clone.source).toBe(original.source));
@@ -932,18 +773,6 @@ describe('a parameter list', () => {
           expect(clone.nodes[1].name).toBe('bar');
         });
       });
-
-      describe('restParameter', () => {
-        it('defined', () =>
-          expect(original.clone({restParameter: 'qux'}).restParameter).toBe(
-            'qux',
-          ));
-
-        it('undefined', () =>
-          expect(
-            original.clone({restParameter: undefined}).restParameter,
-          ).toBeUndefined());
-      });
     });
   });
 

pkg/sass-parser/lib/src/parameter-list.ts

@@ -8,7 +8,6 @@ import {Container} from './container';
 import {Parameter, ParameterProps} from './parameter';
 import {LazySource} from './lazy-source';
 import {Node} from './node';
-import {RawWithValue} from './raw-with-value';
 import * as sassInternal from './sass-internal';
 import * as utils from './utils';
 
@@ -32,7 +31,6 @@ export type NewParameters =
  */
 export interface ParameterListObjectProps {
   nodes?: ReadonlyArray<NewParameters>;
-  restParameter?: string;
   raws?: ParameterListRaws;
 }
 
@@ -51,22 +49,10 @@ export type ParameterListProps =
  * @category Statement
  */
 export interface ParameterListRaws {
-  /** Whitespace before the rest parameter, if one exists. */
-  beforeRestParameter?: string;
-
-  /**
-   * The name of the rest parameter, if one exists.
-   *
-   * This may be different than {@link ParameterList.restParameter} if the name
-   * contains escape codes or underscores.
-   */
-  restParameter?: RawWithValue<string>;
-
   /**
    * Whether the final parameter has a trailing comma.
    *
-   * Ignored if {@link ParameterList.nodes} is empty or if
-   * {@link ParameterList.restParameter} is set.
+   * Ignored if {@link ParameterList.nodes} is empty.
    */
   comma?: boolean;
 
@@ -99,15 +85,6 @@ export class ParameterList
   }
   private declare _nodes?: Array<Parameter>;
 
-  /**
-   * The name of the rest parameter (such as `args` in `...$args`) in this
-   * parameter list.
-   *
-   * This is the parsed and normalized value, with underscores converted to
-   * hyphens and escapes resolved to the characters they represent.
-   */
-  declare restParameter?: string;
-
   /**
    * Iterators that are currently active within this parameter list. Their
    * indices refer to the last position that has already been sent to the
@@ -124,27 +101,26 @@ export class ParameterList
       this.source = new LazySource(inner);
       // TODO: set lazy raws here to use when stringifying
       this._nodes = [];
-      this.restParameter = inner.restArgument ?? undefined;
       for (const argument of inner.arguments) {
         this.append(new Parameter(undefined, argument));
       }
+      if (inner.restArgument) {
+        // TODO: Provide source information for this argument.
+        this.append({name: inner.restArgument, rest: true});
+      }
     }
     if (this._nodes === undefined) this._nodes = [];
   }
 
   clone(overrides?: Partial<ParameterListObjectProps>): this {
-    return utils.cloneNode(this, overrides, [
-      'nodes',
-      {name: 'restParameter', explicitUndefined: true},
-      'raws',
-    ]);
+    return utils.cloneNode(this, overrides, ['nodes', 'raws']);
   }
 
   toJSON(): object;
   /** @hidden */
   toJSON(_: string, inputs: Map<postcss.Input, number>): object;
   toJSON(_?: string, inputs?: Map<postcss.Input, number>): object {
-    return utils.toJSON(this, ['nodes', 'restParameter'], inputs);
+    return utils.toJSON(this, ['nodes'], inputs);
   }
 
   append(...nodes: NewParameters[]): this {
@@ -283,21 +259,7 @@ export class ParameterList
       result += parameter.toString();
       result += parameter.raws.after ?? '';
     }
-
-    if (this.restParameter) {
-      if (this.nodes.length) {
-        result += ',' + (this.raws.beforeRestParameter ?? ' ');
-      } else if (this.raws.beforeRestParameter) {
-        result += this.raws.beforeRestParameter;
-      }
-      result +=
-        '$' +
-        (this.raws.restParameter?.value === this.restParameter
-          ? this.raws.restParameter.raw
-          : sassInternal.toCssIdentifier(this.restParameter)) +
-        '...';
-    }
-    if (this.raws.comma && this.nodes.length && !this.restParameter) {
+    if (this.raws.comma && this.nodes.length) {
       result += ',';
     }
     return result + (this.raws.after ?? '') + ')';