refactor(schematics): enable strict null checks (#12983)

* Enables strict null checks for the schematics and fixes a few potential unsafe accesses.
* Supports specifying a project for `ng-add`.
This commit is contained in:
Paul Gschwendtner
2018-09-06 22:53:21 +02:00
committed by Jeremy Elbourn
parent e124418d07
commit 098f25a2fd
31 changed files with 123 additions and 78 deletions
+1 -1
View File
@@ -30,7 +30,7 @@ export default function(options: Schema): Rule {
*/
function addFormModulesToModule(options: Schema) {
return (host: Tree) => {
const modulePath = findModuleFromOptions(host, options);
const modulePath = findModuleFromOptions(host, options)!;
addModuleImportToModule(host, modulePath, 'MatInputModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatButtonModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatSelectModule', '@angular/material');
+1 -1
View File
@@ -30,7 +30,7 @@ export default function(options: Schema): Rule {
*/
function addNavModulesToModule(options: Schema) {
return (host: Tree) => {
const modulePath = findModuleFromOptions(host, options);
const modulePath = findModuleFromOptions(host, options)!;
addModuleImportToModule(host, modulePath, 'MatGridListModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatCardModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatMenuModule', '@angular/material');
@@ -33,7 +33,9 @@ export function appendElementToHead(host: Tree, project: WorkspaceProject, eleme
throw `Could not find '<head>' element in HTML file: ${indexPath}`;
}
const endTagOffset = headTag.sourceCodeLocation.endTag.startOffset;
// We always have access to the source code location here because the `getHeadTagElement`
// function explicitly has the `sourceCodeLocationInfo` option enabled.
const endTagOffset = headTag.sourceCodeLocation!.endTag.startOffset;
const indentationOffset = getChildElementIndentation(headTag);
const insertion = `${' '.repeat(indentationOffset)}${elementHtml}`;
@@ -8,14 +8,15 @@
import {SchematicsException} from '@angular-devkit/schematics';
import {WorkspaceProject} from '@schematics/angular/utility/config';
import {getArchitectOptions} from '../../utils/architect-options';
/** Looks for the index HTML file in the given project and returns its path. */
export function getIndexHtmlPath(project: WorkspaceProject): string {
const buildTarget = project.architect.build.options;
const buildOptions = getArchitectOptions(project, 'build');
if (buildTarget.index && buildTarget.index.endsWith('index.html')) {
return buildTarget.index;
if (!buildOptions.index) {
throw new SchematicsException('No project "index.html" file could be found.');
}
throw new SchematicsException('No index.html file was found.');
return buildOptions.index;
}
@@ -10,7 +10,7 @@ import {Rule, Tree} from '@angular-devkit/schematics';
import {getWorkspace} from '@schematics/angular/utility/config';
import {getProjectFromWorkspace} from '../../utils/get-project';
import {Schema} from '../schema';
import {getProjectMainFile} from './project-main-file';
import {getProjectMainFile} from '../../utils/project-main-file';
const hammerjsImportStatement = `import 'hammerjs';`;
+5 -5
View File
@@ -1,6 +1,6 @@
import {Tree} from '@angular-devkit/schematics';
import {SchematicTestRunner} from '@angular-devkit/schematics/testing';
import {getProjectStyleFile} from '@angular/material/schematics/utils/project-style-file';
import {getProjectStyleFile} from '../utils/project-style-file';
import {getIndexHtmlPath} from './fonts/project-index-html';
import {getProjectFromWorkspace} from '../utils/get-project';
import {getFileContent} from '@schematics/angular/utility/test';
@@ -71,10 +71,10 @@ describe('material-install-schematic', () => {
const expectedStylesPath = normalize(`/${project.root}/src/styles.scss`);
const buffer = tree.read(expectedStylesPath);
const src = buffer!.toString();
const themeContent = buffer!.toString();
expect(src.indexOf(`@import '~@angular/material/theming';`)).toBeGreaterThan(-1);
expect(src.indexOf(`$app-primary`)).toBeGreaterThan(-1);
expect(themeContent).toContain(`@import '~@angular/material/theming';`);
expect(themeContent).toContain(`$app-primary: mat-palette(`);
});
it('should create a custom theme file if no SCSS file could be found', () => {
@@ -112,7 +112,7 @@ describe('material-install-schematic', () => {
const workspace = getWorkspace(tree);
const project = getProjectFromWorkspace(workspace);
const defaultStylesPath = getProjectStyleFile(project);
const defaultStylesPath = getProjectStyleFile(project)!;
const htmlContent = tree.read(defaultStylesPath)!.toString();
expect(htmlContent).toContain('html, body { height: 100%; }');
+2 -2
View File
@@ -94,9 +94,9 @@ function addMaterialAppStyles(options: Schema) {
const workspace = getWorkspace(host);
const project = getProjectFromWorkspace(workspace, options.project);
const styleFilePath = getProjectStyleFile(project);
const buffer = host.read(styleFilePath);
const buffer = host.read(styleFilePath!);
if (!buffer) {
if (!styleFilePath || !buffer) {
return console.warn(`Could not find styles file: "${styleFilePath}". Skipping styles ` +
`generation. Please consider manually adding the "Roboto" font and resetting the ` +
`body margin.`);
+7
View File
@@ -4,6 +4,13 @@
"title": "Material Install Options Schema",
"type": "object",
"properties": {
"project": {
"type": "string",
"description": "The name of the project.",
"$default": {
"$source": "projectName"
}
},
"skipPackageJson": {
"type": "boolean",
"default": false,
+4 -3
View File
@@ -7,6 +7,10 @@
*/
export interface Schema {
/** Name of the project to target. */
project: string;
/** Whether to skip package.json install. */
skipPackageJson: boolean;
@@ -15,7 +19,4 @@ export interface Schema {
/** Name of pre-built theme to install. */
theme: 'indigo-pink' | 'deeppurple-amber' | 'pink-bluegrey' | 'purple-green' | 'custom';
/** Name of the project to target. */
project?: string;
}
+15 -10
View File
@@ -31,7 +31,7 @@ export function addThemeToAppStyles(options: Schema): (host: Tree) => Tree {
if (themeName === 'custom') {
insertCustomTheme(project, options.project, host, workspace);
} else {
insertPrebuiltTheme(project, host, themeName, workspace, options.project);
insertPrebuiltTheme(project, host, themeName, workspace);
}
return host;
@@ -49,13 +49,20 @@ function insertCustomTheme(project: WorkspaceProject, projectName: string, host:
const themeContent = createCustomTheme(projectName);
if (!stylesPath) {
if (!project.sourceRoot) {
throw new Error(`Could not find source root for project: "${projectName}". Please make ` +
`sure that the "sourceRoot" property is set in the workspace config.`);
}
// Normalize the path through the devkit utilities because we want to avoid having
// unnecessary path segments and windows backslash delimiters.
const customThemePath = normalize(join(project.sourceRoot, 'custom-theme.scss'));
host.create(customThemePath, themeContent);
addStyleToTarget(project.architect['build'], host, customThemePath, workspace);
return;
// Architect is always defined because we initially asserted if the default builder
// configuration is set up or not.
return addStyleToTarget(project.architect!['build'], host, customThemePath, workspace);
}
const insertion = new InsertChange(stylesPath, 0, themeContent);
@@ -67,17 +74,15 @@ function insertCustomTheme(project: WorkspaceProject, projectName: string, host:
/** Insert a pre-built theme into the angular.json file. */
function insertPrebuiltTheme(project: WorkspaceProject, host: Tree, theme: string,
workspace: WorkspaceSchema, projectName: string) {
workspace: WorkspaceSchema) {
// Path needs to be always relative to the `package.json` or workspace root.
const themePath = `./node_modules/@angular/material/prebuilt-themes/${theme}.css`;
if (project.architect) {
addStyleToTarget(project.architect['build'], host, themePath, workspace);
addStyleToTarget(project.architect['test'], host, themePath, workspace);
} else {
throw new SchematicsException(`${projectName} does not have an architect configuration`);
}
// Architect is always defined because we initially asserted if the default builder
// configuration is set up or not.
addStyleToTarget(project.architect!['build'], host, themePath, workspace);
addStyleToTarget(project.architect!['test'], host, themePath, workspace);
}
/** Adds a style entry to the given target. */
+1 -1
View File
@@ -30,7 +30,7 @@ export default function(options: Schema): Rule {
*/
function addNavModulesToModule(options: Schema) {
return (host: Tree) => {
const modulePath = findModuleFromOptions(host, options);
const modulePath = findModuleFromOptions(host, options)!;
addModuleImportToModule(host, modulePath, 'LayoutModule', '@angular/cdk/layout');
addModuleImportToModule(host, modulePath, 'MatToolbarModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatButtonModule', '@angular/material');
+1 -1
View File
@@ -30,7 +30,7 @@ export default function(options: Schema): Rule {
*/
function addTableModulesToModule(options: Schema) {
return (host: Tree) => {
const modulePath = findModuleFromOptions(host, options);
const modulePath = findModuleFromOptions(host, options)!;
addModuleImportToModule(host, modulePath, 'MatTableModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatPaginatorModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatSortModule', '@angular/material');
+1 -1
View File
@@ -30,7 +30,7 @@ export default function(options: Schema): Rule {
*/
function addTreeModulesToModule(options: Schema) {
return (host: Tree) => {
const modulePath = findModuleFromOptions(host, options);
const modulePath = findModuleFromOptions(host, options)!;
addModuleImportToModule(host, modulePath, 'MatTreeModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatIconModule', '@angular/material');
addModuleImportToModule(host, modulePath, 'MatButtonModule', '@angular/material');
+1
View File
@@ -5,6 +5,7 @@
"moduleResolution": "node",
"outDir": "../../../dist/schematics",
"noEmitOnError": false,
"strictNullChecks": true,
"skipDefaultLibCheck": true,
"skipLibCheck": true,
"sourceMap": true,
@@ -15,17 +15,11 @@ export interface MaterialInputNameData {
/** The new name for the @Input(). */
replaceWith: string;
/** Whitelist where this replacement is made. If omitted it is made in all HTML & CSS */
whitelist?: {
whitelist: {
/** Limit to elements with any of these element tags. */
elements?: string[],
/** Limit to elements with any of these attributes. */
attributes?: string[],
/**
* Whether inputs in stylesheets should be updated or not. Note that inputs inside of
* stylesheets usually don't make sense, but if developers use an input as a plain one-time
* attribute, it can be targeted through CSS selectors.
*/
stylesheet?: boolean,
};
}
@@ -15,7 +15,7 @@ export interface MaterialOutputNameData {
/** The new name for the @Output(). */
replaceWith: string;
/** Whitelist where this replacement is made. If omitted it is made in all HTML & CSS */
whitelist?: {
whitelist: {
/** Limit to elements with any of these element tags. */
elements?: string[],
/** Limit to elements with any of these attributes. */
@@ -17,7 +17,7 @@ export interface MaterialPropertyNameData {
/** Whitelist where this replacement is made. If omitted it is made for all Classes. */
whitelist: {
/** Replace the property only when its type is one of the given Classes. */
classes?: string[];
classes: string[];
};
}
@@ -33,5 +33,5 @@ export function getChangesForTarget<T>(target: TargetVersion, data: VersionChang
return [];
}
return data[target].reduce((result, changes) => result.concat(changes.changes), []);
return data[target]!.reduce((result, prData) => result.concat(prData.changes), [] as T[]);
}
@@ -41,6 +41,7 @@ export class Walker extends ProgramAwareRuleWalker {
visitClassDeclaration(node: ts.ClassDeclaration) {
const baseTypes = determineBaseTypes(node);
const className = node.name ? node.name.text : '{unknown-name}';
if (!baseTypes) {
return;
@@ -50,7 +51,7 @@ export class Walker extends ProgramAwareRuleWalker {
const data = this.propertyNames.get(typeName);
if (data) {
this.addFailureAtNode(node, `Found class "${bold(node.name.text)}" which extends class ` +
this.addFailureAtNode(node, `Found class "${bold(className)}" which extends class ` +
`"${bold(typeName)}". Please note that the base class property ` +
`"${red(data.replace)}" has changed to "${green(data.replaceWith)}". ` +
`You may need to update your class as well`);
@@ -25,6 +25,7 @@ export class Walker extends ProgramAwareRuleWalker {
visitClassDeclaration(node: ts.ClassDeclaration) {
const baseTypes = determineBaseTypes(node);
const className = node.name ? node.name.text : '{unknown-name}';
if (!baseTypes) {
return;
@@ -32,10 +33,11 @@ export class Walker extends ProgramAwareRuleWalker {
if (baseTypes.includes('MatFormFieldControl')) {
const hasFloatLabelMember = node.members
.find(member => member.name && member.name.getText() === 'shouldFloatLabel');
.filter(member => member.name)
.find(member => member.name!.getText() === 'shouldFloatLabel');
if (!hasFloatLabelMember) {
this.addFailureAtNode(node, `Found class "${bold(node.name.text)}" which extends ` +
this.addFailureAtNode(node, `Found class "${bold(className)}" which extends ` +
`"${bold('MatFormFieldControl')}". This class must define ` +
`"${green('shouldLabelFloat')}" which is now a required property.`);
}
@@ -68,10 +68,6 @@ export class Walker extends ComponentWalker {
const replacements: {failureMessage: string, replacement: Replacement}[] = [];
this.data.forEach(name => {
if (name.whitelist && !name.whitelist.stylesheet) {
return;
}
const currentSelector = `[${name.replace}]`;
const updatedSelector = `[${name.replaceWith}]`;
@@ -55,16 +55,16 @@ export class Walker extends ComponentWalker {
this.data.forEach(name => {
const whitelist = name.whitelist;
const relativeOffsets = [];
const relativeOffsets: number[] = [];
const failureMessage = `Found deprecated @Input() "${red(name.replace)}"` +
` which has been renamed to "${green(name.replaceWith)}"`;
if (!whitelist || whitelist.attributes) {
if (whitelist.attributes) {
relativeOffsets.push(
...findInputsOnElementWithAttr(templateContent, name.replace, whitelist.attributes));
}
if (!whitelist || whitelist.elements) {
if (whitelist.elements) {
relativeOffsets.push(
...findInputsOnElementWithTag(templateContent, name.replace, whitelist.elements));
}
@@ -55,16 +55,16 @@ export class Walker extends ComponentWalker {
this.data.forEach(name => {
const whitelist = name.whitelist;
const relativeOffsets = [];
const relativeOffsets: number[] = [];
const failureMessage = `Found deprecated @Output() "${red(name.replace)}"` +
` which has been renamed to "${green(name.replaceWith)}"`;
if (!whitelist || whitelist.attributes) {
if (whitelist.attributes) {
relativeOffsets.push(
...findOutputsOnElementWithAttr(templateContent, name.replace, whitelist.attributes));
}
if (!whitelist || whitelist.elements) {
if (whitelist.elements) {
relativeOffsets.push(
...findOutputsOnElementWithTag(templateContent, name.replace, whitelist.elements));
}
@@ -40,7 +40,8 @@ export class Walker extends ProgramAwareRuleWalker {
return signature.getParameters()
.map(param => param.declarations[0] as ts.ParameterDeclaration)
.map(node => node.type)
.map(node => this.getTypeChecker().getTypeFromTypeNode(node));
// TODO(devversion): handle non resolvable constructor types
.map(typeNode => this.getTypeChecker().getTypeFromTypeNode(typeNode!));
}
private checkExpressionSignature(node: ts.CallExpression | ts.NewExpression) {
@@ -50,7 +51,7 @@ export class Walker extends ProgramAwareRuleWalker {
// TODO(devversion): Consider handling pass-through classes better.
// TODO(devversion): e.g. `export class CustomCalendar extends MatCalendar {}`
if (!classType || !constructorChecks.includes(className)) {
if (!classType || !constructorChecks.includes(className) || !node.arguments) {
return;
}
@@ -62,6 +63,8 @@ export class Walker extends ProgramAwareRuleWalker {
// TODO(devversion): we should check if the type is assignable to the signature
// TODO(devversion): blocked on https://github.com/Microsoft/TypeScript/issues/9879
const doesMatchSignature = classSignatures.some(signature => {
// TODO(devversion): better handling if signature item type is unresolved but assignable
// to everything.
return signature.every((type, index) => callExpressionSignature[index] === type) &&
signature.length === callExpressionSignature.length;
});
@@ -15,8 +15,8 @@ export function determineBaseTypes(node: ts.ClassDeclaration): string[] | null {
}
return node.heritageClauses
.reduce((types, clause) => types.concat(clause.types), [])
.reduce((types, clause) => types.concat(clause.types), [] as ts.ExpressionWithTypeArguments[])
.map(typeExpression => typeExpression.expression)
.filter(expression => expression && ts.isIdentifier(expression))
.map(identifier => identifier.text);
.map((identifier: ts.Identifier) => identifier.text);
}
@@ -0,0 +1,21 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {WorkspaceProject} from '@schematics/angular/utility/config';
/** Resolves the architect options for the build target of the given project. */
export function getArchitectOptions(project: WorkspaceProject, buildTarget: string) {
if (project.architect &&
project.architect[buildTarget] &&
project.architect[buildTarget].options) {
return project.architect[buildTarget].options;
}
throw new Error(`Cannot determine architect configuration for target: ${buildTarget}.`);
}
+8 -5
View File
@@ -7,12 +7,14 @@
*/
import {SchematicsException, Tree} from '@angular-devkit/schematics';
import {Schema as ComponentOptions} from '@schematics/angular/component/schema';
import {addImportToModule} from '@schematics/angular/utility/ast-utils';
import {InsertChange} from '@schematics/angular/utility/change';
import {getWorkspace, WorkspaceProject} from '@schematics/angular/utility/config';
import {findModuleFromOptions as internalFindModule} from '@schematics/angular/utility/find-module';
import {getAppModulePath} from '@schematics/angular/utility/ng-ast-utils';
import * as ts from 'typescript';
import {getProjectMainFile} from './project-main-file';
/** Reads file given path and returns TypeScript source file. */
@@ -28,8 +30,7 @@ export function getSourceFile(host: Tree, path: string): ts.SourceFile {
/** Import and add module to root app module. */
export function addModuleImportToRootModule(host: Tree, moduleName: string, src: string,
project: WorkspaceProject) {
const modulePath = getAppModulePath(host, project.architect.build.options.main);
const modulePath = getAppModulePath(host, getProjectMainFile(project));
addModuleImportToModule(host, modulePath, moduleName, src);
}
@@ -40,8 +41,9 @@ export function addModuleImportToRootModule(host: Tree, moduleName: string, src:
* @param moduleName name of module to import
* @param src src location to import
*/
export function addModuleImportToModule(
host: Tree, modulePath: string, moduleName: string, src: string) {
export function addModuleImportToModule(host: Tree, modulePath: string, moduleName: string,
src: string) {
const moduleSource = getSourceFile(host, modulePath);
if (!moduleSource) {
@@ -63,8 +65,9 @@ export function addModuleImportToModule(
}
/** Wraps the internal find module from options with undefined path handling */
export function findModuleFromOptions(host: Tree, options: any) {
export function findModuleFromOptions(host: Tree, options: ComponentOptions): string | undefined {
const workspace = getWorkspace(host);
if (!options.project) {
options.project = Object.keys(workspace.projects)[0];
}
+6 -5
View File
@@ -39,6 +39,7 @@ import {validateHtmlSelector, validateName} from '@schematics/angular/utility/va
import {readFileSync} from 'fs';
import {dirname, join, resolve} from 'path';
import * as ts from 'typescript';
import {getProjectFromWorkspace} from './get-project';
import {getDefaultComponentOptions} from './schematic-options';
function readIntoSourceFile(host: Tree, modulePath: string): ts.SourceFile {
@@ -162,7 +163,7 @@ export function buildComponent(options: ComponentOptions,
return (host: Tree, context: FileSystemSchematicContext) => {
const workspace = getWorkspace(host);
const project = workspace.projects[options.project || workspace.defaultProject];
const project = getProjectFromWorkspace(workspace, options.project);
const defaultComponentOptions = getDefaultComponentOptions(project);
const schematicFilesUrl = './files';
@@ -216,8 +217,10 @@ export function buildComponent(options: ComponentOptions,
options.inlineTemplate ? filter(path => !path.endsWith('.html')) : noop(),
// Treat the template options as any, because the type definition for the template options
// is made unnecessarily explicit. Every type of object can be used in the EJS template.
template({ indentTextContent, resolvedFiles, ...baseTemplateContext} as any),
move(null, parsedPath.path),
template({indentTextContent, resolvedFiles, ...baseTemplateContext} as any),
// TODO(devversion): figure out why we cannot just remove the first parameter
// See for example: angular-cli#schematics/angular/component/index.ts#L160
move(null as any, parsedPath.path),
]);
return chain([
@@ -228,5 +231,3 @@ export function buildComponent(options: ComponentOptions,
])(host, context);
};
}
// TODO(paul): move this utility out of the `devkit-utils` because it's no longer taken from there.
+7 -2
View File
@@ -13,13 +13,18 @@ export function getChildElementIndentation(element: DefaultTreeElement) {
const childElement = element.childNodes
.find(node => node['tagName']) as DefaultTreeElement | null;
if ((childElement && !childElement.sourceCodeLocation) || !element.sourceCodeLocation) {
throw new Error('Cannot determine child element indentation because the specified Parse5 ' +
'element does not have any source code location metadata.');
}
const startColumns = childElement ?
// In case there are child elements inside of the element, we assume that their
// indentation is also applicable for other child elements.
childElement.sourceCodeLocation.startCol :
childElement.sourceCodeLocation!.startCol :
// In case there is no child element, we just assume that child elements should be indented
// by two spaces.
element.sourceCodeLocation.startCol + 2;
element.sourceCodeLocation!.startCol + 2;
// Since Parse5 does not set the `startCol` properties as zero-based, we need to subtract
// one column in order to have a proper zero-based offset for the indentation.
@@ -8,15 +8,16 @@
import {SchematicsException} from '@angular-devkit/schematics';
import {WorkspaceProject} from '@schematics/angular/utility/config';
import {getArchitectOptions} from './architect-options';
/** Looks for the main TypeScript file in the given project and returns its path. */
export function getProjectMainFile(project: WorkspaceProject): string {
const buildTarget = project.architect.build.options;
const buildOptions = getArchitectOptions(project, 'build');
if (buildTarget.main) {
return buildTarget.main;
if (!buildOptions.main) {
throw new SchematicsException(`Could not find the project main file inside of the ` +
`workspace config (${project.sourceRoot})`);
}
throw new SchematicsException(
'Could not find the project main file inside of the workspace config.');
return buildOptions.main;
}
@@ -8,6 +8,7 @@
import {normalize} from '@angular-devkit/core';
import {WorkspaceProject} from '@schematics/angular/utility/config';
import {getArchitectOptions} from './architect-options';
/** Regular expression that matches all possible Angular CLI default style files. */
const defaultStyleFileRegex = /styles\.(c|le|sc)ss/;
@@ -20,10 +21,10 @@ const validStyleFileRegex = /\.(c|le|sc)ss/;
* extension is specified, any style file with a valid extension will be returned.
*/
export function getProjectStyleFile(project: WorkspaceProject, extension?: string): string | null {
const buildTarget = project.architect['build'];
const buildOptions = getArchitectOptions(project, 'build');
if (buildTarget.options && buildTarget.options.styles && buildTarget.options.styles.length) {
const styles = buildTarget.options.styles.map(s => typeof s === 'string' ? s : s.input);
if (buildOptions.styles && buildOptions.styles.length) {
const styles = buildOptions.styles.map(s => typeof s === 'string' ? s : s.input);
// Look for the default style file that is generated for new projects by the Angular CLI. This
// default style file is usually called `styles.ext` unless it has been changed explicitly.