feat: validate simple custom state template (#2905)

wix · Oct 1, 2023 · ee072f8 · ee072f8
1 parent 2d2e5b4
commit ee072f8
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 29 deletions.
diff --git a/packages/core/src/helpers/custom-state.ts b/packages/core/src/helpers/custom-state.ts
@@ -4,7 +4,7 @@ import postcssValueParser, {
     type FunctionNode,
 } from 'postcss-value-parser';
 import cssesc from 'cssesc';
-import type { PseudoClass, SelectorNode } from '@tokey/css-selector-parser';
+import type { PseudoClass, SelectorList, SelectorNode } from '@tokey/css-selector-parser';
 import { createDiagnosticReporter, Diagnostics } from '../diagnostics';
 import {
     parseSelectorWithCache,
@@ -338,7 +338,21 @@ function defineTemplateState(
     const template = stripQuotation(postcssValueParser.stringify(templateDef));
     if (argsFullValue.length === 1) {
         // simple template with no params
-        mappedStates[stateName] = template.trim().replace(/\\["']/g, '"');
+        const selectorStr = template.trim().replace(/\\["']/g, '"');
+        const selectorAst = parseSelectorWithCache(selectorStr, { clone: true });
+        if (
+            !validateTemplateSelector({
+                stateName,
+                selectorStr,
+                selectorAst,
+                cssNode: decl,
+                diagnostics,
+            })
+        ) {
+            return;
+        } else {
+            mappedStates[stateName] = selectorStr;
+        }
     } else if (argsFullValue.length === 2) {
         // single parameter template
         if (!template.includes('$0')) {
@@ -1075,30 +1089,57 @@ function transformMappedStateWithParam({
     selectorNode?: postcss.Node;
     diagnostics: Diagnostics;
 }) {
-    const targetSelectorStr = template.replace(/\$0/g, param);
-    const selectorAst = parseSelectorWithCache(targetSelectorStr, { clone: true });
+    const selectorStr = template.replace(/\$0/g, param);
+    const selectorAst = parseSelectorWithCache(selectorStr, { clone: true });
+    if (
+        !validateTemplateSelector({
+            stateName,
+            selectorStr,
+            selectorAst,
+            cssNode: selectorNode,
+            diagnostics,
+        })
+    ) {
+        return;
+    }
+    convertToSelector(node).nodes = selectorAst[0].nodes;
+}
+
+function validateTemplateSelector({
+    stateName,
+    selectorStr,
+    selectorAst,
+    cssNode,
+    diagnostics,
+}: {
+    stateName: string;
+    selectorStr: string;
+    selectorAst: SelectorList;
+    cssNode?: postcss.Node;
+    diagnostics: Diagnostics;
+}): boolean {
     if (selectorAst.length > 1) {
-        if (selectorNode) {
+        if (cssNode) {
             diagnostics.report(
-                stateDiagnostics.UNSUPPORTED_MULTI_SELECTOR(stateName, targetSelectorStr),
+                stateDiagnostics.UNSUPPORTED_MULTI_SELECTOR(stateName, selectorStr),
                 {
-                    node: selectorNode,
+                    node: cssNode,
                 }
             );
         }
-        return;
+        return false;
     } else {
         const firstSelector = selectorAst[0].nodes.find(({ type }) => type !== 'comment');
         if (firstSelector?.type === 'type' || firstSelector?.type === 'universal') {
-            if (selectorNode) {
+            if (cssNode) {
                 diagnostics.report(
-                    stateDiagnostics.UNSUPPORTED_INITIAL_SELECTOR(stateName, targetSelectorStr),
+                    stateDiagnostics.UNSUPPORTED_INITIAL_SELECTOR(stateName, selectorStr),
                     {
-                        node: selectorNode,
+                        node: cssNode,
                     }
                 );
             }
-            return;
+            return false;
         }
         let unexpectedSelector: undefined | SelectorNode = undefined;
         for (const node of selectorAst[0].nodes) {
@@ -1108,33 +1149,30 @@ function transformMappedStateWithParam({
             }
         }
         if (unexpectedSelector) {
-            if (selectorNode) {
+            if (cssNode) {
                 switch (unexpectedSelector.type) {
                     case 'combinator':
                         diagnostics.report(
-                            stateDiagnostics.UNSUPPORTED_COMPLEX_SELECTOR(
-                                stateName,
-                                targetSelectorStr
-                            ),
+                            stateDiagnostics.UNSUPPORTED_COMPLEX_SELECTOR(stateName, selectorStr),
                             {
-                                node: selectorNode,
+                                node: cssNode,
                             }
                         );
                         break;
                     case 'invalid':
                         diagnostics.report(
-                            stateDiagnostics.INVALID_SELECTOR(stateName, targetSelectorStr),
+                            stateDiagnostics.INVALID_SELECTOR(stateName, selectorStr),
                             {
-                                node: selectorNode,
+                                node: cssNode,
                             }
                         );
                         break;
                 }
             }
-            return;
+            return false;
         }
     }
-    convertToSelector(node).nodes = selectorAst[0].nodes;
+    return true;
 }
 
 function resolveParam(

diff --git a/packages/core/test/features/css-pseudo-class.spec.ts b/packages/core/test/features/css-pseudo-class.spec.ts
@@ -459,6 +459,50 @@ describe('features/css-pseudo-class', () => {
                     .root:static(unknown-param) {}
                 `);
             });
+            it('should report invalid template selector', () => {
+                testStylableCore(`
+                    .a {
+                        /* 
+                            @analyze-error(not-compound) ${stCustomStateDiagnostics.UNSUPPORTED_COMPLEX_SELECTOR(
+                                'notCompound',
+                                '.x .y'
+                            )}
+                        */
+                        -st-states: notCompound(".x .y");
+                    }
+                    .b {
+                        /* 
+                            @analyze-error(multi) ${stCustomStateDiagnostics.UNSUPPORTED_MULTI_SELECTOR(
+                                'multi',
+                                '.x, .y'
+                            )}
+                        */
+                        -st-states: multi(".x, .y");
+                    }
+                    .c {
+                        /* 
+                            @analyze-error(invalid) ${stCustomStateDiagnostics.INVALID_SELECTOR(
+                                'invalid',
+                                ':unclosed('
+                            )}
+                        */
+                        -st-states: invalid(":unclosed(");
+                    }
+                    .d {
+                        /* 
+                            @analyze-error(invalidStart) ${stCustomStateDiagnostics.UNSUPPORTED_INITIAL_SELECTOR(
+                                'invalidStartElement',
+                                'div.x'
+                            )}
+                            @analyze-error(invalidStart) ${stCustomStateDiagnostics.UNSUPPORTED_INITIAL_SELECTOR(
+                                'invalidStartWildcard',
+                                '*.x'
+                            )}
+                        */
+                        -st-states: invalidStartElement("div.x"), invalidStartWildcard("*.x");
+                    }
+                `);
+            });
         });
         describe('custom mapped parameter', () => {
             it('should transform mapped state (quoted)', () => {
@@ -517,12 +561,6 @@ describe('features/css-pseudo-class', () => {
                 shouldReportNoDiagnostics(meta);
             });
             it('should report invalid template selector', () => {
-                /**
-                 * currently only checks template with parameter
-                 * for backwards compatibility standalone template can accept
-                 * any kind of selector - we might want to limit this in a future
-                 * major version.
-                 */
                 testStylableCore(`
                     .root {
                         -st-states: classAndThenParam(".x$0", string),

diff --git a/packages/schema-extract/test/test.spec.ts b/packages/schema-extract/test/test.spec.ts
@@ -259,7 +259,7 @@ describe('Stylable JSON Schema Extractor', () => {
 
         it('schema with mapped states', () => {
             const css = `.root{
-                -st-states: state("custom");
+                -st-states: state(".custom");
             }`;
 
             const res = extractSchema(css, '/entry.st.css', '/', path);