Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cdk/testing/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
4 changes: 3 additions & 1 deletion src/cdk/testing/private/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
1 change: 0 additions & 1 deletion src/cdk/testing/protractor/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ ts_library(
module_name = "@angular/cdk/testing/protractor",
deps = [
"//src/cdk/testing",
"//src/cdk/testing/private",
"@npm//protractor",
],
)
Expand Down
4 changes: 1 addition & 3 deletions src/cdk/testing/protractor/protractor-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
4 changes: 1 addition & 3 deletions src/cdk/testing/protractor/protractor-harness-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
1 change: 1 addition & 0 deletions src/cdk/testing/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
export * from './component-harness';
export * from './harness-environment';
export * from './test-element';
export * from './element-dimensions';
9 changes: 8 additions & 1 deletion src/cdk/testing/test-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/cdk/testing/testbed/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ ts_library(
deps = [
"//src/cdk/keycodes",
"//src/cdk/testing",
"//src/cdk/testing/private",
"@npm//@angular/core",
"@npm//rxjs",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 7 additions & 3 deletions src/cdk/testing/testbed/testbed-harness-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
10 changes: 4 additions & 6 deletions src/cdk/testing/testbed/unit-test-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
2 changes: 1 addition & 1 deletion test/karma-system-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions tools/public_api_guard/cdk/testing.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>;
Expand Down
78 changes: 78 additions & 0 deletions tools/tslint-rules/noCrossEntryPointRelativeImportsRule.ts
Original file line number Diff line number Diff line change
@@ -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));
}
7 changes: 7 additions & 0 deletions tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand Down