Skip to content
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

When --noUnusedLocals/--noUnusedParameters is disabled, add suggestions instead of errors #22361

Merged
12 commits merged into from
Apr 5, 2018
272 changes: 151 additions & 121 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,7 @@ namespace ts {
messageText: text,
category: message.category,
code: message.code,
reportsUnused: message.unused,
};
}

Expand Down Expand Up @@ -1652,7 +1653,8 @@ namespace ts {

messageText: text,
category: message.category,
code: message.code
code: message.code,
reportsUnused: message.unused,
};
}

Expand All @@ -1664,7 +1666,7 @@ namespace ts {

code: chain.code,
category: chain.category,
messageText: chain.next ? chain : chain.messageText
messageText: chain.next ? chain : chain.messageText,
};
}

Expand Down
12 changes: 8 additions & 4 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3250,7 +3250,8 @@
},
"'{0}' is declared but its value is never read.": {
"category": "Error",
"code": 6133
"code": 6133,
"unused": true
},
"Report errors on unused locals.": {
"category": "Message",
Expand All @@ -3270,7 +3271,8 @@
},
"Property '{0}' is declared but its value is never read.": {
"category": "Error",
"code": 6138
"code": 6138,
"unused": true
},
"Import emit helpers from 'tslib'.": {
"category": "Message",
Expand Down Expand Up @@ -3482,7 +3484,8 @@
},
"All imports in import declaration are unused.": {
"category": "Error",
"code": 6192
"code": 6192,
"unused": true
},
"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
Expand Down Expand Up @@ -3562,7 +3565,8 @@
},
"Unused label.": {
"category": "Error",
"code": 7028
"code": 7028,
"unused": true
},
"Fallthrough case in switch.": {
"category": "Error",
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4040,6 +4040,7 @@ namespace ts {
category: DiagnosticCategory;
code: number;
message: string;
unused?: {};
}

/**
Expand All @@ -4061,6 +4062,8 @@ namespace ts {
length: number | undefined;
messageText: string | DiagnosticMessageChain;
category: DiagnosticCategory;
/** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
reportsUnused?: {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. why not call both this property and the one on DiagnosticMessage isUnused or isUnusedDeclaration

@amcasey what does Roslyn call this property?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're asking about WellKnownDiagnosticTags.Unnecessary

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit weird ... but sure.

code: number;
source?: string;
}
Expand Down
2 changes: 1 addition & 1 deletion src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2567,7 +2567,7 @@ Actual: ${stringify(fullActual)}`);
}
const range = ts.first(ranges);

const codeFixes = this.getCodeFixes(fileName, errorCode, preferences);
const codeFixes = this.getCodeFixes(fileName, errorCode, preferences).filter(f => f.fixId === undefined); // TODO: GH#20315 filter out those that use the import fix ID;

if (codeFixes.length === 0) {
if (expectedTextArray.length !== 0) {
Expand Down
3 changes: 2 additions & 1 deletion src/harness/unittests/matchFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,14 @@ namespace ts {
}
{
const actual = parseJsonConfigFileContent(json, host, basePath, existingOptions, configFileName, resolutionStack);
expected.errors = expected.errors.map<Diagnostic>(error => ({
expected.errors = expected.errors.map((error): Diagnostic => ({
category: error.category,
code: error.code,
file: undefined,
length: undefined,
messageText: error.messageText,
start: undefined,
reportsUnused: undefined,
}));
assertParsed(actual, expected);
}
Expand Down
2 changes: 1 addition & 1 deletion src/harness/unittests/tsserverProjectSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3958,7 +3958,7 @@ namespace ts.projectSystem {
const folderPath = "/a/b/projects/temp";
const file1: FileOrFolder = {
path: `${folderPath}/a.ts`,
content: 'import f = require("pad")'
content: 'import f = require("pad"); f;'
};
const files = [file1, libFile];
const host = createServerHost(files);
Expand Down
7 changes: 4 additions & 3 deletions src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,11 @@ namespace ts.server {
return this.getDiagnostics(file, CommandNames.SuggestionDiagnosticsSync);
}

private getDiagnostics(file: string, command: CommandNames) {
private getDiagnostics(file: string, command: CommandNames): Diagnostic[] {
const request = this.processRequest<protocol.SyntacticDiagnosticsSyncRequest | protocol.SemanticDiagnosticsSyncRequest | protocol.SuggestionDiagnosticsSyncRequest>(command, { file, includeLinePosition: true });
const response = this.processResponse<protocol.SyntacticDiagnosticsSyncResponse | protocol.SemanticDiagnosticsSyncResponse | protocol.SuggestionDiagnosticsSyncResponse>(request);

return (<protocol.DiagnosticWithLinePosition[]>response.body).map(entry => {
return (<protocol.DiagnosticWithLinePosition[]>response.body).map((entry): Diagnostic => {
const category = firstDefined(Object.keys(DiagnosticCategory), id =>
isString(id) && entry.category === id.toLowerCase() ? (<any>DiagnosticCategory)[id] : undefined);
return {
Expand All @@ -366,7 +366,8 @@ namespace ts.server {
length: entry.length,
messageText: entry.message,
category: Debug.assertDefined(category, "convertDiagnostic: category should not be undefined"),
code: entry.code
code: entry.code,
reportsUnused: entry.reportsUnused,
};
});
}
Expand Down
2 changes: 2 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,8 @@ namespace ts.server.protocol {
endLocation: Location;
category: string;
code: number;
/** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
reportsUnused?: {};
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/services/shims.ts
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,7 @@ namespace ts {
length: number;
category: string;
code: number;
unused?: {};
}
export function realizeDiagnostics(diagnostics: ReadonlyArray<Diagnostic>, newLine: string): RealizedDiagnostic[] {
return diagnostics.map(d => realizeDiagnostic(d, newLine));
Expand Down
5 changes: 5 additions & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2260,6 +2260,7 @@ declare namespace ts {
category: DiagnosticCategory;
code: number;
message: string;
unused?: {};
}
/**
* A linked list of formatted diagnostic messages to be used as part of a multiline message.
Expand All @@ -2279,6 +2280,8 @@ declare namespace ts {
length: number | undefined;
messageText: string | DiagnosticMessageChain;
category: DiagnosticCategory;
/** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
reportsUnused?: {};
code: number;
source?: string;
}
Expand Down Expand Up @@ -5365,6 +5368,8 @@ declare namespace ts.server.protocol {
endLocation: Location;
category: string;
code: number;
/** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
reportsUnused?: {};
}
/**
* Response message for "projectInfo" request
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2260,6 +2260,7 @@ declare namespace ts {
category: DiagnosticCategory;
code: number;
message: string;
unused?: {};
}
/**
* A linked list of formatted diagnostic messages to be used as part of a multiline message.
Expand All @@ -2279,6 +2280,8 @@ declare namespace ts {
length: number | undefined;
messageText: string | DiagnosticMessageChain;
category: DiagnosticCategory;
/** May store more in future. For now, this will simply be `true` to indicate when a diagnostic is an unused-identifier diagnostic. */
reportsUnused?: {};
code: number;
source?: string;
}
Expand Down
2 changes: 2 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc12.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//// * @return {...*}
//// */
//// m(x) {
//// return [x];
//// }
////}

Expand All @@ -16,6 +17,7 @@ verify.codeFix({
* @return {...*}
*/
m(x): any[] {
return [x];
}
}`,
});
2 changes: 2 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc15.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
//// * @param {promise<String>} zeta
//// */
////function f(x, y, z, alpha, beta, gamma, delta, epsilon, zeta) {
//// x; y; z; alpha; beta; gamma; delta; epsilon; zeta;
////}

verify.codeFix({
Expand All @@ -29,5 +30,6 @@ verify.codeFix({
* @param {promise<String>} zeta
*/
function f(x: boolean, y: string, z: number, alpha: object, beta: Date, gamma: Promise<any>, delta: Array<any>, epsilon: Array<number>, zeta: Promise<string>) {
x; y; z; alpha; beta; gamma; delta; epsilon; zeta;
}`,
});
4 changes: 2 additions & 2 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc16.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/// <reference path='fourslash.ts' />
// @strict: true
/////** @type {function(*, ...number, ...boolean): void} */
////var x = (x, ys, ...zs) => { };
////var x = (x, ys, ...zs) => { x; ys; zs; };

verify.codeFix({
description: "Annotate with type from JSDoc",
index: 0,
newFileContent:
`/** @type {function(*, ...number, ...boolean): void} */
var x: (arg0: any, arg1: number[], ...rest: boolean[]) => void = (x, ys, ...zs) => { };`,
var x: (arg0: any, arg1: number[], ...rest: boolean[]) => void = (x, ys, ...zs) => { x; ys; zs; };`,
});
4 changes: 2 additions & 2 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc17.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//// /**
//// * @param {number} x - the first parameter
//// */
//// constructor(x) {
//// constructor(readonly x) {
//// }
////}

Expand All @@ -14,7 +14,7 @@ verify.codeFix({
/**
* @param {number} x - the first parameter
*/
constructor(x: number) {
constructor(readonly x: number) {
}
}`,
});
4 changes: 2 additions & 2 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc18.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/// <reference path='fourslash.ts' />
////class C {
//// /** @param {number} value */
//// set c(value) { return 12 }
//// set c(value) { return value }
////}

verify.codeFix({
description: "Annotate with type from JSDoc",
newFileContent:
`class C {
/** @param {number} value */
set c(value: number) { return 12 }
set c(value: number) { return value }
}`,
});
2 changes: 2 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc19.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//// * @param {T} b
//// */
////function f(a, b) {
//// return a || b;
////}

verify.codeFix({
Expand All @@ -17,5 +18,6 @@ verify.codeFix({
* @param {T} b
*/
function f<T>(a: number, b: T) {
return a || b;
}`,
});
3 changes: 2 additions & 1 deletion tests/cases/fourslash/annotateWithTypeFromJSDoc20.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
//// * @param {number} a
//// * @param {T} b
//// */
////function /*1*/f<T>(a, b) {
////function f<T>(a, b) {
////}

verify.codeFix({
description: "Annotate with type from JSDoc",
errorCode: 80004, // ignore 'unused T'
newFileContent:
`/**
* @param {number} a
Expand Down
19 changes: 12 additions & 7 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc21.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,35 @@
//// * @return {number}
//// */
////function [|f|](x, y) {
//// return x + y;
////}
////
/////**
//// * @return {number}
//// */
////function g(x, y): number {
//// return 0;
//// return x + y;
////}
/////**
//// * @param {number} x
//// */
////function h(x: number, y): number {
//// return 0;
//// return x + y;
////}
////
/////**
//// * @param {number} x
//// * @param {string} y
//// */
////function i(x: number, y: string) {
//// return x + y;
////}
/////**
//// * @param {number} x
//// * @return {boolean}
//// */
////function j(x: number, y): boolean {
//// return true;
//// return x < y;
////}

// Only first location triggers a suggestion
Expand All @@ -41,37 +43,40 @@ verify.getSuggestionDiagnostics([{

verify.codeFix({
description: "Annotate with type from JSDoc",
newFileContent:
errorCode: 80004,
newFileContent:
`/**
* @return {number}
*/
function f(x, y): number {
return x + y;
}

/**
* @return {number}
*/
function g(x, y): number {
return 0;
return x + y;
}
/**
* @param {number} x
*/
function h(x: number, y): number {
return 0;
return x + y;
}

/**
* @param {number} x
* @param {string} y
*/
function i(x: number, y: string) {
return x + y;
}
/**
* @param {number} x
* @return {boolean}
*/
function j(x: number, y): boolean {
return true;
return x < y;
}`,
});
Loading