Skip to content

Commit 798ab56

Browse files
committed
fix: use ChildProcess type for LanguageServerProcess
lsProcess is spawned using a `spawn` function of `child_process`. That means hiding the types behind an extended type can result in bugs. This is also simpler as we don't need to maintain the types for LanguageServerProcess
1 parent 2940f89 commit 798ab56

File tree

2 files changed

+20
-24
lines changed

2 files changed

+20
-24
lines changed

lib/auto-languageclient.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ export default class AutoLanguageClient {
283283
* Use this inside the `startServerProcess` override if the language server is a general executable
284284
* Also see the `spawnChildNode` method
285285
*/
286-
protected spawn(exe: string, args: string[], options: cp.SpawnOptions = {}): cp.ChildProcess {
286+
protected spawn(exe: string, args: string[], options: cp.SpawnOptions = {}): LanguageServerProcess {
287287
this.logger.debug(`starting "${exe} ${args.join(' ')}"`);
288288
return cp.spawn(exe, args, options);
289289
}
@@ -292,7 +292,7 @@ export default class AutoLanguageClient {
292292
* Use this inside the `startServerProcess` override if the language server is a JavaScript file.
293293
* Also see the `spawn` method
294294
*/
295-
protected spawnChildNode(args: string[], options: cp.SpawnOptions = {}): cp.ChildProcess {
295+
protected spawnChildNode(args: string[], options: cp.SpawnOptions = {}): LanguageServerProcess {
296296
this.logger.debug(`starting child Node "${args.join(' ')}"`);
297297
options.env = options.env || Object.create(process.env);
298298
if (options.env) {
@@ -370,8 +370,8 @@ export default class AutoLanguageClient {
370370
lsProcess.on('error', (err) => this.handleSpawnFailure(err));
371371
lsProcess.on("close", (...args) => this.handleCloseFailure(...args));
372372
lsProcess.on('exit', (code, signal) => this.logger.debug(`exit: code ${code} signal ${signal}`));
373-
lsProcess.stderr.setEncoding('utf8');
374-
lsProcess.stderr.on('data', (chunk: Buffer) => {
373+
lsProcess.stderr?.setEncoding('utf8');
374+
lsProcess.stderr?.on('data', (chunk: Buffer) => {
375375
const errorString = chunk.toString();
376376
this.handleServerStderr(errorString, projectPath);
377377
// Keep the last 5 lines for packages to use in messages
@@ -415,8 +415,13 @@ export default class AutoLanguageClient {
415415
writer = new rpc.SocketMessageWriter(this.socket);
416416
break;
417417
case 'stdio':
418-
reader = new rpc.StreamMessageReader(lsProcess.stdout);
419-
writer = new rpc.StreamMessageWriter(lsProcess.stdin);
418+
if (lsProcess.stdin !== null && lsProcess.stdout !== null) {
419+
reader = new rpc.StreamMessageReader(lsProcess.stdout);
420+
writer = new rpc.StreamMessageWriter(lsProcess.stdin);
421+
} else {
422+
this.logger.error(`The language server process for ${this.getLanguageName()} does not have a valid stdin and stdout`);
423+
return Utils.assertUnreachable('stdio' as never)
424+
}
420425
break;
421426
default:
422427
return Utils.assertUnreachable(connectionType);

lib/server-manager.ts

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import Convert from './convert';
22
import * as path from 'path';
3-
import * as stream from 'stream';
43
import * as ls from './languageclient';
5-
import { EventEmitter } from 'events';
4+
import { ChildProcess } from 'child_process'
65
import { Logger } from './logger';
76
import {
87
CompositeDisposable,
@@ -11,24 +10,16 @@ import {
1110
} from 'atom';
1211
import { ReportBusyWhile } from './utils';
1312

13+
14+
type MinimalLanguageServerProcess = Pick<ChildProcess, 'stdin' | 'stdout' | 'stderr' | 'pid' | 'kill' | 'on'>
15+
1416
/**
15-
* Public: Defines the minimum surface area for an object that resembles a
16-
* ChildProcess. This is used so that language packages with alternative
17-
* language server process hosting strategies can return something compatible
18-
* with AutoLanguageClient.startServerProcess.
17+
* Public: Defines a language server process which is either a ChildProcess, or it is a minimal object that resembles a
18+
* ChildProcess.
19+
* `MinimalLanguageServerProcess` is used so that language packages with alternative language server process hosting strategies
20+
* can return something compatible with `AutoLanguageClient.startServerProcess`.
1921
*/
20-
export interface LanguageServerProcess extends EventEmitter {
21-
stdin: stream.Writable;
22-
stdout: stream.Readable;
23-
stderr: stream.Readable;
24-
pid: number;
25-
26-
kill(signal?: NodeJS.Signals | number): void;
27-
on(event: 'close', listener: (code: number | null, signal: NodeJS.Signals | null) => void): this;
28-
on(event: 'disconnect', listener: () => void): this;
29-
on(event: 'error', listener: (err: Error) => void): this;
30-
on(event: 'exit', listener: (code: number | null, signal: NodeJS.Signals | null) => void): this;
31-
}
22+
export type LanguageServerProcess = ChildProcess | MinimalLanguageServerProcess
3223

3324
/** The necessary elements for a server that has started or is starting. */
3425
export interface ActiveServer {

0 commit comments

Comments
 (0)