diff --git a/.changeset/move-token-name-validation-to-parser.md b/.changeset/move-token-name-validation-to-parser.md new file mode 100644 index 00000000..c64ede61 --- /dev/null +++ b/.changeset/move-token-name-validation-to-parser.md @@ -0,0 +1,7 @@ +--- +'@css-modules-kit/core': patch +'@css-modules-kit/ts-plugin': patch +'@css-modules-kit/codegen': patch +--- + +refactor(core, ts-plugin, codegen): report token name violations in the parse phase instead of the check phase diff --git a/packages/codegen/src/project.ts b/packages/codegen/src/project.ts index 89ac6c55..cb3bb0e3 100644 --- a/packages/codegen/src/project.ts +++ b/packages/codegen/src/project.ts @@ -158,6 +158,7 @@ export function createProject(args: ProjectArgs): Project { animation: config.animation, dashedIdents: config.dashedIdents, container: config.container, + namedExports: config.namedExports, }); } @@ -191,7 +192,6 @@ export function createProject(args: ProjectArgs): Project { let diagnostics = semanticDiagnosticsMap.get(cssModule.fileName); if (!diagnostics) { diagnostics = checkCSSModule(cssModule, { - config, getExportRecord: (m) => exportBuilder.build(m), matchesPattern, resolver, diff --git a/packages/core/src/checker.test.ts b/packages/core/src/checker.test.ts index 21a2bddb..6a57d184 100644 --- a/packages/core/src/checker.test.ts +++ b/packages/core/src/checker.test.ts @@ -6,7 +6,6 @@ import { createExportBuilder } from './export-builder.js'; import { createResolver } from './resolver.js'; import { readAndParseCSSModule } from './test/css-module.js'; import { formatDiagnostics } from './test/diagnostic.js'; -import { fakeConfig } from './test/faker.js'; import { createIFF } from './test/fixture.js'; import type { CSSModule } from './type.js'; @@ -16,7 +15,6 @@ const matchesPattern = (path: string) => path.endsWith('.module.css'); type Checker = (cssModule: CSSModule) => ReturnType; function prepareChecker(args?: Partial): Checker { - const config = args?.config ?? fakeConfig(); const resolverFn = args?.resolver ?? resolver; const matchesPatternFn = args?.matchesPattern ?? matchesPattern; const exportBuilder = createExportBuilder({ @@ -26,7 +24,6 @@ function prepareChecker(args?: Partial): Checker { }); return (cssModule: CSSModule) => { return checkCSSModule(cssModule, { - config, getExportRecord: (m) => exportBuilder.build(m), matchesPattern: matchesPatternFn, resolver: resolverFn, @@ -36,127 +33,6 @@ function prepareChecker(args?: Partial): Checker { } describe('checkCSSModule', () => { - test('report diagnostics for "__proto__" name', async () => { - const iff = await createIFF({ - 'a.module.css': dedent` - .__proto__ { color: red; } - @value __proto__, valid as __proto__ from './b.module.css'; - `, - 'b.module.css': dedent` - @value __proto__: red; - @value valid: red; - `, - }); - const check = prepareChecker(); - const diagnostics = check(readAndParseCSSModule(iff.paths['a.module.css'])!); - expect(formatDiagnostics(diagnostics, iff.rootDir)).toMatchInlineSnapshot(` - [ - { - "category": "error", - "fileName": "/a.module.css", - "length": 9, - "start": { - "column": 2, - "line": 1, - }, - "text": "\`__proto__\` is not allowed as names.", - }, - { - "category": "error", - "fileName": "/a.module.css", - "length": 9, - "start": { - "column": 8, - "line": 2, - }, - "text": "\`__proto__\` is not allowed as names.", - }, - { - "category": "error", - "fileName": "/a.module.css", - "length": 9, - "start": { - "column": 28, - "line": 2, - }, - "text": "\`__proto__\` is not allowed as names.", - }, - ] - `); - }); - test('report diagnostics for "default" name when namedExports is true', async () => { - const iff = await createIFF({ - 'a.module.css': dedent` - .default { color: red; } - @value default, valid as default from './b.module.css'; - `, - 'b.module.css': dedent` - @value default: red; - @value valid: red; - `, - }); - const check = prepareChecker({ config: fakeConfig({ namedExports: true }) }); - const diagnostics = check(readAndParseCSSModule(iff.paths['a.module.css'])!); - expect(formatDiagnostics(diagnostics, iff.rootDir)).toMatchInlineSnapshot(` - [ - { - "category": "error", - "fileName": "/a.module.css", - "length": 7, - "start": { - "column": 2, - "line": 1, - }, - "text": "\`default\` is not allowed as names when \`cmkOptions.namedExports\` is set to \`true\`.", - }, - { - "category": "error", - "fileName": "/a.module.css", - "length": 7, - "start": { - "column": 8, - "line": 2, - }, - "text": "\`default\` is not allowed as names when \`cmkOptions.namedExports\` is set to \`true\`.", - }, - { - "category": "error", - "fileName": "/a.module.css", - "length": 7, - "start": { - "column": 26, - "line": 2, - }, - "text": "\`default\` is not allowed as names when \`cmkOptions.namedExports\` is set to \`true\`.", - }, - ] - `); - }); - test('report diagnostics for backslash in name', async () => { - // NOTE: The backslash is valid syntax in class selectors, but it is invalid syntax in `@value`. - // Therefore, it is sufficient for diagnostics to be reported only for class selectors. - const iff = await createIFF({ - 'a.module.css': dedent` - .a\\1 { color: red; } - `, - }); - const check = prepareChecker(); - const diagnostics = check(readAndParseCSSModule(iff.paths['a.module.css'])!); - expect(formatDiagnostics(diagnostics, iff.rootDir)).toMatchInlineSnapshot(` - [ - { - "category": "error", - "fileName": "/a.module.css", - "length": 4, - "start": { - "column": 2, - "line": 1, - }, - "text": "Backslash (\\) is not allowed in names.", - }, - ] - `); - }); test('report diagnostics for non-exported token', async () => { const iff = await createIFF({ 'a.module.css': `@value b_1, b_2 from './b.module.css';`, diff --git a/packages/core/src/checker.ts b/packages/core/src/checker.ts index 18340848..dfa4183d 100644 --- a/packages/core/src/checker.ts +++ b/packages/core/src/checker.ts @@ -1,4 +1,3 @@ -import type { CMKConfig } from './config.js'; import type { CSSModule, Diagnostic, @@ -8,10 +7,9 @@ import type { MatchesPattern, Resolver, } from './type.js'; -import { isURLSpecifier, type TokenNameViolation, validateTokenName } from './util.js'; +import { isURLSpecifier } from './util.js'; export interface CheckerArgs { - config: CMKConfig; getExportRecord: (cssModule: CSSModule) => ExportRecord; matchesPattern: MatchesPattern; resolver: Resolver; @@ -19,17 +17,8 @@ export interface CheckerArgs { } export function checkCSSModule(cssModule: CSSModule, args: CheckerArgs): Diagnostic[] { - const { config } = args; const diagnostics: Diagnostic[] = []; - for (const token of cssModule.localTokens) { - // Reject special names as they may break .d.ts files - const violation = validateTokenName(token.name, { namedExports: config.namedExports }); - if (violation) { - diagnostics.push(createTokenNameDiagnostic(cssModule, token.loc, violation)); - } - } - for (const tokenImporter of cssModule.tokenImporters) { if (isURLSpecifier(tokenImporter.from)) continue; const from = args.resolver(tokenImporter.from, { request: cssModule.fileName }); @@ -49,16 +38,6 @@ export function checkCSSModule(cssModule: CSSModule, args: CheckerArgs): Diagnos createModuleHasNoExportedTokenDiagnostic(cssModule, tokenImporter.from, entry.name, entry.loc), ); } - const nameViolation = validateTokenName(entry.name, { namedExports: config.namedExports }); - if (nameViolation) { - diagnostics.push(createTokenNameDiagnostic(cssModule, entry.loc, nameViolation)); - } - if (entry.localName) { - const localNameViolation = validateTokenName(entry.localName, { namedExports: config.namedExports }); - if (localNameViolation) { - diagnostics.push(createTokenNameDiagnostic(cssModule, entry.localLoc!, localNameViolation)); - } - } } } } @@ -90,30 +69,6 @@ export function checkCSSModule(cssModule: CSSModule, args: CheckerArgs): Diagnos return diagnostics; } -function createTokenNameDiagnostic(cssModule: CSSModule, loc: Location, violation: TokenNameViolation): Diagnostic { - let text: string; - switch (violation) { - case 'proto-not-allowed': - text = `\`__proto__\` is not allowed as names.`; - break; - case 'default-not-allowed': - text = `\`default\` is not allowed as names when \`cmkOptions.namedExports\` is set to \`true\`.`; - break; - case 'backslash-not-allowed': - text = `Backslash (\\) is not allowed in names.`; - break; - default: - throw new Error('unreachable: unknown TokenNameViolation'); - } - return { - text, - category: 'error', - file: { fileName: cssModule.fileName, text: cssModule.text }, - start: { line: loc.start.line, column: loc.start.column }, - length: loc.end.offset - loc.start.offset, - }; -} - function createCannotImportModuleDiagnostic(cssModule: CSSModule, from: string, fromLoc: Location): Diagnostic { return { text: `Cannot import module '${from}'`, diff --git a/packages/core/src/parser/css-module-parser.test.ts b/packages/core/src/parser/css-module-parser.test.ts index f9554c49..55c2b79d 100644 --- a/packages/core/src/parser/css-module-parser.test.ts +++ b/packages/core/src/parser/css-module-parser.test.ts @@ -8,6 +8,7 @@ const options: ParseCSSModuleOptions = { animation: true, dashedIdents: false, container: false, + namedExports: false, }; describe('parseCSSModule', () => { @@ -961,6 +962,149 @@ describe('parseCSSModule', () => { } `); }); + test('reports diagnostics for `__proto__` in token names', () => { + const parsed = parseCSSModule( + dedent` + .__proto__ {} + @value __proto__, valid as __proto__ from './b.module.css'; + `, + options, + ); + expect(parsed.diagnostics).toMatchInlineSnapshot(` + [ + { + "category": "error", + "file": { + "fileName": "/test.module.css", + "text": ".__proto__ {} + @value __proto__, valid as __proto__ from './b.module.css';", + }, + "length": 9, + "start": { + "column": 2, + "line": 1, + "offset": 1, + }, + "text": "\`__proto__\` is not allowed as names.", + }, + { + "category": "error", + "file": { + "fileName": "/test.module.css", + "text": ".__proto__ {} + @value __proto__, valid as __proto__ from './b.module.css';", + }, + "length": 9, + "start": { + "column": 8, + "line": 2, + "offset": 21, + }, + "text": "\`__proto__\` is not allowed as names.", + }, + { + "category": "error", + "file": { + "fileName": "/test.module.css", + "text": ".__proto__ {} + @value __proto__, valid as __proto__ from './b.module.css';", + }, + "length": 9, + "start": { + "column": 28, + "line": 2, + "offset": 41, + }, + "text": "\`__proto__\` is not allowed as names.", + }, + ] + `); + }); + test('reports diagnostics for `default` in token names when namedExports is true', () => { + const parsed = parseCSSModule( + dedent` + .default {} + @value default, valid as default from './b.module.css'; + `, + { ...options, namedExports: true }, + ); + expect(parsed.diagnostics).toMatchInlineSnapshot(` + [ + { + "category": "error", + "file": { + "fileName": "/test.module.css", + "text": ".default {} + @value default, valid as default from './b.module.css';", + }, + "length": 7, + "start": { + "column": 2, + "line": 1, + "offset": 1, + }, + "text": "\`default\` is not allowed as names when \`cmkOptions.namedExports\` is set to \`true\`.", + }, + { + "category": "error", + "file": { + "fileName": "/test.module.css", + "text": ".default {} + @value default, valid as default from './b.module.css';", + }, + "length": 7, + "start": { + "column": 8, + "line": 2, + "offset": 19, + }, + "text": "\`default\` is not allowed as names when \`cmkOptions.namedExports\` is set to \`true\`.", + }, + { + "category": "error", + "file": { + "fileName": "/test.module.css", + "text": ".default {} + @value default, valid as default from './b.module.css';", + }, + "length": 7, + "start": { + "column": 26, + "line": 2, + "offset": 37, + }, + "text": "\`default\` is not allowed as names when \`cmkOptions.namedExports\` is set to \`true\`.", + }, + ] + `); + }); + test('does not report diagnostics for `default` in token names when namedExports is false', () => { + const parsed = parseCSSModule('.default {}', options); + expect(parsed.diagnostics).toEqual([]); + }); + test('reports diagnostics for backslash in token names', () => { + // NOTE: The backslash is valid syntax in class selectors, but it is invalid syntax in `@value`. + // Therefore, it is sufficient for diagnostics to be reported only for class selectors. + const parsed = parseCSSModule(`.a\\1 {}`, options); + expect(parsed.diagnostics).toMatchInlineSnapshot(` + [ + { + "category": "error", + "file": { + "fileName": "/test.module.css", + "text": ".a\\1 {}", + }, + "length": 4, + "start": { + "column": 2, + "line": 1, + "offset": 1, + }, + "text": "Backslash (\\) is not allowed in names.", + }, + ] + `); + }); test('does not include the token of keyframes if animation is false', () => { const cssModule = parseCSSModule('@keyframes slide-in {}', { ...options, animation: false }); expect(cssModule.localTokens).toMatchInlineSnapshot(`[]`); diff --git a/packages/core/src/parser/css-module-parser.ts b/packages/core/src/parser/css-module-parser.ts index a382b27a..6db53092 100644 --- a/packages/core/src/parser/css-module-parser.ts +++ b/packages/core/src/parser/css-module-parser.ts @@ -5,10 +5,12 @@ import type { CSSModule, DiagnosticWithDetachedLocation, DiagnosticWithLocation, + Location, Token, TokenImporter, TokenReference, } from '../type.js'; +import { type TokenNameViolation, validateTokenName } from '../util.js'; import { isAnimationNameProp, isAnimationProp, @@ -136,6 +138,60 @@ function collectTokens(ast: Root, animation: boolean, dashedIdents: boolean, con return { localTokens, tokenImporters, tokenReferences, diagnostics: allDiagnostics }; } +function validateTokenNames( + localTokens: Token[], + tokenImporters: TokenImporter[], + namedExports: boolean, +): DiagnosticWithDetachedLocation[] { + const diagnostics: DiagnosticWithDetachedLocation[] = []; + for (const token of localTokens) { + // Reject special names as they may break .d.ts files + const violation = validateTokenName(token.name, { namedExports }); + if (violation) { + diagnostics.push(createTokenNameDiagnostic(token.loc, violation)); + } + } + for (const tokenImporter of tokenImporters) { + if (tokenImporter.type !== 'named') continue; + for (const entry of tokenImporter.entries) { + const nameViolation = validateTokenName(entry.name, { namedExports }); + if (nameViolation) { + diagnostics.push(createTokenNameDiagnostic(entry.loc, nameViolation)); + } + if (entry.localName) { + const localNameViolation = validateTokenName(entry.localName, { namedExports }); + if (localNameViolation) { + diagnostics.push(createTokenNameDiagnostic(entry.localLoc!, localNameViolation)); + } + } + } + } + return diagnostics; +} + +function createTokenNameDiagnostic(loc: Location, violation: TokenNameViolation): DiagnosticWithDetachedLocation { + let text: string; + switch (violation) { + case 'proto-not-allowed': + text = `\`__proto__\` is not allowed as names.`; + break; + case 'default-not-allowed': + text = `\`default\` is not allowed as names when \`cmkOptions.namedExports\` is set to \`true\`.`; + break; + case 'backslash-not-allowed': + text = `Backslash (\\) is not allowed in names.`; + break; + default: + throw new Error('unreachable: unknown TokenNameViolation'); + } + return { + text, + category: 'error', + start: loc.start, + length: loc.end.offset - loc.start.offset, + }; +} + export interface ParseCSSModuleOptions { fileName: string; /** Whether to include syntax errors from diagnostics */ @@ -143,6 +199,7 @@ export interface ParseCSSModuleOptions { animation: boolean; dashedIdents: boolean; container: boolean; + namedExports: boolean; } /** * Parse CSS Module text. @@ -150,7 +207,7 @@ export interface ParseCSSModuleOptions { */ export function parseCSSModule( text: string, - { fileName, includeSyntaxError, animation, dashedIdents, container }: ParseCSSModuleOptions, + { fileName, includeSyntaxError, animation, dashedIdents, container, namedExports }: ParseCSSModuleOptions, ): CSSModule { let ast: Root; const diagnosticFile = { fileName, text }; @@ -184,6 +241,12 @@ export function parseCSSModule( container, ); allDiagnostics.push(...diagnostics.map((diagnostic) => ({ ...diagnostic, file: diagnosticFile }))); + allDiagnostics.push( + ...validateTokenNames(localTokens, tokenImporters, namedExports).map((diagnostic) => ({ + ...diagnostic, + file: diagnosticFile, + })), + ); return { fileName, text, diff --git a/packages/core/src/test/css-module.ts b/packages/core/src/test/css-module.ts index 4e894ad9..1a6898be 100644 --- a/packages/core/src/test/css-module.ts +++ b/packages/core/src/test/css-module.ts @@ -16,7 +16,7 @@ export function fakeCSSModule(args?: Partial): CSSModule { export function readAndParseCSSModule( path: string, - options?: { animation?: boolean; dashedIdents?: boolean; container?: boolean }, + options?: { animation?: boolean; dashedIdents?: boolean; container?: boolean; namedExports?: boolean }, ): CSSModule | undefined { let text: string; try { @@ -30,5 +30,6 @@ export function readAndParseCSSModule( animation: options?.animation ?? true, dashedIdents: options?.dashedIdents ?? false, container: options?.container ?? false, + namedExports: options?.namedExports ?? false, }); } diff --git a/packages/ts-plugin/src/language-plugin.ts b/packages/ts-plugin/src/language-plugin.ts index 0d886e92..368a7e83 100644 --- a/packages/ts-plugin/src/language-plugin.ts +++ b/packages/ts-plugin/src/language-plugin.ts @@ -52,6 +52,7 @@ export function createCSSLanguagePlugin( animation: config.animation, dashedIdents: config.dashedIdents, container: config.container, + namedExports: config.namedExports, }); // oxlint-disable-next-line prefer-const let { text, mapping, linkedCodeMapping } = generateDts(cssModule, { diff --git a/packages/ts-plugin/src/language-service/feature/semantic-diagnostic.ts b/packages/ts-plugin/src/language-service/feature/semantic-diagnostic.ts index 6b84cdb5..db548656 100644 --- a/packages/ts-plugin/src/language-service/feature/semantic-diagnostic.ts +++ b/packages/ts-plugin/src/language-service/feature/semantic-diagnostic.ts @@ -1,4 +1,4 @@ -import type { CMKConfig, CSSModule, ExportBuilder, MatchesPattern, Resolver } from '@css-modules-kit/core'; +import type { CSSModule, ExportBuilder, MatchesPattern, Resolver } from '@css-modules-kit/core'; import { checkCSSModule, convertDiagnostic } from '@css-modules-kit/core'; import type { Language } from '@volar/language-core'; import type ts from 'typescript'; @@ -11,7 +11,6 @@ export function getSemanticDiagnostics( resolver: Resolver, matchesPattern: MatchesPattern, getCSSModule: (path: string) => CSSModule | undefined, - config: CMKConfig, ): ts.LanguageService['getSemanticDiagnostics'] { return (...args) => { const [fileName] = args; @@ -25,7 +24,6 @@ export function getSemanticDiagnostics( exportBuilder.clearCache(); const diagnostics = checkCSSModule(cssModule, { - config, getExportRecord: (m) => exportBuilder.build(m), matchesPattern, resolver, diff --git a/packages/ts-plugin/src/language-service/proxy.ts b/packages/ts-plugin/src/language-service/proxy.ts index 9cf25a4a..04f54928 100644 --- a/packages/ts-plugin/src/language-service/proxy.ts +++ b/packages/ts-plugin/src/language-service/proxy.ts @@ -45,7 +45,6 @@ export function proxyLanguageService( resolver, matchesPattern, getCSSModule, - config, ); proxy.getApplicableRefactors = getApplicableRefactors(languageService, project); proxy.getEditsForRefactor = getEditsForRefactor(languageService);