improves validator logic

Author: danieldelcore

.changeset/cool-clocks-behave.md

@@ -0,0 +1,7 @@
+---
+'@codeshift/cli': patch
+'@codeshift/fetcher': patch
+'@codeshift/validator': patch
+---
+
+Updates validator logic to check for invalid properties in config files. Also requires configs via absolute paths to make the validation logic more robust

packages/cli/src/main.spec.ts

@@ -4,6 +4,7 @@ jest.mock('jscodeshift/src/Runner', () => ({
   run: jest.fn().mockImplementation(() => Promise.resolve()),
 }));
 
+import fs from 'fs';
 import path from 'path';
 // @ts-ignore
 import * as jscodeshift from 'jscodeshift/src/Runner';
@@ -558,6 +559,7 @@ describe('main', () => {
     });
 
     it('should run package transform for single version', async () => {
+      jest.spyOn(fs, 'existsSync').mockReturnValue(true);
       jest.mock(
         mockMatchedPath,
         () => ({
@@ -589,6 +591,7 @@ describe('main', () => {
     });
 
     it('should run single preset', async () => {
+      jest.spyOn(fs, 'existsSync').mockReturnValue(true);
       await main([mockPath], {
         packages: 'mylib#update-formatting',
         parser: 'babel',

packages/cli/src/validate.ts

@@ -1,13 +1,8 @@
 import { isValidConfigAtPath, isValidPackageJson } from '@codeshift/validator';
 
 export default async function validate(targetPath: string = '.') {
-  try {
-    await isValidConfigAtPath(targetPath);
-    await isValidPackageJson(targetPath);
-  } catch (error) {
-    console.warn(error);
-    process.exit(1);
-  }
+  await isValidConfigAtPath(targetPath);
+  await isValidPackageJson(targetPath);
 
   console.log('Valid ✅');
 }

packages/fetcher/src/index.spec.ts

@@ -1,7 +1,8 @@
 jest.mock('globby');
 
-import globby from 'globby';
+import fs from 'fs';
 import path from 'path';
+import globby from 'globby';
 import { PluginManager } from 'live-plugin-manager';
 
 import { fetchConfig, fetchPackage, fetchRemotePackage } from '.';
@@ -27,6 +28,8 @@ describe('fetcher', () => {
     ((globby as unknown) as jest.Mock).mockImplementation(() =>
       Promise.resolve(mockMatchedPaths),
     );
+
+    jest.spyOn(fs, 'existsSync').mockReturnValue(true);
   });
 
   afterEach(() => {

packages/fetcher/src/index.ts

@@ -1,3 +1,4 @@
+import fs from 'fs';
 import path from 'path';
 import globby from 'globby';
 import { PluginManager } from 'live-plugin-manager';
@@ -18,11 +19,20 @@ export async function fetchConfig(
   ]);
 
   for (const matchedPath of matchedPaths) {
+    const resolvedMatchedPath = path.resolve(matchedPath);
+    const exists = fs.existsSync(resolvedMatchedPath);
+
+    if (!exists) continue;
+
     try {
       // eslint-disable-next-line @typescript-eslint/no-var-requires
-      const pkg = require(matchedPath);
+      const pkg = require(resolvedMatchedPath);
       return resolveConfigExport(pkg);
-    } catch (e) {}
+    } catch (e) {
+      throw new Error(
+        `Found config file "${matchedPath}" but was unable to parse it. This can be caused when transform or preset paths are incorrect.`,
+      );
+    }
   }
 
   return undefined;

packages/validator/src/index.spec.ts

@@ -125,6 +125,28 @@ describe('validator', () => {
       expect(result).toEqual(true);
     });
 
+    it('should error if config contains an invalid property', async () => {
+      (fetchConfig as jest.Mock).mockResolvedValue({
+        invalidProperty: 'foo',
+      });
+
+      await expect(isValidConfigAtPath('path/to/')).rejects.toThrowError(
+        `Invalid transform ids found: invalidProperty`,
+      );
+    });
+
+    it('should error if config contains multiple invalid properties', async () => {
+      (fetchConfig as jest.Mock).mockResolvedValue({
+        invalidProperty: 'foo',
+        invalidProperty2: 'foo',
+        invalidProperty3: 'foo',
+      });
+
+      await expect(isValidConfigAtPath('path/to/')).rejects.toThrowError(
+        `Invalid transform ids found: invalidProperty, invalidProperty2, invalidProperty3`,
+      );
+    });
+
     it('should error if config contains invalid transforms', async () => {
       (fetchConfig as jest.Mock).mockResolvedValue({
         transforms: {

packages/validator/src/index.ts

@@ -19,6 +19,18 @@ function hasValidPresets(config: CodeshiftConfig): boolean {
   );
 }
 
+function getInvalidProperties(config: CodeshiftConfig) {
+  const validProperties = [
+    'maintainers',
+    'description',
+    'targets',
+    'transforms',
+    'presets',
+  ];
+
+  return Object.keys(config).filter(key => !validProperties.includes(key));
+}
+
 export function isValidPackageName(dir: string): boolean {
   return !!dir.match(/^(@[a-z0-9-~][a-z0-9-._~]*__)?[a-z0-9-~][a-z0-9-._~]*$/);
 }
@@ -34,6 +46,13 @@ export async function isValidConfigAtPath(filePath: string) {
     throw new Error(`Unable to locate config file at path: ${filePath}`);
   }
 
+  const invalidProperites = getInvalidProperties(config);
+  if (invalidProperites.length) {
+    throw new Error(
+      `Invalid transform ids found: ${invalidProperites.join(', ')}`,
+    );
+  }
+
   if (!hasValidTransforms(config)) {
     throw new Error(`Invalid transform ids found for config at "${filePath}".
 Please make sure all transforms are identified by a valid semver version. ie 10.0.0`);

yarn.lock

@@ -2463,9 +2463,9 @@ camelcase@^6.0.0:
   integrity sha512-c7wVvbw3f37nuobQNtgsgG9POC9qMbNuMQmTCqZv23b6MIz0fcYpBiOlv9gEN/hdLdnZTDQhg6e9Dq5M1vKvfg==
 
 caniuse-lite@^1.0.30001251:
-  version "1.0.30001252"
-  resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001252.tgz#cb16e4e3dafe948fc4a9bb3307aea054b912019a"
-  integrity sha512-I56jhWDGMtdILQORdusxBOH+Nl/KgQSdDmpJezYddnAkVOmnoU8zwjTV9xAjMIYxr0iPreEAVylCGcmHCjfaOw==
+  version "1.0.30001332"
+  resolved "https://packages.atlassian.com/api/npm/npm-remote/caniuse-lite/-/caniuse-lite-1.0.30001332.tgz"
+  integrity sha512-10T30NYOEQtN6C11YGg411yebhvpnC6Z102+B95eAsN0oB6KUs01ivE8u+G6FMIRtIrVlYXhL+LUwQ3/hXwDWw==
 
 capture-exit@^2.0.0:
   version "2.0.0"