Skip to content
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

Show warning on deprecated option usage #18520

Merged
merged 5 commits into from
Aug 17, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions etc/api/angular_devkit/core/src/_golden-api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ export declare class CoreSchemaRegistry implements SchemaRegistry {
flatten(schema: JsonObject): Observable<JsonObject>;
registerUriHandler(handler: UriHandler): void;
usePromptProvider(provider: PromptProvider): void;
useXDeprecatedProvider(onUsage: (message: string) => void): void;
}

export declare function createWorkspaceHost(host: virtualFs.Host): WorkspaceHost;
Expand Down Expand Up @@ -904,6 +905,7 @@ export interface SchemaRegistry {
compile(schema: Object): Observable<SchemaValidator>;
flatten(schema: JsonObject | string): Observable<JsonObject>;
usePromptProvider(provider: PromptProvider): void;
useXDeprecatedProvider(onUsage: (message: string) => void): void;
}

export declare class SchemaValidationException extends BaseException {
Expand Down
1 change: 1 addition & 0 deletions packages/angular/cli/models/architect-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export abstract class ArchitectCommand<

this._registry = new json.schema.CoreSchemaRegistry();
this._registry.addPostTransform(json.schema.transforms.addUndefinedDefaults);
this._registry.useXDeprecatedProvider(msg => this.logger.warn(msg));

const { workspace } = await workspaces.readWorkspace(
this.workspace.root,
Expand Down
6 changes: 0 additions & 6 deletions packages/angular/cli/models/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,6 @@ export interface Option {
*/
positional?: number;

/**
* Deprecation. If this flag is not false a warning will be shown on the console. Either `true`
* or a string to show the user as a notice.
*/
deprecated?: boolean | string;

/**
* Smart default object.
*/
Expand Down
5 changes: 0 additions & 5 deletions packages/angular/cli/models/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,6 @@ function _assignOption(
}

parsedOptions[option.name] = v;

if (option.deprecated !== undefined && option.deprecated !== false) {
warnings.push(`Option ${JSON.stringify(option.name)} is deprecated${
typeof option.deprecated == 'string' ? ': ' + option.deprecated : '.'}`);
}
}
} else {
let error = `Argument ${key} could not be parsed using value ${JSON.stringify(value)}.`;
Expand Down
37 changes: 0 additions & 37 deletions packages/angular/cli/models/parser_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,43 +171,6 @@ describe('parseArguments', () => {
});
});

it('handles deprecation', () => {
const options = [
{ name: 'bool', aliases: [], type: OptionType.Boolean, description: '' },
{ name: 'depr', aliases: [], type: OptionType.Boolean, description: '', deprecated: true },
{ name: 'deprM', aliases: [], type: OptionType.Boolean, description: '', deprecated: 'ABCD' },
];

const logger = new logging.Logger('');
const messages: string[] = [];

logger.subscribe(entry => messages.push(entry.message));

let result = parseArguments(['--bool'], options, logger);
expect(result).toEqual({ bool: true });
expect(messages).toEqual([]);

result = parseArguments(['--depr'], options, logger);
expect(result).toEqual({ depr: true });
expect(messages.length).toEqual(1);
expect(messages[0]).toMatch(/\bdepr\b/);
messages.shift();

result = parseArguments(['--depr', '--bool'], options, logger);
expect(result).toEqual({ depr: true, bool: true });
expect(messages.length).toEqual(1);
expect(messages[0]).toMatch(/\bdepr\b/);
messages.shift();

result = parseArguments(['--depr', '--bool', '--deprM'], options, logger);
expect(result).toEqual({ depr: true, deprM: true, bool: true });
expect(messages.length).toEqual(2);
expect(messages[0]).toMatch(/\bdepr\b/);
expect(messages[1]).toMatch(/\bdeprM\b/);
expect(messages[1]).toMatch(/\bABCD\b/);
messages.shift();
});

it('handles a flag being added multiple times', () => {
const options = [
{ name: 'bool', aliases: [], type: OptionType.Boolean, description: '' },
Expand Down
1 change: 1 addition & 0 deletions packages/angular/cli/models/schematic-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ export abstract class SchematicCommand<
workflow.engineHost.registerOptionsTransform(validateOptionsWithSchema(workflow.registry));

workflow.registry.addSmartDefaultProvider('projectName', getProjectName);
workflow.registry.useXDeprecatedProvider(msg => this.logger.warn(msg));

if (options.interactive !== false && isTTY()) {
workflow.registry.usePromptProvider((definitions: Array<schema.PromptDefinition>) => {
Expand Down
6 changes: 0 additions & 6 deletions packages/angular/cli/utilities/json-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,6 @@ export async function parseJsonSchemaToOptions(
const visible = current.visible === undefined || current.visible === true;
const hidden = !!current.hidden || !visible;

// Deprecated is set only if it's true or a string.
const xDeprecated = current['x-deprecated'];
const deprecated = (xDeprecated === true || typeof xDeprecated == 'string')
? xDeprecated : undefined;

const xUserAnalytics = current['x-user-analytics'];
const userAnalytics = typeof xUserAnalytics == 'number' ? xUserAnalytics : undefined;

Expand All @@ -267,7 +262,6 @@ export async function parseJsonSchemaToOptions(
...format !== undefined ? { format } : {},
hidden,
...userAnalytics ? { userAnalytics } : {},
...deprecated !== undefined ? { deprecated } : {},
...positional !== undefined ? { positional } : {},
};

Expand Down
3 changes: 3 additions & 0 deletions packages/angular_devkit/architect_cli/bin/architect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,9 @@ async function main(args: string[]): Promise<number> {
const registry = new schema.CoreSchemaRegistry();
registry.addPostTransform(schema.transforms.addUndefinedDefaults);

// Show usage of deprecated options
registry.useXDeprecatedProvider(msg => logger.warn(msg));

const { workspace } = await workspaces.readWorkspace(
configFilePath,
workspaces.createWorkspaceHost(new NodeJsSyncHost()),
Expand Down
1 change: 1 addition & 0 deletions packages/angular_devkit/core/src/json/schema/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export interface SchemaRegistry {
addFormat(format: SchemaFormat): void;
addSmartDefaultProvider<T>(source: string, provider: SmartDefaultProvider<T>): void;
usePromptProvider(provider: PromptProvider): void;
useXDeprecatedProvider(onUsage: (message: string) => void): void;

/**
* Add a transformation step before the validation of any Json.
Expand Down
17 changes: 14 additions & 3 deletions packages/angular_devkit/core/src/json/schema/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,7 @@ export class CoreSchemaRegistry implements SchemaRegistry {
this._ajv.addFormat(format.name, {
async: format.formatter.async,
validate,
// AJV typings list `compare` as required, but it is optional.
// tslint:disable-next-line:no-any
} as any);
});
}

addSmartDefaultProvider<T>(source: string, provider: SmartDefaultProvider<T>) {
Expand Down Expand Up @@ -755,4 +753,17 @@ export class CoreSchemaRegistry implements SchemaRegistry {
}),
);
}

useXDeprecatedProvider(onUsage: (message: string) => void): void {
this._ajv.addKeyword('x-deprecated', {
validate: (schema, _data, _parentSchema, _dataPath, _parentDataObject, propertyName) => {
if (schema) {
onUsage(`Option "${propertyName}" is deprecated${typeof schema == 'string' ? ': ' + schema : '.'}`);
}

return true;
},
errors: false,
});
}
}
31 changes: 31 additions & 0 deletions packages/angular_devkit/core/src/json/schema/registry_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,4 +481,35 @@ describe('CoreSchemaRegistry', () => {
.toPromise().then(done, done.fail);
});

it('adds deprecated options usage', done => {
const registry = new CoreSchemaRegistry();
const deprecatedMessages: string[] = [];
registry.useXDeprecatedProvider(m => deprecatedMessages.push(m));

const data = {
foo: true,
bar: true,
bat: true,
};

registry
.compile({
properties: {
foo: { type: 'boolean', 'x-deprecated': 'Use bar instead.' },
bar: { type: 'boolean', 'x-deprecated': true },
buz: { type: 'boolean', 'x-deprecated': true },
bat: { type: 'boolean', 'x-deprecated': false },
},
})
.pipe(
mergeMap(validator => validator(data)),
map(result => {
expect(deprecatedMessages.length).toBe(2);
expect(deprecatedMessages[0]).toBe('Option "foo" is deprecated: Use bar instead.');
expect(deprecatedMessages[1]).toBe('Option "bar" is deprecated.');
expect(result.success).toBe(true, result.errors);
}),
)
.toPromise().then(done, done.fail);
});
});
3 changes: 3 additions & 0 deletions packages/angular_devkit/schematics_cli/bin/schematics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ export async function main({
parsedArgs[key] = argv2[key];
}

// Show usage of deprecated options
workflow.registry.useXDeprecatedProvider(msg => logger.warn(msg));

// Pass the rest of the arguments as the smart default "argv". Then delete it.
workflow.registry.addSmartDefaultProvider('argv', (schema: JsonObject) => {
if ('index' in schema) {
Expand Down