Skip to content

Commit

Permalink
build: enable more strictness flags in non-library code (#23077)
Browse files Browse the repository at this point in the history
Updates the compiler options for the schematics and other Node-based code, and resolves any compilation errors that showed up as a result. Also fixes a bunch of references to non-existent types from `parse5`.

(cherry picked from commit 7b3006f)
  • Loading branch information
crisbeto authored and amysorto committed Jun 29, 2021
1 parent e14b93c commit 8a98236
Show file tree
Hide file tree
Showing 37 changed files with 104 additions and 82 deletions.
1 change: 1 addition & 0 deletions src/cdk/schematics/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ ts_library(
# TODO(devversion): Only include jasmine for test sources (See: tsconfig types).
"@npm//@types/jasmine",
"@npm//@types/glob",
"@npm//@types/parse5",
"@npm//@types/node",
"@npm//glob",
"@npm//parse5",
Expand Down
7 changes: 5 additions & 2 deletions src/cdk/schematics/ng-add/package-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ interface PackageJson {
* Sorts the keys of the given object.
* @returns A new object instance with sorted keys
*/
function sortObjectByKeys(obj: object) {
return Object.keys(obj).sort().reduce((result, key) => (result[key] = obj[key]) && result, {});
function sortObjectByKeys(obj: Record<string, string>) {
return Object.keys(obj).sort().reduce((result, key) => {
result[key] = obj[key];
return result;
}, {} as Record<string, string>);
}

/** Adds a package to the package.json in the given host tree. */
Expand Down
6 changes: 3 additions & 3 deletions src/cdk/schematics/ng-update/find-stylesheets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {join} from '@angular-devkit/core';
import {join, Path} from '@angular-devkit/core';
import {Tree} from '@angular-devkit/schematics';

/** Regular expression that matches stylesheet paths */
Expand All @@ -21,7 +21,7 @@ const STYLESHEET_REGEX = /.*\.(css|scss)/;
*/
export function findStylesheetFiles(tree: Tree, startDirectory: string = '/'): string[] {
const result: string[] = [];
const visitDir = dirPath => {
const visitDir = (dirPath: Path) => {
const {subfiles, subdirs} = tree.getDir(dirPath);

subfiles.forEach(fileName => {
Expand All @@ -38,6 +38,6 @@ export function findStylesheetFiles(tree: Tree, startDirectory: string = '/'): s
}
});
};
visitDir(startDirectory);
visitDir(startDirectory as Path);
return result;
}
14 changes: 7 additions & 7 deletions src/cdk/schematics/ng-update/html-parsing/elements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,23 @@
* found in the LICENSE file at https://angular.io/license
*/

import {DefaultTreeDocument, DefaultTreeElement, parseFragment} from 'parse5';
import {ChildNode, Element, parseFragment} from 'parse5';

/**
* Parses a HTML fragment and traverses all AST nodes in order find elements that
* include the specified attribute.
*/
export function findElementsWithAttribute(html: string, attributeName: string) {
const document = parseFragment(html, {sourceCodeLocationInfo: true}) as DefaultTreeDocument;
const elements: DefaultTreeElement[] = [];
const document = parseFragment(html, {sourceCodeLocationInfo: true});
const elements: Element[] = [];

const visitNodes = nodes => {
nodes.forEach(node => {
const visitNodes = (nodes: ChildNode[]) => {
nodes.forEach((node: Element) => {
if (node.childNodes) {
visitNodes(node.childNodes);
}

if (node.attrs && node.attrs.some(attr => attr.name === attributeName.toLowerCase())) {
if (node.attrs?.some(attr => attr.name === attributeName.toLowerCase())) {
elements.push(node);
}
});
Expand Down Expand Up @@ -54,7 +54,7 @@ export function findAttributeOnElementWithAttrs(html: string, name: string, attr
}

/** Shorthand function that checks if the specified element contains the given attribute. */
function hasElementAttribute(element: DefaultTreeElement, attributeName: string): boolean {
function hasElementAttribute(element: Element, attributeName: string): boolean {
return element.attrs && element.attrs.some(attr => attr.name === attributeName.toLowerCase());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,23 @@ export class AttributeSelectorsMigration extends Migration<UpgradeData> {
data = getVersionUpgradeData(this, 'attributeSelectors');

// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;
enabled: boolean = this.data.length !== 0;

visitNode(node: ts.Node) {
override visitNode(node: ts.Node) {
if (ts.isStringLiteralLike(node)) {
this._visitStringLiteralLike(node);
}
}

visitTemplate(template: ResolvedResource) {
override visitTemplate(template: ResolvedResource) {
this.data.forEach(selector => {
findAllSubstringIndices(template.content, selector.replace)
.map(offset => template.start + offset)
.forEach(start => this._replaceSelector(template.filePath, start, selector));
});
}

visitStylesheet(stylesheet: ResolvedResource): void {
override visitStylesheet(stylesheet: ResolvedResource): void {
this.data.forEach(selector => {
const currentSelector = `[${selector.replace}]`;
const updatedSelector = `[${selector.replaceWith}]`;
Expand Down
4 changes: 2 additions & 2 deletions src/cdk/schematics/ng-update/migrations/class-inheritance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ export class ClassInheritanceMigration extends Migration<UpgradeData> {
// Only enable the migration rule if there is upgrade data.
enabled = this.propertyNames.size !== 0;

init(): void {
override init(): void {
getVersionUpgradeData(this, 'propertyNames')
.filter(data => data.limitedTo && data.limitedTo.classes)
.forEach(
data => data.limitedTo.classes.forEach(name => this.propertyNames.set(name, data)));
}

visitNode(node: ts.Node): void {
override visitNode(node: ts.Node): void {
if (ts.isClassDeclaration(node)) {
this._visitClassDeclaration(node);
}
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/schematics/ng-update/migrations/class-names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class ClassNamesMigration extends Migration<UpgradeData> {
// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;

visitNode(node: ts.Node): void {
override visitNode(node: ts.Node): void {
if (ts.isIdentifier(node)) {
this._visitIdentifier(node);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class ConstructorSignatureMigration extends Migration<UpgradeData> {
// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;

visitNode(node: ts.Node): void {
override visitNode(node: ts.Node): void {
if (ts.isSourceFile(node)) {
this._visitSourceFile(node);
}
Expand Down
6 changes: 3 additions & 3 deletions src/cdk/schematics/ng-update/migrations/css-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ export class CssSelectorsMigration extends Migration<UpgradeData> {
// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;

visitNode(node: ts.Node): void {
override visitNode(node: ts.Node): void {
if (ts.isStringLiteralLike(node)) {
this._visitStringLiteralLike(node);
}
}

visitTemplate(template: ResolvedResource): void {
override visitTemplate(template: ResolvedResource): void {
this.data.forEach(data => {
if (data.replaceIn && !data.replaceIn.html) {
return;
Expand All @@ -43,7 +43,7 @@ export class CssSelectorsMigration extends Migration<UpgradeData> {
});
}

visitStylesheet(stylesheet: ResolvedResource): void {
override visitStylesheet(stylesheet: ResolvedResource): void {
this.data.forEach(data => {
if (data.replaceIn && !data.replaceIn.stylesheet) {
return;
Expand Down
8 changes: 4 additions & 4 deletions src/cdk/schematics/ng-update/migrations/element-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,23 @@ export class ElementSelectorsMigration extends Migration<UpgradeData> {
data = getVersionUpgradeData(this, 'elementSelectors');

// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;
enabled: boolean = this.data.length !== 0;

visitNode(node: ts.Node): void {
override visitNode(node: ts.Node): void {
if (ts.isStringLiteralLike(node)) {
this._visitStringLiteralLike(node);
}
}

visitTemplate(template: ResolvedResource): void {
override visitTemplate(template: ResolvedResource): void {
this.data.forEach(selector => {
findAllSubstringIndices(template.content, selector.replace)
.map(offset => template.start + offset)
.forEach(start => this._replaceSelector(template.filePath, start, selector));
});
}

visitStylesheet(stylesheet: ResolvedResource): void {
override visitStylesheet(stylesheet: ResolvedResource): void {
this.data.forEach(selector => {
findAllSubstringIndices(stylesheet.content, selector.replace)
.map(offset => stylesheet.start + offset)
Expand Down
4 changes: 2 additions & 2 deletions src/cdk/schematics/ng-update/migrations/input-names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class InputNamesMigration extends Migration<UpgradeData> {
// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;

visitStylesheet(stylesheet: ResolvedResource): void {
override visitStylesheet(stylesheet: ResolvedResource): void {
this.data.forEach(name => {
const currentSelector = `[${name.replace}]`;
const updatedSelector = `[${name.replaceWith}]`;
Expand All @@ -42,7 +42,7 @@ export class InputNamesMigration extends Migration<UpgradeData> {
});
}

visitTemplate(template: ResolvedResource): void {
override visitTemplate(template: ResolvedResource): void {
this.data.forEach(name => {
const limitedTo = name.limitedTo;
const relativeOffsets: number[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class MethodCallArgumentsMigration extends Migration<UpgradeData> {
// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;

visitNode(node: ts.Node): void {
override visitNode(node: ts.Node): void {
if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression)) {
this._checkPropertyAccessMethodCall(node);
}
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/schematics/ng-update/migrations/misc-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class MiscTemplateMigration extends Migration<UpgradeData> {
// currently only includes migrations for V6 deprecations.
enabled = this.targetVersion === TargetVersion.V6;

visitTemplate(template: ResolvedResource): void {
override visitTemplate(template: ResolvedResource): void {
// Migration for https://github.com/angular/components/pull/10325 (v6)
findAllSubstringIndices(template.content, 'cdk-focus-trap').forEach(offset => {
this.failures.push({
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/schematics/ng-update/migrations/output-names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class OutputNamesMigration extends Migration<UpgradeData> {
// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;

visitTemplate(template: ResolvedResource): void {
override visitTemplate(template: ResolvedResource): void {
this.data.forEach(name => {
const limitedTo = name.limitedTo;
const relativeOffsets: number[] = [];
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/schematics/ng-update/migrations/property-names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class PropertyNamesMigration extends Migration<UpgradeData> {
// Only enable the migration rule if there is upgrade data.
enabled = this.data.length !== 0;

visitNode(node: ts.Node): void {
override visitNode(node: ts.Node): void {
if (ts.isPropertyAccessExpression(node)) {
this._visitPropertyAccessExpression(node);
}
Expand Down
5 changes: 5 additions & 0 deletions src/cdk/schematics/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@
"outDir": "../../../dist/packages/cdk/schematics",
"noEmitOnError": false,
"strictNullChecks": true,
"noImplicitOverride": true,
"noImplicitReturns": true,
"noImplicitAny": true,
"skipDefaultLibCheck": true,
"noFallthroughCasesInSwitch": true,
"noUnusedLocals": false,
"noImplicitThis": true,
"skipLibCheck": true,
"sourceMap": true,
"target": "es2015",
Expand Down
10 changes: 7 additions & 3 deletions src/cdk/schematics/utils/build-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,12 @@ export function buildComponent(options: ComponentOptions,
// Add the default component option values to the options if an option is not explicitly
// specified but a default component option is available.
Object.keys(options)
.filter(optionName => options[optionName] == null && defaultComponentOptions[optionName])
.forEach(optionName => options[optionName] = defaultComponentOptions[optionName]);
.filter((optionName: keyof ComponentOptions) => {
return options[optionName] == null && defaultComponentOptions[optionName];
})
.forEach((optionName: keyof ComponentOptions) => {
(options as any)[optionName] = (defaultComponentOptions as ComponentOptions)[optionName];
});

if (options.path === undefined) {
// TODO(jelbourn): figure out if the need for this `as any` is a bug due to two different
Expand Down Expand Up @@ -214,7 +218,7 @@ export function buildComponent(options: ComponentOptions,

// Key-value object that includes the specified additional files with their loaded content.
// The resolved contents can be used inside EJS templates.
const resolvedFiles = {};
const resolvedFiles: Record<string, string> = {};

for (let key in additionalFiles) {
if (additionalFiles[key]) {
Expand Down
10 changes: 5 additions & 5 deletions src/cdk/schematics/utils/html-manipulation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {SchematicsException, Tree} from '@angular-devkit/schematics';
import {getChildElementIndentation} from './parse5-element';
import {DefaultTreeDocument, DefaultTreeElement, parse as parseHtml} from 'parse5';
import {Element, parse as parseHtml} from 'parse5';

/** Appends the given element HTML fragment to the `<head>` element of the specified HTML file. */
export function appendHtmlElementToHead(host: Tree, htmlFilePath: string, elementHtml: string) {
Expand Down Expand Up @@ -44,7 +44,7 @@ export function appendHtmlElementToHead(host: Tree, htmlFilePath: string, elemen
}

/** Parses the given HTML file and returns the head element if available. */
export function getHtmlHeadTagElement(htmlContent: string): DefaultTreeElement | null {
export function getHtmlHeadTagElement(htmlContent: string): Element | null {
return getElementByTagName('head', htmlContent);
}

Expand Down Expand Up @@ -85,12 +85,12 @@ export function addBodyClass(host: Tree, htmlFilePath: string, className: string

/** Finds an element by its tag name. */
function getElementByTagName(tagName: string, htmlContent: string):
DefaultTreeElement | null {
const document = parseHtml(htmlContent, {sourceCodeLocationInfo: true}) as DefaultTreeDocument;
Element | null {
const document = parseHtml(htmlContent, {sourceCodeLocationInfo: true});
const nodeQueue = [...document.childNodes];

while (nodeQueue.length) {
const node = nodeQueue.shift() as DefaultTreeElement;
const node = nodeQueue.shift() as Element;

if (node.nodeName.toLowerCase() === tagName) {
return node;
Expand Down
6 changes: 3 additions & 3 deletions src/cdk/schematics/utils/parse5-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
*/

import {SchematicsException} from '@angular-devkit/schematics';
import {DefaultTreeElement} from 'parse5';
import {Element} from 'parse5';

/** Determines the indentation of child elements for the given Parse5 element. */
export function getChildElementIndentation(element: DefaultTreeElement) {
export function getChildElementIndentation(element: Element) {
const childElement = element.childNodes
.find(node => node['tagName']) as DefaultTreeElement | null;
.find(node => (node as Element).tagName) as Element | null;

if ((childElement && !childElement.sourceCodeLocation) || !element.sourceCodeLocation) {
throw new SchematicsException('Cannot determine child element indentation because the ' +
Expand Down
5 changes: 3 additions & 2 deletions src/cdk/schematics/utils/schematic-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import {ProjectDefinition} from '@angular-devkit/core/src/workspace';
import {isJsonObject, JsonObject} from '@angular-devkit/core';
import {Schema, Style} from '@schematics/angular/component/schema';

/**
* Returns the default options for the `@schematics/angular:component` schematic which would
Expand All @@ -16,7 +17,7 @@ import {isJsonObject, JsonObject} from '@angular-devkit/core';
* This is necessary because the Angular CLI only exposes the default values for the "--style",
* "--inlineStyle", "--skipTests" and "--inlineTemplate" options to the "component" schematic.
*/
export function getDefaultComponentOptions(project: ProjectDefinition) {
export function getDefaultComponentOptions(project: ProjectDefinition): Partial<Schema> {
// Note: Not all options which are available when running "ng new" will be stored in the
// workspace config. List of options which will be available in the configuration:
// angular/angular-cli/blob/master/packages/schematics/angular/application/index.ts#L109-L131
Expand All @@ -30,7 +31,7 @@ export function getDefaultComponentOptions(project: ProjectDefinition) {
}

return {
style: getDefaultComponentOption(project, ['style', 'styleext'], 'css'),
style: getDefaultComponentOption<Style>(project, ['style', 'styleext'], Style.Css),
inlineStyle: getDefaultComponentOption(project, ['inlineStyle'], false),
inlineTemplate: getDefaultComponentOption(project, ['inlineTemplate'], false),
skipTests: skipTests,
Expand Down
Loading

0 comments on commit 8a98236

Please sign in to comment.