Improve non-JS identifier error message: Phase 2

.changeset/spicy-wings-hide.md

@@ -0,0 +1,5 @@
+---
+'@css-modules-kit/core': minor
+---
+
+feat: improve non-JS identifier error message

packages/core/src/checker.test.ts

@@ -33,6 +33,96 @@ function fakeLoc({ column }: { column: number }): Location {
 }
 
 describe('checkCSSModule', () => {
+  test('report diagnostics for invalid name as js identifier', () => {
+    const args = prepareCheckerArgs([
+      fakeCSSModule({
+        fileName: '/a.module.css',
+        localTokens: [fakeToken({ name: 'a-1', loc: fakeLoc({ column: 1 }) })],
+        tokenImporters: [
+          fakeAtValueTokenImporter({
+            from: './b.module.css',
+            fromLoc: fakeLoc({ column: 2 }),
+            values: [
+              fakeAtValueTokenImporterValue({ name: 'b-1', loc: fakeLoc({ column: 3 }) }),
+              fakeAtValueTokenImporterValue({
+                name: 'b-2',
+                loc: fakeLoc({ column: 4 }),
+                localName: 'a-2',
+                localLoc: fakeLoc({ column: 5 }),
+              }),
+            ],
+          }),
+        ],
+      }),
+      fakeCSSModule({
+        fileName: '/b.module.css',
+        localTokens: [fakeToken({ name: 'b-1' }), fakeToken({ name: 'b-2' })],
+      }),
+    ]);
+    const diagnostics = checkCSSModule(
+      args.cssModules[0],
+      args.exportBuilder,
+      args.matchesPattern,
+      args.resolver,
+      args.getCSSModule,
+    );
+    expect(diagnostics).toMatchInlineSnapshot(`
+      [
+        {
+          "category": "error",
+          "file": {
+            "fileName": "/a.module.css",
+            "text": "",
+          },
+          "length": 0,
+          "start": {
+            "column": 1,
+            "line": 1,
+          },
+          "text": "css-modules-kit does not support invalid names as JavaScript identifiers.",
+        },
+        {
+          "category": "error",
+          "file": {
+            "fileName": "/a.module.css",
+            "text": "",
+          },
+          "length": 0,
+          "start": {
+            "column": 3,
+            "line": 1,
+          },
+          "text": "css-modules-kit does not support invalid names as JavaScript identifiers.",
+        },
+        {
+          "category": "error",
+          "file": {
+            "fileName": "/a.module.css",
+            "text": "",
+          },
+          "length": 0,
+          "start": {
+            "column": 4,
+            "line": 1,
+          },
+          "text": "css-modules-kit does not support invalid names as JavaScript identifiers.",
+        },
+        {
+          "category": "error",
+          "file": {
+            "fileName": "/a.module.css",
+            "text": "",
+          },
+          "length": 0,
+          "start": {
+            "column": 5,
+            "line": 1,
+          },
+          "text": "css-modules-kit does not support invalid names as JavaScript identifiers.",
+        },
+      ]
+    `);
+  });
   test('report diagnostics for non-existing module', () => {
     const args = prepareCheckerArgs([
       fakeCSSModule({

packages/core/src/checker.ts

@@ -4,10 +4,12 @@ import type {
   CSSModule,
   Diagnostic,
   ExportBuilder,
+  Location,
   MatchesPattern,
   Resolver,
   TokenImporter,
 } from './type.js';
+import { isValidAsJSIdentifier } from './util.js';
 
 export function checkCSSModule(
   cssModule: CSSModule,
@@ -18,6 +20,12 @@ export function checkCSSModule(
 ): Diagnostic[] {
   const diagnostics: Diagnostic[] = [];
 
+  for (const token of cssModule.localTokens) {
+    if (!isValidAsJSIdentifier(token.name)) {
+      diagnostics.push(createInvalidNameAsJSIdentifiersDiagnostic(cssModule, token.loc));
+    }
+  }
+
   for (const tokenImporter of cssModule.tokenImporters) {
     const from = resolver(tokenImporter.from, { request: cssModule.fileName });
     if (!from || !matchesPattern(from)) continue;
@@ -33,6 +41,12 @@ export function checkCSSModule(
         if (!exportRecord.allTokens.includes(value.name)) {
           diagnostics.push(createModuleHasNoExportedTokenDiagnostic(cssModule, tokenImporter, value));
         }
+        if (!isValidAsJSIdentifier(value.name)) {
+          diagnostics.push(createInvalidNameAsJSIdentifiersDiagnostic(cssModule, value.loc));
+        }
+        if (value.localName && !isValidAsJSIdentifier(value.localName)) {
+          diagnostics.push(createInvalidNameAsJSIdentifiersDiagnostic(cssModule, value.localLoc!));
+        }
       }
     }
   }
@@ -62,3 +76,13 @@ function createModuleHasNoExportedTokenDiagnostic(
     length: value.loc.end.offset - value.loc.start.offset,
   };
 }
+
+function createInvalidNameAsJSIdentifiersDiagnostic(cssModule: CSSModule, loc: Location): Diagnostic {
+  return {
+    text: `css-modules-kit does not support invalid names as JavaScript identifiers.`,
+    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,
+  };
+}

packages/core/src/dts-creator.test.ts

@@ -3,6 +3,7 @@ import type { CreateDtsHost } from './dts-creator.js';
 import { createDts, type CreateDtsOptions } from './dts-creator.js';
 import { fakeCSSModule } from './test/css-module.js';
 import { fakeMatchesPattern, fakeResolver } from './test/faker.js';
+import { fakeAtValueTokenImporter, fakeAtValueTokenImporterValue, fakeToken } from './test/token.js';
 
 const host: CreateDtsHost = {
   resolver: fakeResolver(),
@@ -208,6 +209,38 @@ describe('createDts', () => {
       "
     `);
   });
+  test('does not create types for invalid name as JS identifier', () => {
+    expect(
+      createDts(
+        fakeCSSModule({
+          localTokens: [fakeToken({ name: 'a-1', loc: fakeLoc(0) })],
+          tokenImporters: [
+            fakeAtValueTokenImporter({
+              from: './b.module.css',
+              fromLoc: fakeLoc(1),
+              values: [
+                fakeAtValueTokenImporterValue({ name: 'b-1', loc: fakeLoc(2) }),
+                fakeAtValueTokenImporterValue({
+                  name: 'b_2',
+                  loc: fakeLoc(3),
+                  localName: 'a-2',
+                  localLoc: fakeLoc(4),
+                }),
+              ],
+            }),
+          ],
+        }),
+        host,
+        options,
+      ).text,
+    ).toMatchInlineSnapshot(`
+      "// @ts-nocheck
+      declare const styles = {
+      };
+      export default styles;
+      "
+    `);
+  });
   test('creates d.ts file with named exports', () => {
     expect(
       createDts(

packages/core/src/dts-creator.ts

@@ -1,4 +1,5 @@
 import type { CSSModule, MatchesPattern, Resolver, Token, TokenImporter } from './type.js';
+import { isValidAsJSIdentifier } from './util.js';
 
 export const STYLES_EXPORT_NAME = 'styles';
 
@@ -46,15 +47,34 @@ interface CreateDtsResult {
  * Create a d.ts file.
  */
 export function createDts(cssModules: CSSModule, host: CreateDtsHost, options: CreateDtsOptions): CreateDtsResult {
-  // Filter external files
-  const tokenImporters = cssModules.tokenImporters.filter((tokenImporter) => {
-    const resolved = host.resolver(tokenImporter.from, { request: cssModules.fileName });
-    return resolved !== undefined && host.matchesPattern(resolved);
-  });
+  // Exclude tokens that are not valid as JS identifiers
+  const localTokens = cssModules.localTokens.filter((token) => isValidAsJSIdentifier(token.name));
+  const tokenImporters = cssModules.tokenImporters
+    // Exclude imported tokens that are not valid as JS identifiers
+    .map((tokenImporter) => {
+      if (tokenImporter.type === 'value') {
+        return {
+          ...tokenImporter,
+          values: tokenImporter.values.filter(
+            (value) =>
+              isValidAsJSIdentifier(value.name) &&
+              (value.localName === undefined || isValidAsJSIdentifier(value.localName)),
+          ),
+        };
+      } else {
+        return tokenImporter;
+      }
+    })
+    // Exclude token importers for external files
+    .filter((tokenImporter) => {
+      const resolved = host.resolver(tokenImporter.from, { request: cssModules.fileName });
+      return resolved !== undefined && host.matchesPattern(resolved);
+    });
+
   if (options.namedExports) {
-    return createNamedExportsDts(cssModules.localTokens, tokenImporters, options);
+    return createNamedExportsDts(localTokens, tokenImporters, options);
   } else {
-    return createDefaultExportDts(cssModules.localTokens, tokenImporters);
+    return createDefaultExportDts(localTokens, tokenImporters);
   }
 }
 

packages/core/src/parser/at-value-parser.test.ts

@@ -412,12 +412,10 @@ describe('parseAtValue', () => {
     `);
   });
   test('invalid', () => {
-    const [atValue1, atValue2, atValue3, atValue4] = fakeAtValues(
+    const [atValue1, atValue2] = fakeAtValues(
       fakeRoot(dedent`
         @value;
         @value a,,b from "test.css";
-        @value non-js-ident-1: #000;
-        @value non-js-ident-1, non-js-ident-2 as alias_1, a as non-js-ident-3 from "test.css";
       `),
     );
     expect(parseAtValue(atValue1!)).toMatchInlineSnapshot(`
@@ -498,70 +496,5 @@ describe('parseAtValue', () => {
         ],
       }
     `);
-    expect(parseAtValue(atValue3!)).toMatchInlineSnapshot(`
-      {
-        "diagnostics": [
-          {
-            "category": "error",
-            "length": 14,
-            "start": {
-              "column": 8,
-              "line": 3,
-            },
-            "text": "css-modules-kit does not support non-JavaScript identifier as value names.",
-          },
-        ],
-      }
-    `);
-    expect(parseAtValue(atValue4!)).toMatchInlineSnapshot(`
-      {
-        "atValue": {
-          "from": "test.css",
-          "fromLoc": {
-            "end": {
-              "column": 85,
-              "line": 4,
-              "offset": 150,
-            },
-            "start": {
-              "column": 77,
-              "line": 4,
-              "offset": 142,
-            },
-          },
-          "type": "valueImportDeclaration",
-          "values": [],
-        },
-        "diagnostics": [
-          {
-            "category": "error",
-            "length": 14,
-            "start": {
-              "column": 8,
-              "line": 4,
-            },
-            "text": "css-modules-kit does not support non-JavaScript identifier as value names.",
-          },
-          {
-            "category": "error",
-            "length": 14,
-            "start": {
-              "column": 24,
-              "line": 4,
-            },
-            "text": "css-modules-kit does not support non-JavaScript identifier as value names.",
-          },
-          {
-            "category": "error",
-            "length": 14,
-            "start": {
-              "column": 56,
-              "line": 4,
-            },
-            "text": "css-modules-kit does not support non-JavaScript identifier as value names.",
-          },
-        ],
-      }
-    `);
   });
 });

packages/core/src/parser/at-value-parser.ts

@@ -1,6 +1,5 @@
 import type { AtRule } from 'postcss';
 import type { DiagnosticPosition, DiagnosticWithDetachedLocation, Location } from '../type.js';
-import { JS_IDENTIFIER_PATTERN } from '../util.js';
 
 interface ValueDeclaration {
   type: 'valueDeclaration';
@@ -84,17 +83,6 @@ export function parseAtValue(atValue: AtRule): ParseAtValueResult {
           column: start.column + name.length,
           offset: start.offset + name.length,
         };
-
-        if (!JS_IDENTIFIER_PATTERN.test(name)) {
-          diagnostics.push({
-            start: { line: start.line, column: start.column },
-            length: name.length,
-            text: `css-modules-kit does not support non-JavaScript identifier as value names.`,
-            category: 'error',
-          });
-          continue;
-        }
-
         const result = { name, loc: { start, end } };
         if (localName === undefined) {
           values.push(result);
@@ -110,17 +98,6 @@ export function parseAtValue(atValue: AtRule): ParseAtValueResult {
             column: start.column + localName.length,
             offset: start.offset + localName.length,
           };
-
-          if (!JS_IDENTIFIER_PATTERN.test(localName)) {
-            diagnostics.push({
-              start: { line: start.line, column: start.column },
-              length: localName.length,
-              text: `css-modules-kit does not support non-JavaScript identifier as value names.`,
-              category: 'error',
-            });
-            continue;
-          }
-
           values.push({ ...result, localName, localLoc: { start, end } });
         }
       } else {
@@ -177,17 +154,6 @@ export function parseAtValue(atValue: AtRule): ParseAtValueResult {
       column: start.column + name.length,
       offset: start.offset + name.length,
     };
-
-    if (!JS_IDENTIFIER_PATTERN.test(name)) {
-      diagnostics.push({
-        start: { line: start.line, column: start.column },
-        length: name.length,
-        text: `css-modules-kit does not support non-JavaScript identifier as value names.`,
-        category: 'error',
-      });
-      return { diagnostics };
-    }
-
     const parsedAtValue: ValueDeclaration = {
       type: 'valueDeclaration',
       name,