chore(eslint-plugin): rule against invalid paths

tools/@aws-cdk/cdk-build-tools/config/eslintrc.js

@@ -44,6 +44,7 @@ module.exports = {
     '@aws-cdk/no-core-construct': ['error'],
     '@aws-cdk/invalid-cfn-imports': ['error'],
     '@aws-cdk/no-literal-partition': ['error'],
+    '@aws-cdk/no-invalid-path': [ 'error' ],
     // Require use of the `import { foo } from 'bar';` form instead of `import foo = require('bar');`
     '@typescript-eslint/no-require-imports': ['error'],
     '@typescript-eslint/indent': ['error', 2],

tools/@aws-cdk/cdk-build-tools/lib/package-info.ts

@@ -104,6 +104,8 @@ export function packageCompiler(compilers: CompilerOverrides, options?: CDKBuild
       args.push('--compress-assembly');
     }
     if (options?.stripDeprecated) {
+      // This package is not published to npm so the linter rule is invalid
+      // eslint-disable-next-line @aws-cdk/no-invalid-path
       args.push(`--strip-deprecated ${path.join(__dirname, '..', '..', '..', '..', 'deprecated_apis.txt')}`);
     }
     return [compilers.jsii || require.resolve('jsii/bin/jsii'), ...args];

tools/@aws-cdk/eslint-plugin/README.md

@@ -11,6 +11,9 @@ Eslint plugin for the CDK repository. Contains rules that need to be applied spe
   Instead use `Construct` and `IConstruct` from the "constructs" module.
   Rule only applies to typescript files under the `test/` folder.
 
+* `no-invalid-path`: Checks paths specified using `path.join()` for validity, including not going backwards (`'..'`)
+  multiple times in the path and not going backwards beyond a package's `package.json`.
+
 * `no-literal-partition`: Forbids the use of literal partitions (usually `aws`). Instead, use
   `Aws.PARTITION` to ensure that the code works for other partitions too.
 
@@ -26,4 +29,4 @@ Eslint plugin for the CDK repository. Contains rules that need to be applied spe
   as well!
 * You can now run the test in debugging mode (make sure to have `npx tsc -w` running, then from a debugging terminal, `npx jest --no-coverage -it 'your rule name'`), set a breakpoint, and inspect the typeless objects.
 
-To activate it for real on the repo, also add it to `cdk-build-tools/config/eslintrc.js`.
+To activate it for real on the repo, also add it to `cdk-build-tools/config/eslintrc.js`.

tools/@aws-cdk/eslint-plugin/lib/index.ts

@@ -2,4 +2,5 @@ export const rules = {
   'no-core-construct': require('./rules/no-core-construct'),
   'invalid-cfn-imports': require('./rules/invalid-cfn-imports'),
   'no-literal-partition': require('./rules/no-literal-partition'),
+  'no-invalid-path': require('./rules/no-invalid-path'),
 };

tools/@aws-cdk/eslint-plugin/lib/rules/no-invalid-path.ts

@@ -0,0 +1,94 @@
+import { Rule } from 'eslint';
+import { isProdFile } from '../private/is-prod-file';
+import * as path from 'path';
+import * as fs from 'fs';
+
+function isPathJoinFuncCall(node: any): boolean {
+  return node.callee?.property?.name === 'join';
+}
+
+function allArgumentsExpected(node: any): boolean {
+  // Outside of the first argument, all arguments should be strings
+  const components = node.arguments.slice(1);
+  return components.every((a: any) => a.value !== undefined);
+}
+
+function hasSlashes(args: string[]): boolean {
+  return args.some((a) => a.includes('/'));
+}
+
+function firstArgIsDirname(node: any): boolean {
+  return node.arguments[0].name && node.arguments[0].name === '__dirname';
+}
+
+function argumentList(node: any): string[] {
+  // Already confirmed that first argument is '__dirname', so can safely remove it
+  const args: string[] = node.arguments.slice(1).map((a: any) => { return a.value; });
+  return args;
+}
+
+function recreatePath(args: string[]): string {
+  return `path.join(__dirname, '${args.join('\', \'')}')`;
+}
+
+export function create(context: Rule.RuleContext): Rule.NodeListener {
+  return {
+    CallExpression(node: any) {
+      if (!isProdFile(context.getFilename())) {
+        return;
+      }
+
+      if (isPathJoinFuncCall(node)) {
+        if (node.arguments.length === 0) {
+          // ERROR: this is 'path.join()'
+          context.report({ node, message: '\'path.join()\' is not a valid path. You must specify arguments into the function.'})
+          return;
+        }
+
+        if (!allArgumentsExpected(node)) {
+          // ERROR: unexpected non-string in the argument list
+          context.report({ node, message: 'Part of the arguments to \'path.join()\' is not a string. Only the first argument can be \'__dirname\'.'})
+          return;
+        }
+
+        // We currently do not lint any path.join without '__dirname' as the first argument
+        if (!firstArgIsDirname(node)) {
+          return;
+        }
+
+        const args = argumentList(node);
+
+        if (hasSlashes(args)) {
+          // ERROR: This path looks like 'path.join(__dirname, 'a/b')' and should be changed to 'path.join(__dirname, 'a', 'b')'
+          context.report({ node, message: `${recreatePath(args)} is not a valid path. It has '/' in the arguments which is not allowed. Each directory should be its own separate argument.`});
+          return;
+        }
+
+        const firstDownDir = args.findIndex((p) => p !== '..');
+
+        // Confirm path does not have any unnecessary '..' paths
+        // This allows us to validate subsequent checks
+        if (args.some((p, i) => p === '..' && i > firstDownDir)) {
+          // ERROR: This path oscillates between up and down commands
+          context.report({ node, message: `${recreatePath(args)} is not a valid path. It goes backwards and forwards and backwards again, and can be simplified.`});
+          return;
+        }
+
+        // Note the - 1 to exclude the last directory from the check
+        // Exclude the case where there are no '..' at all in the path -- those are never invalid
+        const currentFile = context.getFilename();
+        if (firstDownDir > 0) {
+          for (let i = 0; i < firstDownDir - 1; i++) {
+            // Note i + 1 here to include the current '..' in the path, since Array.slice excludes the end index.
+            const pjFile = path.join(...[path.dirname(currentFile), ...args.slice(0, i + 1), 'package.json']);
+            if (fs.existsSync(pjFile)) {
+              // ERROR: this path will end up going out of the package.json directory
+              context.report({ node, message: `${recreatePath(args)} is not a valid path. It goes beyond the parent library's package.json file so the file it points to will not be available after the library is packaged.`});
+              return;
+            }
+          }
+        }
+      }
+    }
+  }
+}

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/eslintrc.js

@@ -0,0 +1,6 @@
+module.exports = {
+  plugins: ['@aws-cdk'],
+  rules: {
+    '@aws-cdk/no-invalid-path': [ 'error' ],
+  }
+}

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/mistake-path.error.txt

@@ -0,0 +1 @@
+Part of the arguments to 'path.join()' is not a string. Only the first argument can be '__dirname'.

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/mistake-path.ts

@@ -0,0 +1,3 @@
+import * as path from 'path';
+
+path.join('..', __dirname, 'hello.txt');

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/no-path.error.txt

@@ -0,0 +1 @@
+'path.join()' is not a valid path. You must specify arguments into the function.

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/no-path.ts

@@ -0,0 +1,3 @@
+import * as path from 'path';
+
+path.join();

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/path-beyond-packagejson.error.txt

@@ -0,0 +1 @@
+path.join(__dirname, '..', '..', '..', '..', '..', 'cfn2ts', 'README.md') is not a valid path. It goes beyond the parent library's package.json file so the file it points to will not be available after the library is packaged.

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/path-beyond-packagejson.ts

@@ -0,0 +1,4 @@
+import path from 'path';
+
+// depends on eslint-plugin to have a package.json
+path.join(__dirname, '..', '..', '..', '..', '..', 'cfn2ts', 'README.md');

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/path-oscillates.error.txt

@@ -0,0 +1 @@
+path.join(__dirname, '..', '..', 'no-path-outside-module', '..', '..', 'fixtures.test.ts') is not a valid path. It goes backwards and forwards and backwards again, and can be simplified.

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/path-oscillates.ts

@@ -0,0 +1,3 @@
+import path from 'path';
+
+path.join(__dirname, '..', '..', 'no-path-outside-module', '..', '..', 'fixtures.test.ts');

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/path-with-slashes.error.txt

@@ -0,0 +1 @@
+ath.join(__dirname, '../..', 'fixtures.test.ts') is not a valid path. It has '/' in the arguments which is not allowed. Each directory should be its own separate argument.

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/path-with-slashes.ts

@@ -0,0 +1,3 @@
+import * as path from 'path';
+
+path.join(__dirname, '../..', 'fixtures.test.ts');

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/variable-path.error.txt

@@ -0,0 +1 @@
+asdfasdf

tools/@aws-cdk/eslint-plugin/test/rules/fixtures/no-invalid-path/variable-path.ts

@@ -0,0 +1,4 @@
+import * as path from 'path';
+
+const myPath = [__dirname, '..', '../..', 'fixtures'];
+path.join(...myPath); // Should fail but does not currently because myPath is treated as one 'SpreadElement' argument.