fix: introduce deprecation decorator for the methods exposed in the public API

Author: DimitarTachev

lib/commands/preview.ts

@@ -38,7 +38,7 @@ export class PreviewCommand implements ICommand {
 
 	public async canExecute(args: string[]): Promise<boolean> {
 		if (args && args.length) {
-			this.$errors.failWithHelp(`The arguments '${args.join(" ")}' are not valid for the preview command.`);
+			this.$errors.failWithHelp(`The ${args.length > 1 ? "arguments" : "argument"} '${args.join(" ")}' ${args.length > 1 ? "are" : "is"} not valid for the preview command.`);
 		}
 
 		if (!this.$options.force) {

lib/common/decorators.ts

@@ -107,14 +107,14 @@ export function performanceLog(injector?: IInjector): any {
 					performanceService.processExecutionData(trackName, start, end, args);
 				} else {
 					resolvedPromise
-					.then(() => {
-						end = performanceService.now();
-						performanceService.processExecutionData(trackName, start, end, args);
-					})
-					.catch((err) => {
-						end = performanceService.now();
-						performanceService.processExecutionData(trackName, start, end, args);
-					});
+						.then(() => {
+							end = performanceService.now();
+							performanceService.processExecutionData(trackName, start, end, args);
+						})
+						.catch((err) => {
+							end = performanceService.now();
+							performanceService.processExecutionData(trackName, start, end, args);
+						});
 				}
 
 				return result;
@@ -130,3 +130,53 @@ export function performanceLog(injector?: IInjector): any {
 		return descriptor;
 	};
 }
+
+// inspired by https://github.com/NativeScript/NativeScript/blob/55dfe25938569edbec89255008e5ad9804901305/tns-core-modules/globals/globals.ts#L121-L137
+export function deprecated(additionalInfo?: string, injector?: IInjector): any {
+	const isDeprecatedMessage = " is deprecated.";
+	return (target: Object, key: string, descriptor: TypedPropertyDescriptor<any>): TypedPropertyDescriptor<any> => {
+		injector = injector || $injector;
+		additionalInfo = additionalInfo || "";
+		const $logger = <ILogger>injector.resolve("logger");
+		if (descriptor) {
+			if (descriptor.value) {
+				// method
+				const originalMethod = descriptor.value;
+
+				descriptor.value = function (...args: any[]) {
+					$logger.warn(`${key.toString()}${isDeprecatedMessage} ${additionalInfo}`);
+
+					return originalMethod.apply(this, args);
+				};
+
+				return descriptor;
+			} else {
+				// property
+				if (descriptor.set) {
+					const originalSetter = descriptor.set;
+					descriptor.set = function (...args: any[]) {
+						$logger.warn(`${key.toString()}${isDeprecatedMessage} ${additionalInfo}`);
+
+						originalSetter.apply(this, args);
+					};
+				}
+
+				if (descriptor.get) {
+					const originalGetter = descriptor.get;
+					descriptor.get = function (...args: any[]) {
+						$logger.warn(`${key.toString()}${isDeprecatedMessage} ${additionalInfo}`);
+
+						return originalGetter.apply(this, args);
+					};
+				}
+
+				return descriptor;
+			}
+		} else {
+			// class
+			$logger.warn(`${((target && ((<any>target).name || ((<any>target).constructor && (<any>target).constructor.name))) || target)}${isDeprecatedMessage} ${additionalInfo}`);
+
+			return target;
+		}
+	};
+}

lib/common/definitions/logger.d.ts

@@ -15,7 +15,7 @@ declare global {
 
 	interface ILogger {
 		initialize(opts?: ILoggerOptions): void;
-		initializeCliLogger(): void;
+		initializeCliLogger(opts?: ILoggerOptions): void;
 		getLevel(): string;
 		fatal(formatStr?: any, ...args: any[]): void;
 		error(formatStr?: any, ...args: any[]): void;

lib/common/errors.ts

@@ -2,6 +2,7 @@ import * as util from "util";
 import * as path from "path";
 import { SourceMapConsumer } from "source-map";
 import { isInteractive } from "./helpers";
+import { deprecated } from "./decorators";
 
 // we need this to overwrite .stack property (read-only in Error)
 function Exception() {
@@ -128,7 +129,7 @@ export class Errors implements IErrors {
 		return this.failWithOptions(opts, false, args);
 	}
 
-	// DEPRECATED: use .fail instead
+	@deprecated("Use `fail` instead.")
 	public failWithoutHelp(optsOrFormatStr: string | IFailOptions, ...args: any[]): never {
 		return this.fail(optsOrFormatStr, args);
 	}

lib/common/logger/logger.ts

@@ -12,9 +12,10 @@ export class Logger implements ILogger {
 	private log4jsLogger: log4js.Logger = null;
 	private passwordRegex = /(password=).*?(['&,]|$)|(password["']?\s*:\s*["']).*?(["'])/i;
 	private passwordReplacement = "$1$3*******$2$4";
+	private defaultLogLevel: LoggerLevel;
 
-	constructor(private $config: Config.IConfig,
-		private $options: IOptions) {
+	constructor(private $config: Config.IConfig) {
+		this.defaultLogLevel = this.$config.DEBUG ? LoggerLevel.TRACE : LoggerLevel.INFO;
 	}
 
 	@cache()
@@ -40,7 +41,7 @@ export class Logger implements ILogger {
 		const categories: IDictionary<{ appenders: string[]; level: string; }> = {
 			default: {
 				appenders: ['out'],
-				level: level || (this.$config.DEBUG ? "TRACE" : "INFO")
+				level: level || this.defaultLogLevel
 			}
 		};
 
@@ -49,12 +50,12 @@ export class Logger implements ILogger {
 		this.log4jsLogger = log4js.getLogger();
 	}
 
-	public initializeCliLogger(): void {
+	public initializeCliLogger(opts?: ILoggerOptions): void {
 		log4js.addLayout("cli", layout);
 
 		this.initialize({
 			appenderOptions: { type: LoggerAppenders.cliAppender, layout: { type: "cli" } },
-			level: <any>this.$options.log
+			level: opts.level || this.defaultLogLevel
 		});
 	}
 

lib/common/services/commands-service.ts

@@ -91,16 +91,17 @@ export class CommandsService implements ICommandsService {
 		return false;
 	}
 
-	private printHelpSuggestion(commandName: string, commandArguments: string[]): Promise<void> {
-		const commandHelp = `tns ${helpers.stringReplaceAll(this.beautifyCommandName(commandName), "|", " ")} --help`;
-		this.$logger.printMarkdown(`Run \`${commandHelp}\` for more information.`);
+	private printHelpSuggestion(commandName?: string): Promise<void> {
+		const command = commandName ? helpers.stringReplaceAll(this.beautifyCommandName(commandName), "|", " ") + " " : "";
+		const commandHelp = `tns ${command}--help`;
+		this.$logger.printMarkdown(`\`Run '${commandHelp}' for more information.\``);
 		return;
 	}
 
 	private async executeCommandAction(commandName: string, commandArguments: string[], action: (_commandName: string, _commandArguments: string[]) => Promise<boolean>): Promise<boolean> {
 		return this.$errors.beginCommand(
 			() => action.apply(this, [commandName, commandArguments]),
-			() => this.printHelpSuggestion(commandName, commandArguments));
+			() => this.printHelpSuggestion(commandName));
 	}
 
 	private async tryExecuteCommandAction(commandName: string, commandArguments: string[]): Promise<boolean> {
@@ -124,7 +125,7 @@ export class CommandsService implements ICommandsService {
 			const command = this.$injector.resolveCommand(commandName);
 			if (command) {
 				this.$logger.error(`Command '${commandName} ${commandArguments}' cannot be executed.`);
-				await this.printHelpSuggestion(commandName, commandArguments);
+				await this.printHelpSuggestion(commandName);
 			}
 		}
 	}
@@ -167,7 +168,8 @@ export class CommandsService implements ICommandsService {
 		if (extensionData) {
 			this.$logger.warn(extensionData.installationMessage);
 		} else {
-			this.$logger.fatal("Unknown command '%s'. Use '%s help' for help.", beautifiedName, this.$staticConfig.CLIENT_NAME.toLowerCase());
+			this.$logger.error("Unknown command '%s'.", beautifiedName);
+			await this.printHelpSuggestion();
 			this.tryMatchCommand(commandName);
 		}
 

lib/common/test/unit-tests/decorators.ts

@@ -58,14 +58,14 @@ describe("decorators", () => {
 				generatePublicApiFromExportedDecorator();
 				const actualResult: any = $injector.publicApi.__modules__[moduleName][propertyName]();
 				assert.deepEqual(actualResult, expectedResult);
-		});
+			});
 
 			it(`passes correct arguments to original function, when argument type is: ${_.isArray(expectedResult) ? "array" : typeof (expectedResult)}`, () => {
 				$injector.register(moduleName, { propertyName: (arg: any) => arg });
 				generatePublicApiFromExportedDecorator();
 				const actualResult: any = $injector.publicApi.__modules__[moduleName][propertyName](expectedResult);
 				assert.deepEqual(actualResult, expectedResult);
-		});
+			});
 		});
 
 		it("returns Promise, which is resolved to correct value (function without arguments)", (done: mocha.Done) => {
@@ -202,7 +202,7 @@ describe("decorators", () => {
 			generatePublicApiFromExportedDecorator();
 			assert.throws(() => $injector.publicApi.__modules__[moduleName][propertyName](), errorMessage);
 		});
-		});
+	});
 
 	describe("cache", () => {
 		it("executes implementation of method only once and returns the same result each time whent it is called (number return type)", () => {
@@ -433,12 +433,12 @@ describe("decorators", () => {
 		});
 
 		_.each(expectedResults, (expectedResult: any) => {
-			it("returns proper result",  () => {
+			it("returns proper result", () => {
 				const actualResult = testInstance.testMethod(expectedResult);
 				assert.deepEqual(actualResult, expectedResult);
 			});
 
-			it("returns proper result when async",  () => {
+			it("returns proper result when async", () => {
 				const promise = testInstance.testAsyncMehtod(expectedResult);
 
 				assert.notDeepEqual(promise.then, undefined);
@@ -449,20 +449,20 @@ describe("decorators", () => {
 			});
 		});
 
-		it("method has same toString",  () => {
+		it("method has same toString", () => {
 			assert.equal(testInstance.testMethod.toString(), undecoratedTestInstance.testMethod.toString());
 		});
 
-		it("method has same name",  () => {
+		it("method has same name", () => {
 			assert.equal(testInstance.testMethod.name, undecoratedTestInstance.testMethod.name);
 		});
 
-		it("does not eat errors",  () => {
+		it("does not eat errors", () => {
 			assert.throws(testInstance.throwMethod, testErrorMessage);
 			assert.isRejected(testInstance.rejectMethod(), testErrorMessage);
 		});
 
-		it("calls performance service on method call",  async () => {
+		it("calls performance service on method call", async () => {
 			const performanceService = testInjector.resolve("performanceService");
 			const processExecutionDataStub: sinon.SinonStub = sinon.stub(performanceService, "processExecutionData");
 
@@ -486,4 +486,128 @@ describe("decorators", () => {
 			checkSubCall(processExecutionDataStub.secondCall, "TestClass__testAsyncMehtod");
 		});
 	});
+
+	describe("deprecated", () => {
+		const testDepMessage = "Just stop using this!";
+		const warnings: string[] = [];
+		let testInjector: IInjector;
+		interface ITestInterface {
+			testField: string;
+			testProp: string;
+			depMethodWithParam(arg: any): any;
+			depMethodWithoutParam(): void;
+			depAsyncMethod(arg: any): Promise<any>;
+			nonDepMethod(): any;
+		}
+		let testInstance: ITestInterface;
+
+		function createTestInjector(): IInjector {
+			testInjector = new Yok();
+			testInjector.register("config", {});
+			testInjector.register("options", {});
+			testInjector.register("logger", {
+				warn: (message: string) => {
+					warnings.push(message);
+				}
+			});
+
+			return testInjector;
+		}
+
+		beforeEach(() => {
+			warnings.splice(0, warnings.length);
+			testInjector = createTestInjector();
+
+			class TestClass implements ITestInterface {
+				public testField: string = "test";
+
+				@decoratorsLib.deprecated(testDepMessage, testInjector)
+				public get testProp(): string {
+					return "hi";
+				}
+
+				public set testProp(value: string) {
+					return;
+				}
+
+				@decoratorsLib.deprecated(testDepMessage, testInjector)
+				depMethodWithParam(arg: any) {
+					return arg;
+				}
+
+				@decoratorsLib.deprecated(testDepMessage, testInjector)
+				depMethodWithoutParam() {
+					return;
+				}
+
+				@decoratorsLib.deprecated(testDepMessage, testInjector)
+				async depAsyncMethod(arg: any) {
+					return Promise.resolve(arg);
+				}
+
+				nonDepMethod() {
+					return;
+				}
+			}
+
+			testInstance = new TestClass();
+		});
+
+		it("method without params", () => {
+			testInstance.depMethodWithoutParam();
+			assert.equal(warnings.length, 1);
+			assert.equal(warnings[0], `depMethodWithoutParam is deprecated. ${testDepMessage}`);
+		});
+
+		it("method with params", () => {
+			const param = 5;
+			const result = testInstance.depMethodWithParam(param);
+			assert.equal(result, param);
+			assert.equal(warnings.length, 1);
+			assert.equal(warnings[0], `depMethodWithParam is deprecated. ${testDepMessage}`);
+		});
+
+		it("async method with params", async () => {
+			const param = 5;
+			const result = await testInstance.depAsyncMethod(param);
+			assert.equal(result, param);
+			assert.equal(warnings.length, 1);
+			assert.equal(warnings[0], `depAsyncMethod is deprecated. ${testDepMessage}`);
+		});
+
+		it("property getter", async () => {
+			const result = testInstance.testProp;
+			assert.equal(result, "hi");
+			assert.equal(warnings.length, 1);
+			assert.equal(warnings[0], `testProp is deprecated. ${testDepMessage}`);
+		});
+
+		it("property setter", async () => {
+			testInstance.testProp = "newValue";
+			assert.equal(warnings.length, 1);
+			assert.equal(warnings[0], `testProp is deprecated. ${testDepMessage}`);
+		});
+
+		it("non deprecated field", async () => {
+			const result = testInstance.testField;
+			assert.equal(result, "test");
+			assert.equal(warnings.length, 0);
+		});
+
+		it("non deprecated method", () => {
+			testInstance.nonDepMethod();
+			assert.equal(warnings.length, 0);
+		});
+
+		it("class", async () => {
+			@decoratorsLib.deprecated(testDepMessage, testInjector)
+			class TestClassDeprecated {
+			}
+
+			const depClass = new TestClassDeprecated();
+			assert.isNotNull(depClass);
+			assert.equal(warnings.length, 1);
+			assert.equal(warnings[0], `TestClassDeprecated is deprecated. ${testDepMessage}`);
+		});
+	});
 });