Skip to content

Commit

Permalink
Merge pull request #574 from microsoft/tyriar/306
Browse files Browse the repository at this point in the history
Enable strict TS compiler option
  • Loading branch information
Tyriar authored Dec 27, 2022
2 parents a141a9d + afa9867 commit 5528270
Show file tree
Hide file tree
Showing 11 changed files with 90 additions and 76 deletions.
2 changes: 1 addition & 1 deletion src/conpty_console_list_agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ try {

const shellPid = parseInt(process.argv[2], 10);
const consoleProcessList = getConsoleProcessList(shellPid);
process.send({ consoleProcessList });
process.send!({ consoleProcessList });
process.exit(0);
4 changes: 2 additions & 2 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ interface IBasePtyForkOptions {
rows?: number;
cwd?: string;
env?: IProcessEnv;
encoding?: string;
encoding?: string | null;
handleFlowControl?: boolean;
flowControlPause?: string;
flowControlResume?: string;
Expand All @@ -120,5 +120,5 @@ export interface IWindowsPtyForkOptions extends IBasePtyForkOptions {
export interface IPtyOpenOptions {
cols?: number;
rows?: number;
encoding?: string;
encoding?: string | null;
}
40 changes: 20 additions & 20 deletions src/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { Socket } from 'net';
import { EventEmitter } from 'events';
import { ITerminal, IPtyForkOptions } from './interfaces';
import { ITerminal, IPtyForkOptions, IProcessEnv } from './interfaces';
import { EventEmitter2, IEvent } from './eventEmitter2';
import { IExitEvent } from './types';

Expand All @@ -22,18 +22,18 @@ const FLOW_CONTROL_PAUSE = '\x13'; // defaults to XOFF
const FLOW_CONTROL_RESUME = '\x11'; // defaults to XON

export abstract class Terminal implements ITerminal {
protected _socket: Socket;
protected _pid: number;
protected _fd: number;
protected _socket!: Socket; // HACK: This is unsafe
protected _pid: number = 0;
protected _fd: number = 0;
protected _pty: any;

protected _file: string;
protected _name: string;
protected _cols: number;
protected _rows: number;
protected _file!: string; // HACK: This is unsafe
protected _name!: string; // HACK: This is unsafe
protected _cols: number = 0;
protected _rows: number = 0;

protected _readable: boolean;
protected _writable: boolean;
protected _readable: boolean = false;
protected _writable: boolean = false;

protected _internalee: EventEmitter;
private _flowControlPause: string;
Expand All @@ -53,6 +53,11 @@ export abstract class Terminal implements ITerminal {
// for 'close'
this._internalee = new EventEmitter();

// setup flow control handling
this.handleFlowControl = !!(opt?.handleFlowControl);
this._flowControlPause = opt?.flowControlPause || FLOW_CONTROL_PAUSE;
this._flowControlResume = opt?.flowControlResume || FLOW_CONTROL_RESUME;

if (!opt) {
return;
}
Expand All @@ -67,11 +72,6 @@ export abstract class Terminal implements ITerminal {
this._checkType('uid', opt.uid ? opt.uid : undefined, 'number');
this._checkType('gid', opt.gid ? opt.gid : undefined, 'number');
this._checkType('encoding', opt.encoding ? opt.encoding : undefined, 'string');

// setup flow control handling
this.handleFlowControl = !!(opt.handleFlowControl);
this._flowControlPause = opt.flowControlPause || FLOW_CONTROL_PAUSE;
this._flowControlResume = opt.flowControlResume || FLOW_CONTROL_RESUME;
}

protected abstract _write(data: string): void;
Expand Down Expand Up @@ -157,9 +157,9 @@ export abstract class Terminal implements ITerminal {

public emit(eventName: string, ...args: any[]): any {
if (eventName === 'close') {
return this._internalee.emit.apply(this._internalee, arguments);
return this._internalee.emit.apply(this._internalee, arguments as any);
}
return this._socket.emit.apply(this._socket, arguments);
return this._socket.emit.apply(this._socket, arguments as any);
}

public listeners(eventName: string): Function[] {
Expand All @@ -183,8 +183,8 @@ export abstract class Terminal implements ITerminal {
public abstract kill(signal?: string): void;

public abstract get process(): string;
public abstract get master(): Socket;
public abstract get slave(): Socket;
public abstract get master(): Socket| undefined;
public abstract get slave(): Socket | undefined;

protected _close(): void {
this._socket.readable = false;
Expand All @@ -194,7 +194,7 @@ export abstract class Terminal implements ITerminal {
this._readable = false;
}

protected _parseEnv(env: {[key: string]: string}): string[] {
protected _parseEnv(env: IProcessEnv): string[] {
const keys = Object.keys(env || {});
const pairs = [];

Expand Down
4 changes: 1 addition & 3 deletions src/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
"lib": [
"es2015"
],
"alwaysStrict": true,
"noImplicitAny": true,
"noImplicitThis": true
"strict": true
},
"exclude": [
"node_modules",
Expand Down
52 changes: 26 additions & 26 deletions src/unixTerminal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ if (process.platform !== 'win32') {
describe('Constructor', () => {
it('should set a valid pts name', () => {
const term = new UnixTerminal('/bin/bash', [], {});
let regExp: RegExp;
let regExp: RegExp | undefined;
if (process.platform === 'linux') {
// https://linux.die.net/man/4/pts
regExp = /^\/dev\/pts\/\d+$/;
Expand All @@ -38,8 +38,8 @@ if (process.platform !== 'win32') {
it('should default to utf8', (done) => {
const term = new UnixTerminal('/bin/bash', [ '-c', `cat "${FIXTURES_PATH}"` ]);
term.on('data', (data) => {
assert.equal(typeof data, 'string');
assert.equal(data, '\u00E6');
assert.strictEqual(typeof data, 'string');
assert.strictEqual(data, '\u00E6');
done();
});
});
Expand All @@ -48,25 +48,25 @@ if (process.platform !== 'win32') {
encoding: null
});
term.on('data', (data) => {
assert.equal(typeof data, 'object');
assert.strictEqual(typeof data, 'object');
assert.ok(data instanceof Buffer);
assert.equal(0xC3, data[0]);
assert.equal(0xA6, data[1]);
assert.strictEqual(0xC3, data[0]);
assert.strictEqual(0xA6, data[1]);
done();
});
});
it('should support other encodings', (done) => {
const text = 'test æ!';
const term = new UnixTerminal(null, ['-c', 'echo "' + text + '"'], {
const term = new UnixTerminal(undefined, ['-c', 'echo "' + text + '"'], {
encoding: 'base64'
});
let buffer = '';
term.on('data', (data) => {
assert.equal(typeof data, 'string');
term.onData((data) => {
assert.strictEqual(typeof data, 'string');
buffer += data;
});
term.on('exit', () => {
assert.equal(Buffer.alloc(8, buffer, 'base64').toString().replace('\r', '').replace('\n', ''), text);
term.onExit(() => {
assert.strictEqual(Buffer.alloc(8, buffer, 'base64').toString().replace('\r', '').replace('\n', ''), text);
done();
});
});
Expand All @@ -77,21 +77,21 @@ if (process.platform !== 'win32') {

afterEach(() => {
if (term) {
term.slave.destroy();
term.master.destroy();
term.slave!.destroy();
term.master!.destroy();
}
});

it('should open a pty with access to a master and slave socket', (done) => {
term = UnixTerminal.open({});

let slavebuf = '';
term.slave.on('data', (data) => {
term.slave!.on('data', (data) => {
slavebuf += data;
});

let masterbuf = '';
term.master.on('data', (data) => {
term.master!.on('data', (data) => {
masterbuf += data;
});

Expand All @@ -103,8 +103,8 @@ if (process.platform !== 'win32') {
return false;
}, 200, 10);

term.slave.write('slave\n');
term.master.write('master\n');
term.slave!.write('slave\n');
term.master!.write('master\n');
});
});
describe('close', () => {
Expand Down Expand Up @@ -154,8 +154,8 @@ if (process.platform !== 'win32') {
});
p.on('close', () => {
// handlers in parent and child should have been triggered
assert.equal(buffer.indexOf('SIGINT in child') !== -1, true);
assert.equal(buffer.indexOf('SIGINT in parent') !== -1, true);
assert.strictEqual(buffer.indexOf('SIGINT in child') !== -1, true);
assert.strictEqual(buffer.indexOf('SIGINT in parent') !== -1, true);
done();
});
});
Expand Down Expand Up @@ -195,8 +195,8 @@ if (process.platform !== 'win32') {
});
p.on('close', () => {
// handlers in parent and child should have been triggered
assert.equal(buffer.indexOf('should not be printed') !== -1, false);
assert.equal(buffer.indexOf('SIGINT in parent') !== -1, true);
assert.strictEqual(buffer.indexOf('should not be printed') !== -1, false);
assert.strictEqual(buffer.indexOf('SIGINT in parent') !== -1, true);
done();
});
});
Expand All @@ -215,7 +215,7 @@ if (process.platform !== 'win32') {
});
term.on('exit', () => {
// no timeout in buffer
assert.equal(buffer, '');
assert.strictEqual(buffer, '');
done();
});
});
Expand Down Expand Up @@ -247,8 +247,8 @@ if (process.platform !== 'win32') {
});
term.on('exit', () => {
// should have called both handlers and only once
assert.equal(pHandlerCalled, 1);
assert.equal(buffer, 'SIGUSR1 in child\r\n');
assert.strictEqual(pHandlerCalled, 1);
assert.strictEqual(buffer, 'SIGUSR1 in child\r\n');
done();
});
});
Expand All @@ -267,7 +267,7 @@ if (process.platform !== 'win32') {
new UnixTerminal('/bin/echo', [], { cwd: '/nowhere' });
done(new Error('should have failed'));
} catch (e) {
assert.equal(e.toString(), 'Error: chdir() failed: No such file or directory');
assert.strictEqual((e as any).toString(), 'Error: chdir() failed: No such file or directory');
done();
}
});
Expand All @@ -276,7 +276,7 @@ if (process.platform !== 'win32') {
new UnixTerminal('/bin/echo', [], { uid: 999999 });
done(new Error('should have failed'));
} catch (e) {
assert.equal(e.toString(), 'Error: setuid() failed: Operation not permitted');
assert.strictEqual((e as any).toString(), 'Error: setuid() failed: Operation not permitted');
done();
}
});
Expand Down
16 changes: 8 additions & 8 deletions src/unixTerminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ export class UnixTerminal extends Terminal {
protected _readable: boolean;
protected _writable: boolean;

private _boundClose: boolean;
private _emittedClose: boolean;
private _master: net.Socket;
private _slave: net.Socket;
private _boundClose: boolean = false;
private _emittedClose: boolean = false;
private _master: net.Socket | undefined;
private _slave: net.Socket | undefined;

public get master(): net.Socket { return this._master; }
public get slave(): net.Socket { return this._slave; }
public get master(): net.Socket | undefined { return this._master; }
public get slave(): net.Socket | undefined { return this._slave; }

constructor(file?: string, args?: ArgvOrCommandLine, opt?: IPtyForkOptions) {
super(opt);
Expand Down Expand Up @@ -95,7 +95,7 @@ export class UnixTerminal extends Terminal {
// From macOS High Sierra 10.13.2 sometimes the socket never gets
// closed. A timeout is applied here to avoid the terminal never being
// destroyed when this occurs.
let timeout = setTimeout(() => {
let timeout: NodeJS.Timeout | null = setTimeout(() => {
timeout = null;
// Destroying the socket now will cause the close event to fire
this._socket.destroy();
Expand Down Expand Up @@ -217,7 +217,7 @@ export class UnixTerminal extends Terminal {
self._slave.resume();

self._socket = self._master;
self._pid = null;
self._pid = -1;
self._fd = term.master;
self._pty = term.pty;

Expand Down
2 changes: 1 addition & 1 deletion src/windowsConoutConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const FLUSH_DATA_INTERVAL = 1000;
*/
export class ConoutConnection implements IDisposable {
private _worker: Worker;
private _drainTimeout: NodeJS.Timeout;
private _drainTimeout: NodeJS.Timeout | undefined;
private _isDisposed: boolean = false;

private _onReady = new EventEmitter2<void>();
Expand Down
10 changes: 5 additions & 5 deletions src/windowsPtyAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ const FLUSH_DATA_INTERVAL = 1000;
export class WindowsPtyAgent {
private _inSocket: Socket;
private _outSocket: Socket;
private _pid: number;
private _innerPid: number;
private _innerPidHandle: number;
private _closeTimeout: NodeJS.Timer;
private _pid: number = 0;
private _innerPid: number = 0;
private _innerPidHandle: number = 0;
private _closeTimeout: NodeJS.Timer | undefined;
private _exitCode: number | undefined;
private _conoutSocketWorker: ConoutConnection;

Expand Down Expand Up @@ -202,7 +202,7 @@ export class WindowsPtyAgent {
});
}

public get exitCode(): number {
public get exitCode(): number | undefined {
if (this._useConpty) {
return this._exitCode;
}
Expand Down
11 changes: 5 additions & 6 deletions src/windowsTerminal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function pollForProcessTreeSize(pid: number, size: number, intervalMs: number =
})[0]);
const list: IWindowsProcessTreeResult[] = [];
while (openList.length) {
const current = openList.shift();
const current = openList.shift()!;
ps.filter(p => p.ppid === current.pid).map(p => {
return { name: p.name, pid: p.pid };
}).forEach(p => openList.push(p));
Expand All @@ -81,7 +81,6 @@ function pollForProcessTreeSize(pid: number, size: number, intervalMs: number =
if (tries * intervalMs >= timeoutMs) {
clearInterval(interval);
assert.fail(`Bad process state, expected: ${size}, actual: ${list.length}`);
resolve();
}
});
}, intervalMs);
Expand All @@ -105,10 +104,10 @@ if (process.platform === 'win32') {
term.write('notepad.exe\r');
term.write('node.exe\r');
pollForProcessTreeSize(term.pid, 4, 500, 5000).then(list => {
assert.equal(list[0].name, 'cmd.exe');
assert.equal(list[1].name, 'powershell.exe');
assert.equal(list[2].name, 'notepad.exe');
assert.equal(list[3].name, 'node.exe');
assert.strictEqual(list[0].name.toLowerCase(), 'cmd.exe');
assert.strictEqual(list[1].name.toLowerCase(), 'powershell.exe');
assert.strictEqual(list[2].name.toLowerCase(), 'notepad.exe');
assert.strictEqual(list[3].name.toLowerCase(), 'node.exe');
term.kill();
const desiredState: IProcessState = {};
desiredState[list[0].pid] = false;
Expand Down
Loading

0 comments on commit 5528270

Please sign in to comment.