Skip to content

build: enable more strictness flags in non-library code #23077

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 1 commit into from
Jun 29, 2021
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
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this is needed. Seems like something TS could infer quite easily?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a weird one, but for some reason TS was flagging the entire class if the type isn't specified explicitly. It was something about not being able to infer the type from the constructor.


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