From ee072f8e2893d1606efd530aebfaf73a95ad40ef Mon Sep 17 00:00:00 2001 From: Ido Rosenthal Date: Sun, 1 Oct 2023 16:35:30 +0300 Subject: [PATCH] feat: validate simple custom state template (#2905) --- packages/core/src/helpers/custom-state.ts | 82 ++++++++++++++----- .../test/features/css-pseudo-class.spec.ts | 50 +++++++++-- packages/schema-extract/test/test.spec.ts | 2 +- 3 files changed, 105 insertions(+), 29 deletions(-) diff --git a/packages/core/src/helpers/custom-state.ts b/packages/core/src/helpers/custom-state.ts index 1cd8ef0da3..e166de8773 100644 --- 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 index 299335807a..a57ab0e63b 100644 --- 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 index 53479fe96a..949f646a05 100644 --- 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);