Skip to content

Introduce related spans into tsserver protocol #24548

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

Merged
merged 3 commits into from
Jun 15, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 23 additions & 18 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -833,11 +833,12 @@ namespace ts {
return emitResolver;
}

function error(location: Node | undefined, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number, arg3?: string | number): void {
function error(location: Node | undefined, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number, arg3?: string | number): Diagnostic {
const diagnostic = location
? createDiagnosticForNode(location, message, arg0, arg1, arg2, arg3)
: createCompilerDiagnostic(message, arg0, arg1, arg2, arg3);
diagnostics.add(diagnostic);
return diagnostic;
}

function addErrorOrSuggestion(isError: boolean, diagnostic: DiagnosticWithLocation) {
Expand Down Expand Up @@ -10491,18 +10492,21 @@ namespace ts {
}
}

diagnostics.add(createDiagnosticForNodeFromMessageChain(errorNode!, errorInfo)); // TODO: GH#18217
}
// Check if we should issue an extra diagnostic to produce a quickfix for a slightly incorrect import statement
if (headMessage && errorNode && !result && source.symbol) {
const links = getSymbolLinks(source.symbol);
if (links.originatingImport && !isImportCall(links.originatingImport)) {
const helpfulRetry = checkTypeRelatedTo(getTypeOfSymbol(links.target!), target, relation, /*errorNode*/ undefined);
if (helpfulRetry) {
// Likely an incorrect import. Issue a helpful diagnostic to produce a quickfix to change the import
diagnostics.add(createDiagnosticForNode(links.originatingImport, Diagnostics.A_namespace_style_import_cannot_be_called_or_constructed_and_will_cause_a_failure_at_runtime));
let relatedInformation: DiagnosticRelatedInformation[] | undefined;
// Check if we should issue an extra diagnostic to produce a quickfix for a slightly incorrect import statement
if (headMessage && errorNode && !result && source.symbol) {
const links = getSymbolLinks(source.symbol);
if (links.originatingImport && !isImportCall(links.originatingImport)) {
const helpfulRetry = checkTypeRelatedTo(getTypeOfSymbol(links.target!), target, relation, /*errorNode*/ undefined);
if (helpfulRetry) {
// Likely an incorrect import. Issue a helpful diagnostic to produce a quickfix to change the import
const diag = createDiagnosticForNode(links.originatingImport, Diagnostics.Type_originates_at_this_import_A_namespace_style_import_cannot_be_called_or_constructed_and_will_cause_a_failure_at_runtime_Consider_using_a_default_import_or_import_require_here_instead);
relatedInformation = append(relatedInformation, diag); // Cause the error to appear with the error that triggered it
}
}
}

diagnostics.add(createDiagnosticForNodeFromMessageChain(errorNode!, errorInfo, relatedInformation)); // TODO: GH#18217
}
return result !== Ternary.False;

Expand Down Expand Up @@ -18865,14 +18869,13 @@ namespace ts {
}

function invocationError(node: Node, apparentType: Type, kind: SignatureKind) {
error(node, kind === SignatureKind.Call
invocationErrorRecovery(apparentType, kind, error(node, kind === SignatureKind.Call
? Diagnostics.Cannot_invoke_an_expression_whose_type_lacks_a_call_signature_Type_0_has_no_compatible_call_signatures
: Diagnostics.Cannot_use_new_with_an_expression_whose_type_lacks_a_call_or_construct_signature
, typeToString(apparentType));
invocationErrorRecovery(apparentType, kind);
, typeToString(apparentType)));
}

function invocationErrorRecovery(apparentType: Type, kind: SignatureKind) {
function invocationErrorRecovery(apparentType: Type, kind: SignatureKind, diagnostic: Diagnostic) {
if (!apparentType.symbol) {
return;
}
Expand All @@ -18882,7 +18885,8 @@ namespace ts {
if (importNode && !isImportCall(importNode)) {
const sigs = getSignaturesOfType(getTypeOfSymbol(getSymbolLinks(apparentType.symbol).target!), kind);
if (!sigs || !sigs.length) return;
error(importNode, Diagnostics.A_namespace_style_import_cannot_be_called_or_constructed_and_will_cause_a_failure_at_runtime);
diagnostic.relatedInformation = diagnostic.relatedInformation || [];
diagnostic.relatedInformation.push(createDiagnosticForNode(importNode, Diagnostics.Type_originates_at_this_import_A_namespace_style_import_cannot_be_called_or_constructed_and_will_cause_a_failure_at_runtime_Consider_using_a_default_import_or_import_require_here_instead));
}
}

Expand Down Expand Up @@ -18961,8 +18965,9 @@ namespace ts {
if (!callSignatures.length) {
let errorInfo = chainDiagnosticMessages(/*details*/ undefined, Diagnostics.Cannot_invoke_an_expression_whose_type_lacks_a_call_signature_Type_0_has_no_compatible_call_signatures, typeToString(apparentType));
errorInfo = chainDiagnosticMessages(errorInfo, headMessage);
diagnostics.add(createDiagnosticForNodeFromMessageChain(node, errorInfo));
invocationErrorRecovery(apparentType, SignatureKind.Call);
const diag = createDiagnosticForNodeFromMessageChain(node, errorInfo);
diagnostics.add(diag);
invocationErrorRecovery(apparentType, SignatureKind.Call, diag);
return resolveErrorCall(node);
}

Expand Down
145 changes: 85 additions & 60 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ namespace ts {
const gutterSeparator = " ";
const resetEscapeSequence = "\u001b[0m";
const ellipsis = "...";
function getCategoryFormat(category: DiagnosticCategory): string {
const halfIndent = " ";
const indent = " ";
function getCategoryFormat(category: DiagnosticCategory): ForegroundColorEscapeSequences {
switch (category) {
case DiagnosticCategory.Error: return ForegroundColorEscapeSequences.Red;
case DiagnosticCategory.Warning: return ForegroundColorEscapeSequences.Yellow;
Expand All @@ -273,68 +275,79 @@ namespace ts {
return s;
}

function formatCodeSpan(file: SourceFile, start: number, length: number, indent: string, squiggleColor: ForegroundColorEscapeSequences, host: FormatDiagnosticsHost) {
const { line: firstLine, character: firstLineChar } = getLineAndCharacterOfPosition(file, start);
const { line: lastLine, character: lastLineChar } = getLineAndCharacterOfPosition(file, start + length);
const lastLineInFile = getLineAndCharacterOfPosition(file, file.text.length).line;

const hasMoreThanFiveLines = (lastLine - firstLine) >= 4;
let gutterWidth = (lastLine + 1 + "").length;
if (hasMoreThanFiveLines) {
gutterWidth = Math.max(ellipsis.length, gutterWidth);
}

let context = "";
for (let i = firstLine; i <= lastLine; i++) {
context += host.getNewLine();
// If the error spans over 5 lines, we'll only show the first 2 and last 2 lines,
// so we'll skip ahead to the second-to-last line.
if (hasMoreThanFiveLines && firstLine + 1 < i && i < lastLine - 1) {
context += indent + formatColorAndReset(padLeft(ellipsis, gutterWidth), gutterStyleSequence) + gutterSeparator + host.getNewLine();
i = lastLine - 1;
}

const lineStart = getPositionOfLineAndCharacter(file, i, 0);
const lineEnd = i < lastLineInFile ? getPositionOfLineAndCharacter(file, i + 1, 0) : file.text.length;
let lineContent = file.text.slice(lineStart, lineEnd);
lineContent = lineContent.replace(/\s+$/g, ""); // trim from end
lineContent = lineContent.replace("\t", " "); // convert tabs to single spaces

// Output the gutter and the actual contents of the line.
context += indent + formatColorAndReset(padLeft(i + 1 + "", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += lineContent + host.getNewLine();

// Output the gutter and the error span for the line using tildes.
context += indent + formatColorAndReset(padLeft("", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += squiggleColor;
if (i === firstLine) {
// If we're on the last line, then limit it to the last character of the last line.
// Otherwise, we'll just squiggle the rest of the line, giving 'slice' no end position.
const lastCharForLine = i === lastLine ? lastLineChar : undefined;

context += lineContent.slice(0, firstLineChar).replace(/\S/g, " ");
context += lineContent.slice(firstLineChar, lastCharForLine).replace(/./g, "~");
}
else if (i === lastLine) {
context += lineContent.slice(0, lastLineChar).replace(/./g, "~");
}
else {
// Squiggle the entire line.
context += lineContent.replace(/./g, "~");
}
context += resetEscapeSequence;
}
return context;
}

function formatLocation(file: SourceFile, start: number, host: FormatDiagnosticsHost) {
const { line: firstLine, character: firstLineChar } = getLineAndCharacterOfPosition(file, start); // TODO: GH#18217
const relativeFileName = host ? convertToRelativePath(file.fileName, host.getCurrentDirectory(), fileName => host.getCanonicalFileName(fileName)) : file.fileName;

let output = "";
output += formatColorAndReset(relativeFileName, ForegroundColorEscapeSequences.Cyan);
output += ":";
output += formatColorAndReset(`${firstLine + 1}`, ForegroundColorEscapeSequences.Yellow);
output += ":";
output += formatColorAndReset(`${firstLineChar + 1}`, ForegroundColorEscapeSequences.Yellow);
return output;
}

export function formatDiagnosticsWithColorAndContext(diagnostics: ReadonlyArray<Diagnostic>, host: FormatDiagnosticsHost): string {
let output = "";
for (const diagnostic of diagnostics) {
let context = "";
if (diagnostic.file) {
const { start, length, file } = diagnostic;
const { line: firstLine, character: firstLineChar } = getLineAndCharacterOfPosition(file, start!); // TODO: GH#18217
const { line: lastLine, character: lastLineChar } = getLineAndCharacterOfPosition(file, start! + length!);
const lastLineInFile = getLineAndCharacterOfPosition(file, file.text.length).line;
const relativeFileName = host ? convertToRelativePath(file.fileName, host.getCurrentDirectory(), fileName => host.getCanonicalFileName(fileName)) : file.fileName;

const hasMoreThanFiveLines = (lastLine - firstLine) >= 4;
let gutterWidth = (lastLine + 1 + "").length;
if (hasMoreThanFiveLines) {
gutterWidth = Math.max(ellipsis.length, gutterWidth);
}

for (let i = firstLine; i <= lastLine; i++) {
context += host.getNewLine();
// If the error spans over 5 lines, we'll only show the first 2 and last 2 lines,
// so we'll skip ahead to the second-to-last line.
if (hasMoreThanFiveLines && firstLine + 1 < i && i < lastLine - 1) {
context += formatColorAndReset(padLeft(ellipsis, gutterWidth), gutterStyleSequence) + gutterSeparator + host.getNewLine();
i = lastLine - 1;
}

const lineStart = getPositionOfLineAndCharacter(file, i, 0);
const lineEnd = i < lastLineInFile ? getPositionOfLineAndCharacter(file, i + 1, 0) : file.text.length;
let lineContent = file.text.slice(lineStart, lineEnd);
lineContent = lineContent.replace(/\s+$/g, ""); // trim from end
lineContent = lineContent.replace("\t", " "); // convert tabs to single spaces

// Output the gutter and the actual contents of the line.
context += formatColorAndReset(padLeft(i + 1 + "", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += lineContent + host.getNewLine();

// Output the gutter and the error span for the line using tildes.
context += formatColorAndReset(padLeft("", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += ForegroundColorEscapeSequences.Red;
if (i === firstLine) {
// If we're on the last line, then limit it to the last character of the last line.
// Otherwise, we'll just squiggle the rest of the line, giving 'slice' no end position.
const lastCharForLine = i === lastLine ? lastLineChar : undefined;

context += lineContent.slice(0, firstLineChar).replace(/\S/g, " ");
context += lineContent.slice(firstLineChar, lastCharForLine).replace(/./g, "~");
}
else if (i === lastLine) {
context += lineContent.slice(0, lastLineChar).replace(/./g, "~");
}
else {
// Squiggle the entire line.
context += lineContent.replace(/./g, "~");
}
context += resetEscapeSequence;
}

output += formatColorAndReset(relativeFileName, ForegroundColorEscapeSequences.Cyan);
output += ":";
output += formatColorAndReset(`${firstLine + 1}`, ForegroundColorEscapeSequences.Yellow);
output += ":";
output += formatColorAndReset(`${firstLineChar + 1}`, ForegroundColorEscapeSequences.Yellow);
const { file, start } = diagnostic;
output += formatLocation(file, start!, host); // TODO: GH#18217
output += " - ";
}

Expand All @@ -344,7 +357,19 @@ namespace ts {

if (diagnostic.file) {
output += host.getNewLine();
output += context;
output += formatCodeSpan(diagnostic.file, diagnostic.start!, diagnostic.length!, "", getCategoryFormat(diagnostic.category), host); // TODO: GH#18217
if (diagnostic.relatedInformation) {
output += host.getNewLine();
for (const { file, start, length, messageText } of diagnostic.relatedInformation) {
if (file) {
output += host.getNewLine();
output += halfIndent + formatLocation(file, start!, host); // TODO: GH#18217
output += formatCodeSpan(file, start!, length!, indent, ForegroundColorEscapeSequences.Cyan, host); // TODO: GH#18217
}
output += host.getNewLine();
output += indent + flattenDiagnosticMessageText(messageText, host.getNewLine());
}
}
}

output += host.getNewLine();
Expand Down
2 changes: 1 addition & 1 deletion src/parser/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3828,7 +3828,7 @@
"category": "Message",
"code": 7037
},
"A namespace-style import cannot be called or constructed, and will cause a failure at runtime.": {
"Type originates at this import. A namespace-style import cannot be called or constructed, and will cause a failure at runtime. Consider using a default import or import require here instead.": {
"category": "Error",
"code": 7038
},
Expand Down
13 changes: 8 additions & 5 deletions src/parser/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4193,16 +4193,19 @@ namespace ts {
next?: DiagnosticMessageChain;
}

export interface Diagnostic {
file: SourceFile | undefined;
start: number | undefined;
length: number | undefined;
messageText: string | DiagnosticMessageChain;
export interface Diagnostic extends DiagnosticRelatedInformation {
category: DiagnosticCategory;
/** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
reportsUnnecessary?: {};
code: number;
source?: string;
relatedInformation?: DiagnosticRelatedInformation[];
}
export interface DiagnosticRelatedInformation {
file: SourceFile | undefined;
start: number | undefined;
length: number | undefined;
messageText: string | DiagnosticMessageChain;
}
export interface DiagnosticWithLocation extends Diagnostic {
file: SourceFile;
Expand Down
5 changes: 3 additions & 2 deletions src/parser/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ namespace ts {
return createFileDiagnostic(sourceFile, span.start, span.length, message, arg0, arg1, arg2, arg3);
}

export function createDiagnosticForNodeFromMessageChain(node: Node, messageChain: DiagnosticMessageChain): DiagnosticWithLocation {
export function createDiagnosticForNodeFromMessageChain(node: Node, messageChain: DiagnosticMessageChain, relatedInformation?: DiagnosticRelatedInformation[]): DiagnosticWithLocation {
const sourceFile = getSourceFileOfNode(node);
const span = getErrorSpanForNode(sourceFile, node);
return {
Expand All @@ -808,7 +808,8 @@ namespace ts {
length: span.length,
code: messageChain.code,
category: messageChain.category,
messageText: messageChain.next ? messageChain : messageChain.messageText
messageText: messageChain.next ? messageChain : messageChain.messageText,
relatedInformation
};
}

Expand Down
23 changes: 23 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ namespace ts.server.protocol {
code: number;
/** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
reportsUnnecessary?: {};
relatedInformation?: DiagnosticRelatedInformation[];
}

/**
Expand Down Expand Up @@ -2215,6 +2216,11 @@ namespace ts.server.protocol {

reportsUnnecessary?: {};

/**
* Any related spans the diagnostic may have, such as other locations relevant to an error, such as declarartion sites
*/
relatedInformation?: DiagnosticRelatedInformation[];

/**
* The error code of the diagnostic message.
*/
Expand All @@ -2233,6 +2239,23 @@ namespace ts.server.protocol {
fileName: string;
}

/**
* Represents additional spans returned with a diagnostic which are relevant to it
* Like DiagnosticWithLinePosition, this is provided in two forms:
* - start and length of the span
* - startLocation and endLocation a pair of Location objects storing the start/end line offset of the span
*/
export interface DiagnosticRelatedInformation {
/**
* Text of related or additional information.
*/
message: string;
/**
* Associated location
*/
span?: FileSpan;
}

export interface DiagnosticEventBody {
/**
* The file for which diagnostic information is reported.
Expand Down
Loading