build: add lint rule to avoid relative cross entry-point imports/exports (#17741)

devversion · jelbourn · commit a8cff1e2751d · 2019-11-22T15:12:06.000-08:00
Adds a new lint rule to avoid relative cross entry-point imports
and exports. If a public entry-point imports from another entry-point
it should use the public module name in order to comply with the
Angular Package format, and to avoid unnecessary code inlining when
creating flat entry-point bundles.

Restructures the cdk testing fake-event code into `cdk/testing/testbed`
since that is where the fake event code is primarily used. This
makes the new lint rule happy, and also improves the public API
of `cdk/testing` since interfaces like `ElementDimensions` are
part of the `TestElement` class and should therefore be exported.
diff --git a/src/cdk/testing/private/BUILD.bazel b/src/cdk/testing/private/BUILD.bazel
@@ -10,7 +10,7 @@ ts_library(
     ),
     module_name = "@angular/cdk/testing/private",
     deps = [
-        "//src/cdk/testing",
+        "//src/cdk/testing/testbed",
         "@npm//@angular/core",
         "@npm//@types/jasmine",
     ],
diff --git a/src/cdk/testing/private/public-api.ts b/src/cdk/testing/private/public-api.ts
@@ -6,8 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-export * from '../fake-events'; // Re-exported for convenince.
 export * from './expect-async-error';
 export * from './wrapped-error-message';
 export * from './mock-ng-zone';
 export * from './text-dedent';
+
+// Re-exported for convenience.
+export * from '../testbed/fake-events';
diff --git a/src/cdk/testing/protractor/BUILD.bazel b/src/cdk/testing/protractor/BUILD.bazel
@@ -11,7 +11,6 @@ ts_library(
     module_name = "@angular/cdk/testing/protractor",
     deps = [
         "//src/cdk/testing",
-        "//src/cdk/testing/private",
         "@npm//protractor",
     ],
 )
diff --git a/src/cdk/testing/protractor/protractor-element.ts b/src/cdk/testing/protractor/protractor-element.ts
@@ -6,10 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {ElementDimensions, ModifierKeys, TestElement, TestKey} from '@angular/cdk/testing';
 import {browser, ElementFinder, Key} from 'protractor';
-import {ElementDimensions} from '../element-dimensions';
-import {TestElement, TestKey} from '../test-element';
-import {ModifierKeys} from '../fake-events';
 
 /** Maps the `TestKey` constants to Protractor's `Key` constants. */
 const keyMap = {
diff --git a/src/cdk/testing/protractor/protractor-harness-environment.ts b/src/cdk/testing/protractor/protractor-harness-environment.ts
@@ -6,10 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {HarnessEnvironment} from '@angular/cdk/testing';
+import {HarnessEnvironment, HarnessLoader, TestElement} from '@angular/cdk/testing';
 import {by, element as protractorElement, ElementFinder} from 'protractor';
-import {HarnessLoader} from '../component-harness';
-import {TestElement} from '../test-element';
 import {ProtractorElement} from './protractor-element';
 
 /** A `HarnessEnvironment` implementation for Protractor. */
diff --git a/src/cdk/testing/public-api.ts b/src/cdk/testing/public-api.ts
@@ -9,3 +9,4 @@
 export * from './component-harness';
 export * from './harness-environment';
 export * from './test-element';
+export * from './element-dimensions';
diff --git a/src/cdk/testing/test-element.ts b/src/cdk/testing/test-element.ts
@@ -7,7 +7,14 @@
  */
 
 import {ElementDimensions} from './element-dimensions';
-import {ModifierKeys} from './fake-events';
+
+/** Modifier keys that may be held while typing. */
+export interface ModifierKeys {
+  control?: boolean;
+  alt?: boolean;
+  shift?: boolean;
+  meta?: boolean;
+}
 
 /** An enum of non-text keys that can be used with the `sendKeys` method. */
 // NOTE: This is a separate enum from `@angular/cdk/keycodes` because we don't necessarily want to
diff --git a/src/cdk/testing/testbed/BUILD.bazel b/src/cdk/testing/testbed/BUILD.bazel
@@ -12,7 +12,6 @@ ts_library(
     deps = [
         "//src/cdk/keycodes",
         "//src/cdk/testing",
-        "//src/cdk/testing/private",
         "@npm//@angular/core",
         "@npm//rxjs",
     ],
diff --git a/src/cdk/testing/testbed/fake-events/dispatch-events.ts b/src/cdk/testing/testbed/fake-events/dispatch-events.ts
@@ -6,12 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {ModifierKeys} from '@angular/cdk/testing';
 import {
   createFakeEvent,
   createKeyboardEvent,
   createMouseEvent,
   createTouchEvent,
-  ModifierKeys
 } from './event-objects';
 
 /**
diff --git a/src/cdk/testing/testbed/fake-events/element-focus.ts b/src/cdk/testing/testbed/fake-events/element-focus.ts
diff --git a/src/cdk/testing/testbed/fake-events/event-objects.ts b/src/cdk/testing/testbed/fake-events/event-objects.ts
@@ -6,13 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-/** Modifier keys that may be held while typing. */
-export interface ModifierKeys {
-  control?: boolean;
-  alt?: boolean;
-  shift?: boolean;
-  meta?: boolean;
-}
+import {ModifierKeys} from '@angular/cdk/testing';
 
 /**
  * Creates a browser MouseEvent with the specified options.
diff --git a/src/cdk/testing/testbed/fake-events/index.ts b/src/cdk/testing/testbed/fake-events/index.ts
@@ -6,8 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-// These are private APIs that are used both by the public APIs inside of this package, as well as
-// in the unit tests of other packages, hence why we need to re-export them through here.
+// These are private APIs that are used both by the public APIs inside of this package, as well
+// as in unit tests of other entry-points, hence why we need to re-export them through here.
 export * from './dispatch-events';
 export * from './event-objects';
 export * from './element-focus';
diff --git a/src/cdk/testing/testbed/fake-events/type-in-element.ts b/src/cdk/testing/testbed/fake-events/type-in-element.ts
@@ -6,9 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {ModifierKeys} from '@angular/cdk/testing';
 import {dispatchFakeEvent, dispatchKeyboardEvent} from './dispatch-events';
 import {triggerFocus} from './element-focus';
-import {ModifierKeys} from './event-objects';
 
 /**
  * Checks whether the given Element is a text input element.
diff --git a/src/cdk/testing/testbed/testbed-harness-environment.ts b/src/cdk/testing/testbed/testbed-harness-environment.ts
@@ -6,12 +6,16 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {HarnessEnvironment} from '@angular/cdk/testing';
+import {
+  ComponentHarness,
+  ComponentHarnessConstructor,
+  HarnessEnvironment,
+  HarnessLoader,
+  TestElement
+} from '@angular/cdk/testing';
 import {ComponentFixture, flush} from '@angular/core/testing';
 import {Observable} from 'rxjs';
 import {takeWhile} from 'rxjs/operators';
-import {ComponentHarness, ComponentHarnessConstructor, HarnessLoader} from '../component-harness';
-import {TestElement} from '../test-element';
 import {TaskState, TaskStateZoneInterceptor} from './task-state-zone-interceptor';
 import {UnitTestElement} from './unit-test-element';
 
diff --git a/src/cdk/testing/testbed/unit-test-element.ts b/src/cdk/testing/testbed/unit-test-element.ts
@@ -7,17 +7,15 @@
  */
 
 import * as keyCodes from '@angular/cdk/keycodes';
-import {TestElement, TestKey} from '../test-element';
-import {ElementDimensions} from '../element-dimensions';
+import {ElementDimensions, ModifierKeys, TestElement, TestKey} from '@angular/cdk/testing';
 import {
-  ModifierKeys,
+  clearElement,
   dispatchMouseEvent,
+  isTextInput,
   triggerBlur,
   triggerFocus,
-  isTextInput,
-  clearElement,
   typeInElement,
-} from '../fake-events';
+} from './fake-events';
 
 /** Maps `TestKey` constants to the `keyCode` and `key` values used by native browser events. */
 const keyMap = {
diff --git a/src/material-experimental/form-field/testing/form-field-harness.ts b/src/material-experimental/form-field/testing/form-field-harness.ts
@@ -13,9 +13,11 @@ import {
   HarnessQuery,
   TestElement
 } from '@angular/cdk/testing';
+import {
+  MatFormFieldControlHarness
+} from '@angular/material-experimental/form-field/testing/control';
 import {MatInputHarness} from '@angular/material-experimental/input/testing';
 import {MatSelectHarness} from '@angular/material-experimental/select/testing';
-import {MatFormFieldControlHarness} from './control';
 import {FormFieldHarnessFilters} from './form-field-harness-filters';
 
 // TODO(devversion): support datepicker harness once developed (COMP-203).
diff --git a/test/karma-system-config.js b/test/karma-system-config.js
@@ -207,7 +207,7 @@ System.config({
     'rxjs/operators': {main: 'index'},
 
     // Needed for relative imports inside the testing package to work.
-    'dist/packages/cdk/testing/fake-events': {main: 'index'},
+    'dist/packages/cdk/testing/testbed/fake-events': {main: 'index'},
 
     // Set the default extension for the root package, because otherwise the tests can't
     // be built within the production mode. Due to missing file extensions.
diff --git a/tools/public_api_guard/cdk/testing.d.ts b/tools/public_api_guard/cdk/testing.d.ts
@@ -26,6 +26,13 @@ export interface ComponentHarnessConstructor<T extends ComponentHarness> {
     new (locatorFactory: LocatorFactory): T;
 }
 
+export interface ElementDimensions {
+    height: number;
+    left: number;
+    top: number;
+    width: number;
+}
+
 export declare abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFactory {
     protected rawRootElement: E;
     rootElement: TestElement;
@@ -90,6 +97,13 @@ export declare type LocatorFnResult<T extends (HarnessQuery<any> | string)[]> =
     } ? C : T[I] extends string ? TestElement : never;
 }[number];
 
+export interface ModifierKeys {
+    alt?: boolean;
+    control?: boolean;
+    meta?: boolean;
+    shift?: boolean;
+}
+
 export interface TestElement {
     blur(): Promise<void>;
     clear(): Promise<void>;
diff --git a/tools/tslint-rules/noCrossEntryPointRelativeImportsRule.ts b/tools/tslint-rules/noCrossEntryPointRelativeImportsRule.ts
@@ -0,0 +1,78 @@
+import {existsSync} from 'fs';
+import * as minimatch from 'minimatch';
+import {dirname, join, normalize, relative, resolve} from 'path';
+import * as Lint from 'tslint';
+import * as ts from 'typescript';
+
+const BUILD_BAZEL_FILE = 'BUILD.bazel';
+
+/**
+ * Rule that enforces that imports or exports with relative paths do not resolve to
+ * source files outside of the current Bazel package. This enforcement is necessary
+ * because relative cross entry-point imports/exports can cause code being inlined
+ * unintentionally and could break module resolution since the folder structure
+ * changes in the Angular Package release output.
+ */
+export class Rule extends Lint.Rules.TypedRule {
+  applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
+    return this.applyWithFunction(sourceFile, checkSourceFile, this.getOptions().ruleArguments);
+  }
+}
+
+/**
+ * Rule walker function that checks the source file for imports/exports
+ * with relative cross entry-point references.
+ */
+function checkSourceFile(ctx: Lint.WalkContext<string[]>) {
+  const filePath = ctx.sourceFile.fileName;
+  const relativeFilePath = relative(process.cwd(), filePath);
+
+  if (!ctx.options.every(o => minimatch(relativeFilePath, o))) {
+    return;
+  }
+
+  const visitNode = (node: ts.Node) => {
+    if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) {
+      if (!node.moduleSpecifier || !ts.isStringLiteralLike(node.moduleSpecifier) ||
+          !node.moduleSpecifier.text.startsWith('.')) {
+        return;
+      }
+
+      const modulePath = node.moduleSpecifier.text;
+      const basePath = dirname(filePath);
+      const currentPackage = findClosestBazelPackage(basePath);
+      const resolvedPackage = findClosestBazelPackage(resolve(basePath, modulePath));
+
+      if (currentPackage && resolvedPackage &&
+          normalize(currentPackage) !== normalize(resolvedPackage)) {
+        const humanizedType = ts.isImportDeclaration(node) ? 'Import' : 'Export';
+        ctx.addFailureAtNode(
+            node,
+            `${humanizedType} resolves to a different Bazel build package through a relative ` +
+                `path. This is not allowed and can be fixed by using the actual module import.`);
+      }
+      return;
+    }
+    ts.forEachChild(node, visitNode);
+  };
+
+  ts.forEachChild(ctx.sourceFile, visitNode);
+}
+
+/** Finds the closest Bazel build package for the given path. */
+function findClosestBazelPackage(startPath: string): string|null {
+  let currentPath = startPath;
+  while (!hasBuildFile(currentPath)) {
+    const parentPath = dirname(currentPath);
+    if (parentPath === currentPath) {
+      return null;
+    }
+    currentPath = parentPath;
+  }
+  return currentPath;
+}
+
+/** Checks whether the given directory has a Bazel build file. */
+function hasBuildFile(dirPath: string): boolean {
+  return existsSync(join(dirPath, BUILD_BAZEL_FILE));
+}
diff --git a/tslint.json b/tslint.json
@@ -160,6 +160,13 @@
       true,
       "src/!(a11y-demo|e2e-app|components-examples|universal-app)/**/!(*.spec).ts"
     ],
+    "no-cross-entry-point-relative-imports": [
+      true,
+      "src/!(a11y-demo|e2e-app|components-examples|universal-app|dev-app)/**/!(*.spec).ts",
+      "!src/+(cdk|material)/schematics/**/*",
+      "!src/cdk/testing/+(private|tests)/**/*",
+      "!src/google-maps/testing/**/*"
+    ],
     "file-name-casing": [true, {
       // Exclude custom lint rule files since they have to always be camel-cased and end
       // with "Rule".

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,6 @@ ts_library(`
`11`	`11`	`module_name = "@angular/cdk/testing/protractor",`
`12`	`12`	`deps = [`
`13`	`13`	`"//src/cdk/testing",`
`14`		`- "//src/cdk/testing/private",`
`15`	`14`	`"@npm//protractor",`
`16`	`15`	`],`
`17`	`16`	`)`