Skip to content

Commit fb92a7e

Browse files
author
Andy Hanson
committed
Code review
1 parent f5ed9f3 commit fb92a7e

21 files changed

+76
-107
lines changed

src/compiler/diagnosticMessages.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3811,7 +3811,7 @@
38113811
},
38123812

38133813
"File is a CommonJS module; it may be converted to an ES6 module.": {
3814-
"category": "Info",
3814+
"category": "Suggestion",
38153815
"code": 80001
38163816
},
38173817

src/compiler/program.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ namespace ts {
255255
switch (category) {
256256
case DiagnosticCategory.Error: return ForegroundColorEscapeSequences.Red;
257257
case DiagnosticCategory.Warning: return ForegroundColorEscapeSequences.Yellow;
258-
case DiagnosticCategory.Info: return Debug.fail("Should never get an Info diagnostic on the command line.");
258+
case DiagnosticCategory.Suggestion: return Debug.fail("Should never get an Info diagnostic on the command line.");
259259
case DiagnosticCategory.Message: return ForegroundColorEscapeSequences.Blue;
260260
}
261261
}

src/compiler/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4017,7 +4017,7 @@ namespace ts {
40174017
export enum DiagnosticCategory {
40184018
Warning,
40194019
Error,
4020-
Info,
4020+
Suggestion,
40214021
Message
40224022
}
40234023
/* @internal */

src/harness/fourslash.ts

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ namespace FourSlash {
509509
return [
510510
...this.languageService.getSyntacticDiagnostics(fileName),
511511
...this.languageService.getSemanticDiagnostics(fileName),
512-
...this.languageService.getInfoDiagnostics(fileName),
512+
...this.languageService.getSuggestionDiagnostics(fileName),
513513
];
514514
}
515515

@@ -581,11 +581,10 @@ namespace FourSlash {
581581
return "global";
582582
}
583583

584-
public verifyNoErrors(options: FourSlashInterface.VerifyNoErrorsOptions) {
584+
public verifyNoErrors() {
585585
ts.forEachKey(this.inputFiles, fileName => {
586586
if (!ts.isAnySupportedFileExtension(fileName)) return;
587-
let errors = this.getDiagnostics(fileName);
588-
if (options.ignoreInfoDiagnostics) errors = errors.filter(e => e.category !== ts.DiagnosticCategory.Info);
587+
const errors = this.getDiagnostics(fileName).filter(e => e.category !== ts.DiagnosticCategory.Suggestion);
589588
if (errors.length) {
590589
this.printErrorLog(/*expectErrors*/ false, errors);
591590
const error = errors[0];
@@ -1250,8 +1249,8 @@ Actual: ${stringify(fullActual)}`);
12501249
this.testDiagnostics(expected, diagnostics);
12511250
}
12521251

1253-
public getInfoDiagnostics(expected: ReadonlyArray<ts.RealizedDiagnostic>): void {
1254-
this.testDiagnostics(expected, this.languageService.getInfoDiagnostics(this.activeFile.fileName));
1252+
public getSuggestionDiagnostics(expected: ReadonlyArray<ts.RealizedDiagnostic>): void {
1253+
this.testDiagnostics(expected, this.languageService.getSuggestionDiagnostics(this.activeFile.fileName));
12551254
}
12561255

12571256
private testDiagnostics(expected: ReadonlyArray<ts.RealizedDiagnostic>, diagnostics: ReadonlyArray<ts.Diagnostic>) {
@@ -4146,8 +4145,8 @@ namespace FourSlashInterface {
41464145
this.state.verifyCurrentSignatureHelpIs(expected);
41474146
}
41484147

4149-
public noErrors(options: VerifyNoErrorsOptions = {}) {
4150-
this.state.verifyNoErrors(options);
4148+
public noErrors() {
4149+
this.state.verifyNoErrors();
41514150
}
41524151

41534152
public numberOfErrorsInCurrentFile(expected: number) {
@@ -4335,8 +4334,8 @@ namespace FourSlashInterface {
43354334
this.state.getSemanticDiagnostics(expected);
43364335
}
43374336

4338-
public getInfoDiagnostics(expected: ReadonlyArray<ts.RealizedDiagnostic>) {
4339-
this.state.getInfoDiagnostics(expected);
4337+
public getSuggestionDiagnostics(expected: ReadonlyArray<ts.RealizedDiagnostic>) {
4338+
this.state.getSuggestionDiagnostics(expected);
43404339
}
43414340

43424341
public ProjectInfo(expected: string[]) {
@@ -4677,8 +4676,4 @@ namespace FourSlashInterface {
46774676
source?: string;
46784677
description: string;
46794678
}
4680-
4681-
export interface VerifyNoErrorsOptions {
4682-
ignoreInfoDiagnostics?: boolean;
4683-
}
46844679
}

src/harness/harnessLanguageService.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,8 +402,8 @@ namespace Harness.LanguageService {
402402
getSemanticDiagnostics(fileName: string): ts.Diagnostic[] {
403403
return unwrapJSONCallResult(this.shim.getSemanticDiagnostics(fileName));
404404
}
405-
getInfoDiagnostics(fileName: string): ts.Diagnostic[] {
406-
return unwrapJSONCallResult(this.shim.getInfoDiagnostics(fileName));
405+
getSuggestionDiagnostics(fileName: string): ts.Diagnostic[] {
406+
return unwrapJSONCallResult(this.shim.getSuggestionDiagnostics(fileName));
407407
}
408408
getCompilerOptionsDiagnostics(): ts.Diagnostic[] {
409409
return unwrapJSONCallResult(this.shim.getCompilerOptionsDiagnostics());

src/harness/unittests/session.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ namespace ts.server {
216216
CommandNames.GeterrForProject,
217217
CommandNames.SemanticDiagnosticsSync,
218218
CommandNames.SyntacticDiagnosticsSync,
219-
CommandNames.InfoDiagnosticsSync,
219+
CommandNames.SuggestionDiagnosticsSync,
220220
CommandNames.NavBar,
221221
CommandNames.NavBarFull,
222222
CommandNames.Navto,

src/harness/unittests/tsserverProjectSystem.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3080,7 +3080,7 @@ namespace ts.projectSystem {
30803080

30813081
host.runQueuedImmediateCallbacks(1);
30823082
assert.isFalse(hasError());
3083-
checkErrorMessage(session, "infoDiag", { file: untitledFile, diagnostics: [] });
3083+
checkErrorMessage(session, "suggestionDiag", { file: untitledFile, diagnostics: [] });
30843084
checkCompleteEvent(session, 2, expectedSequenceId);
30853085
session.clearMessages();
30863086
}
@@ -3144,7 +3144,7 @@ namespace ts.projectSystem {
31443144
session.clearMessages();
31453145

31463146
host.runQueuedImmediateCallbacks(1);
3147-
checkErrorMessage(session, "infoDiag", { file: app.path, diagnostics: [] });
3147+
checkErrorMessage(session, "suggestionDiag", { file: app.path, diagnostics: [] });
31483148
checkCompleteEvent(session, 2, expectedSequenceId);
31493149
session.clearMessages();
31503150
}
@@ -3953,7 +3953,7 @@ namespace ts.projectSystem {
39533953
session.clearMessages();
39543954

39553955
host.runQueuedImmediateCallbacks(1);
3956-
checkErrorMessage(session, "infoDiag", { file: file1.path, diagnostics: [] });
3956+
checkErrorMessage(session, "suggestionDiag", { file: file1.path, diagnostics: [] });
39573957
checkCompleteEvent(session, 2, expectedSequenceId);
39583958
session.clearMessages();
39593959

@@ -4018,7 +4018,7 @@ namespace ts.projectSystem {
40184018

40194019
host.runQueuedImmediateCallbacks(1);
40204020

4021-
checkErrorMessage(session, "infoDiag", {
4021+
checkErrorMessage(session, "suggestionDiag", {
40224022
file: file.path,
40234023
diagnostics: [
40244024
createDiagnostic({ line: 1, offset: 1 }, { line: 1, offset: 13 }, Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module)
@@ -5227,7 +5227,7 @@ namespace ts.projectSystem {
52275227
host.runQueuedImmediateCallbacks(1);
52285228
assert.equal(host.getOutput().length, 2);
52295229
const e3 = <protocol.Event>getMessage(0);
5230-
assert.equal(e3.event, "infoDiag");
5230+
assert.equal(e3.event, "suggestionDiag");
52315231
verifyRequestCompleted(getErrId, 1);
52325232

52335233
cancellationToken.resetToken();

src/server/client.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -348,13 +348,13 @@ namespace ts.server {
348348
getSemanticDiagnostics(file: string): Diagnostic[] {
349349
return this.getDiagnostics(file, CommandNames.SemanticDiagnosticsSync);
350350
}
351-
getInfoDiagnostics(file: string): Diagnostic[] {
352-
return this.getDiagnostics(file, CommandNames.InfoDiagnosticsSync);
351+
getSuggestionDiagnostics(file: string): Diagnostic[] {
352+
return this.getDiagnostics(file, CommandNames.SuggestionDiagnosticsSync);
353353
}
354354

355355
private getDiagnostics(file: string, command: CommandNames) {
356-
const request = this.processRequest<protocol.SyntacticDiagnosticsSyncRequest | protocol.SemanticDiagnosticsSyncRequest | protocol.InfoDiagnosticsSyncRequest>(command, { file, includeLinePosition: true });
357-
const response = this.processResponse<protocol.SyntacticDiagnosticsSyncResponse | protocol.SemanticDiagnosticsSyncResponse | protocol.InfoDiagnosticsSyncResponse>(request);
356+
const request = this.processRequest<protocol.SyntacticDiagnosticsSyncRequest | protocol.SemanticDiagnosticsSyncRequest | protocol.SuggestionDiagnosticsSyncRequest>(command, { file, includeLinePosition: true });
357+
const response = this.processResponse<protocol.SyntacticDiagnosticsSyncResponse | protocol.SemanticDiagnosticsSyncResponse | protocol.SuggestionDiagnosticsSyncResponse>(request);
358358

359359
return (<protocol.DiagnosticWithLinePosition[]>response.body).map(entry => {
360360
const category = firstDefined(Object.keys(DiagnosticCategory), id =>

src/server/protocol.ts

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ namespace ts.server.protocol {
4242
GeterrForProject = "geterrForProject",
4343
SemanticDiagnosticsSync = "semanticDiagnosticsSync",
4444
SyntacticDiagnosticsSync = "syntacticDiagnosticsSync",
45-
InfoDiagnosticsSync = "infoDiagnosticsSync",
45+
SuggestionDiagnosticsSync = "suggestionDiagnosticsSync",
4646
NavBar = "navbar",
4747
/* @internal */
4848
NavBarFull = "navbar-full",
@@ -2011,18 +2011,13 @@ namespace ts.server.protocol {
20112011
body?: Diagnostic[] | DiagnosticWithLinePosition[];
20122012
}
20132013

2014-
export interface InfoDiagnosticsSyncRequest extends FileRequest {
2015-
command: CommandTypes.InfoDiagnosticsSync;
2016-
arguments: InfoDiagnosticsSyncRequestArgs;
2014+
export interface SuggestionDiagnosticsSyncRequest extends FileRequest {
2015+
command: CommandTypes.SuggestionDiagnosticsSync;
2016+
arguments: SuggestionDiagnosticsSyncRequestArgs;
20172017
}
20182018

2019-
export interface InfoDiagnosticsSyncRequestArgs extends FileRequestArgs {
2020-
includeLinePosition?: boolean;
2021-
}
2022-
2023-
export interface InfoDiagnosticsSyncResponse extends Response {
2024-
body?: Diagnostic[] | DiagnosticWithLinePosition[];
2025-
}
2019+
export type SuggestionDiagnosticsSyncRequestArgs = SemanticDiagnosticsSyncRequestArgs;
2020+
export type SuggestionDiagnosticsSyncResponse = SemanticDiagnosticsSyncResponse;
20262021

20272022
/**
20282023
* Synchronous request for syntactic diagnostics of one file.
@@ -2135,7 +2130,7 @@ namespace ts.server.protocol {
21352130
text: string;
21362131

21372132
/**
2138-
* The category of the diagnostic message. This is the lower-case of a DiagnosticCategory member.
2133+
* The category of the diagnostic message, e.g. "error", "warning", or "suggestion".
21392134
*/
21402135
category: string;
21412136

@@ -2169,7 +2164,7 @@ namespace ts.server.protocol {
21692164
diagnostics: Diagnostic[];
21702165
}
21712166

2172-
export type DiagnosticEventKind = "semanticDiag" | "syntaxDiag" | "infoDiag";
2167+
export type DiagnosticEventKind = "semanticDiag" | "syntaxDiag" | "suggestionDiag";
21732168

21742169
/**
21752170
* Event message for DiagnosticEventKind event types.

src/server/session.ts

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -466,43 +466,29 @@ namespace ts.server {
466466
}
467467

468468
private semanticCheck(file: NormalizedPath, project: Project) {
469-
try {
470-
const diags = isDeclarationFileInJSOnlyNonConfiguredProject(project, file)
471-
? emptyArray
472-
: project.getLanguageService().getSemanticDiagnostics(file);
473-
this.sendDiagnosticsEvent(file, project, diags, "semanticDiag");
474-
}
475-
catch (err) {
476-
this.logError(err, "semantic check");
477-
}
469+
const diags = isDeclarationFileInJSOnlyNonConfiguredProject(project, file)
470+
? emptyArray
471+
: project.getLanguageService().getSemanticDiagnostics(file);
472+
this.sendDiagnosticsEvent(file, project, diags, "semanticDiag");
478473
}
479474

480475
private syntacticCheck(file: NormalizedPath, project: Project) {
481-
try {
482-
// TODO: why do we check for diagnostics existence here, but in semanticCheck send unconditionally?
483-
const diagnostics = project.getLanguageService().getSyntacticDiagnostics(file);
484-
if (diagnostics) {
485-
this.sendDiagnosticsEvent(file, project, diagnostics, "syntaxDiag");
486-
}
487-
}
488-
catch (err) {
489-
this.logError(err, "syntactic check");
490-
}
476+
this.sendDiagnosticsEvent(file, project, project.getLanguageService().getSyntacticDiagnostics(file), "syntaxDiag");
491477
}
492478

493479
private infoCheck(file: NormalizedPath, project: Project) {
480+
this.sendDiagnosticsEvent(file, project, project.getLanguageService().getSuggestionDiagnostics(file), "suggestionDiag");
481+
}
482+
483+
private sendDiagnosticsEvent(file: NormalizedPath, project: Project, diagnostics: ReadonlyArray<Diagnostic>, kind: protocol.DiagnosticEventKind): void {
494484
try {
495-
this.sendDiagnosticsEvent(file, project, project.getLanguageService().getInfoDiagnostics(file), "infoDiag");
485+
this.event<protocol.DiagnosticEventBody>({ file, diagnostics: diagnostics.map(diag => formatDiag(file, project, diag)) }, kind);
496486
}
497487
catch (err) {
498-
this.logError(err, "info check");
488+
this.logError(err, kind);
499489
}
500490
}
501491

502-
private sendDiagnosticsEvent(file: NormalizedPath, project: Project, diagnostics: ReadonlyArray<Diagnostic>, kind: protocol.DiagnosticEventKind): void {
503-
this.event<protocol.DiagnosticEventBody>({ file, diagnostics: diagnostics.map(diag => formatDiag(file, project, diag)) }, kind);
504-
}
505-
506492
private updateErrorCheck(next: NextStep, checkList: PendingErrorCheck[], ms: number, requireOpen = true) {
507493
const seq = this.changeSeq;
508494
const followMs = Math.min(ms, 200);
@@ -779,14 +765,14 @@ namespace ts.server {
779765
return this.getDiagnosticsWorker(args, /*isSemantic*/ true, (project, file) => project.getLanguageService().getSemanticDiagnostics(file), args.includeLinePosition);
780766
}
781767

782-
private getInfoDiagnosticsSync(args: protocol.InfoDiagnosticsSyncRequestArgs): ReadonlyArray<protocol.Diagnostic> | ReadonlyArray<protocol.DiagnosticWithLinePosition> {
768+
private getSuggestionDiagnosticsSync(args: protocol.SuggestionDiagnosticsSyncRequestArgs): ReadonlyArray<protocol.Diagnostic> | ReadonlyArray<protocol.DiagnosticWithLinePosition> {
783769
const { configFile } = this.getConfigFileAndProject(args);
784770
if (configFile) {
785771
// Currently there are no info diagnostics for config files.
786772
return emptyArray;
787773
}
788774
// isSemantic because we don't want to info diagnostics in declaration files for JS-only users
789-
return this.getDiagnosticsWorker(args, /*isSemantic*/ true, (project, file) => project.getLanguageService().getInfoDiagnostics(file), args.includeLinePosition);
775+
return this.getDiagnosticsWorker(args, /*isSemantic*/ true, (project, file) => project.getLanguageService().getSuggestionDiagnostics(file), args.includeLinePosition);
790776
}
791777

792778
private getDocumentHighlights(args: protocol.DocumentHighlightsRequestArgs, simplifiedResult: boolean): ReadonlyArray<protocol.DocumentHighlightsItem> | ReadonlyArray<DocumentHighlights> {
@@ -1986,8 +1972,8 @@ namespace ts.server {
19861972
[CommandNames.SyntacticDiagnosticsSync]: (request: protocol.SyntacticDiagnosticsSyncRequest) => {
19871973
return this.requiredResponse(this.getSyntacticDiagnosticsSync(request.arguments));
19881974
},
1989-
[CommandNames.InfoDiagnosticsSync]: (request: protocol.InfoDiagnosticsSyncRequest) => {
1990-
return this.requiredResponse(this.getInfoDiagnosticsSync(request.arguments));
1975+
[CommandNames.SuggestionDiagnosticsSync]: (request: protocol.SuggestionDiagnosticsSyncRequest) => {
1976+
return this.requiredResponse(this.getSuggestionDiagnosticsSync(request.arguments));
19911977
},
19921978
[CommandNames.Geterr]: (request: protocol.GeterrRequest) => {
19931979
this.errorCheck.startNew(next => this.getDiagnostics(next, request.arguments.delay, request.arguments.files));

src/services/codefixes/convertToEs6Module.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
/* @internal */
22
namespace ts.codefix {
3-
const errorCodes = [Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module.code];
43
registerCodeFix({
5-
errorCodes,
4+
errorCodes: [Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module.code],
65
getCodeActions(context) {
76
const description = getLocaleSpecificMessage(Diagnostics.Convert_to_ES6_module);
87
const { sourceFile, program } = context;

src/services/services.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
/// <reference path='documentRegistry.ts' />
1111
/// <reference path='findAllReferences.ts' />
1212
/// <reference path='goToDefinition.ts' />
13-
/// <reference path='infoDiagnostics.ts' />
1413
/// <reference path='jsDoc.ts' />
1514
/// <reference path='jsTyping.ts' />
1615
/// <reference path='navigateTo.ts' />
@@ -21,6 +20,7 @@
2120
/// <reference path='preProcess.ts' />
2221
/// <reference path='rename.ts' />
2322
/// <reference path='signatureHelp.ts' />
23+
/// <reference path='suggestionDiagnostics.ts' />
2424
/// <reference path='symbolDisplay.ts' />
2525
/// <reference path='transpile.ts' />
2626
/// <reference path='formatting\formatting.ts' />
@@ -1420,9 +1420,9 @@ namespace ts {
14201420
return [...semanticDiagnostics, ...declarationDiagnostics];
14211421
}
14221422

1423-
function getInfoDiagnostics(fileName: string): Diagnostic[] {
1423+
function getSuggestionDiagnostics(fileName: string): Diagnostic[] {
14241424
synchronizeHostData();
1425-
return infoDiagnostics(getValidSourceFile(fileName));
1425+
return computeSuggestionDiagnostics(getValidSourceFile(fileName));
14261426
}
14271427

14281428
function getCompilerOptionsDiagnostics() {
@@ -2107,7 +2107,7 @@ namespace ts {
21072107
cleanupSemanticCache,
21082108
getSyntacticDiagnostics,
21092109
getSemanticDiagnostics,
2110-
getInfoDiagnostics,
2110+
getSuggestionDiagnostics,
21112111
getCompilerOptionsDiagnostics,
21122112
getSyntacticClassifications,
21132113
getSemanticClassifications,

src/services/shims.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ namespace ts {
144144

145145
getSyntacticDiagnostics(fileName: string): string;
146146
getSemanticDiagnostics(fileName: string): string;
147-
getInfoDiagnostics(fileName: string): string;
147+
getSuggestionDiagnostics(fileName: string): string;
148148
getCompilerOptionsDiagnostics(): string;
149149

150150
getSyntacticClassifications(fileName: string, start: number, length: number): string;
@@ -716,8 +716,8 @@ namespace ts {
716716
});
717717
}
718718

719-
public getInfoDiagnostics(fileName: string): string {
720-
return this.forwardJSONCall(`getInfoDiagnostics('${fileName}')`, () => this.realizeDiagnostics(this.languageService.getInfoDiagnostics(fileName)));
719+
public getSuggestionDiagnostics(fileName: string): string {
720+
return this.forwardJSONCall(`getSuggestionDiagnostics('${fileName}')`, () => this.realizeDiagnostics(this.languageService.getSuggestionDiagnostics(fileName)));
721721
}
722722

723723
public getCompilerOptionsDiagnostics(): string {

src/services/infoDiagnostics.ts renamed to src/services/suggestionDiagnostics.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* @internal */
22
namespace ts {
3-
export function infoDiagnostics(sourceFile: SourceFile): Diagnostic[] {
3+
export function computeSuggestionDiagnostics(sourceFile: SourceFile): Diagnostic[] {
44
return sourceFile.commonJsModuleIndicator
55
? [createDiagnosticForNode(sourceFile.commonJsModuleIndicator, Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module)]
66
: emptyArray;

0 commit comments

Comments
 (0)