Skip to content

Commit

Permalink
noImplicitAny as suggestion (#27693)
Browse files Browse the repository at this point in the history 
* noImplicitAny as suggestion

Note that not all noImplicitAny errors turn into suggestions. In
particular,

1. reportErrorsFromWidening does not, because it doesn't log an error
that infer-from-usage fixes, and fixing it would require
otherwise-unnecessary code.
2. auto types do not have implicit any suggestions, because that would
require running control flow in noImplicitAny mode.

* Rename reportImplicitAny+forbid it for non-checkJS

In JS, you only get implicit any errors/suggestions with checkJS or
ts-check turned on.

* Update baselines

* Use isCheckJsEnabledForFile

* Remove noImplicitAny parameter since it's in scope already
  • Loading branch information
@sandersn
sandersn authored Oct 11, 2018
1 parent 99b2718 commit ec0e8cb
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 49 deletions.
62 changes: 29 additions & 33 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4915,9 +4915,7 @@ namespace ts {
}
const widened = getWidenedType(addOptionality(type, definedInMethod && !definedInConstructor));
if (filterType(widened, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) {
if (noImplicitAny) {
reportImplicitAnyError(symbol.valueDeclaration, anyType);
}
reportImplicitAny(symbol.valueDeclaration, anyType);
return anyType;
}
return widened;
Expand Down Expand Up @@ -4992,9 +4990,7 @@ namespace ts {
return result;
}
if (isEmptyArrayLiteralType(type)) {
if (noImplicitAny) {
reportImplicitAnyError(expression, anyArrayType);
}
reportImplicitAny(expression, anyArrayType);
return anyArrayType;
}
return type;
Expand Down Expand Up @@ -5044,8 +5040,8 @@ namespace ts {
if (isBindingPattern(element.name)) {
return getTypeFromBindingPattern(element.name, includePatternInType, reportErrors);
}
if (reportErrors && noImplicitAny && !declarationBelongsToPrivateAmbientMember(element)) {
reportImplicitAnyError(element, anyType);
if (reportErrors && !declarationBelongsToPrivateAmbientMember(element)) {
reportImplicitAny(element, anyType);
}
return anyType;
}
Expand Down Expand Up @@ -5145,9 +5141,9 @@ namespace ts {
type = isParameter(declaration) && declaration.dotDotDotToken ? anyArrayType : anyType;

// Report implicit any errors unless this is a private property within an ambient declaration
if (reportErrors && noImplicitAny) {
if (reportErrors) {
if (!declarationBelongsToPrivateAmbientMember(declaration)) {
reportImplicitAnyError(declaration, type);
reportImplicitAny(declaration, type);
}
}
return type;
Expand Down Expand Up @@ -5328,14 +5324,12 @@ namespace ts {
}
// Otherwise, fall back to 'any'.
else {
if (noImplicitAny) {
if (setter) {
error(setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
}
else {
Debug.assert(!!getter, "there must existed getter as we are current checking either setter or getter in this function");
error(getter, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
}
if (setter) {
errorOrSuggestion(noImplicitAny, setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_parameter_type_annotation, symbolToString(symbol));
}
else {
Debug.assert(!!getter, "there must existed getter as we are current checking either setter or getter in this function");
errorOrSuggestion(noImplicitAny, getter!, Diagnostics.Property_0_implicitly_has_type_any_because_its_get_accessor_lacks_a_return_type_annotation, symbolToString(symbol));
}
type = anyType;
}
Expand Down Expand Up @@ -13282,8 +13276,12 @@ namespace ts {
return errorReported;
}

function reportImplicitAnyError(declaration: Declaration, type: Type) {
function reportImplicitAny(declaration: Declaration, type: Type) {
const typeAsString = typeToString(getWidenedType(type));
if (isInJSFile(declaration) && !isCheckJsEnabledForFile(getSourceFileOfNode(declaration), compilerOptions)) {
// Only report implicit any errors/suggestions in TS and ts-check JS files
return;
}
let diagnostic: DiagnosticMessage;
switch (declaration.kind) {
case SyntaxKind.BinaryExpression:
Expand All @@ -13306,26 +13304,28 @@ namespace ts {
case SyntaxKind.SetAccessor:
case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
if (!(declaration as NamedDeclaration).name) {
if (noImplicitAny && !(declaration as NamedDeclaration).name) {
error(declaration, Diagnostics.Function_expression_which_lacks_return_type_annotation_implicitly_has_an_0_return_type, typeAsString);
return;
}
diagnostic = Diagnostics._0_which_lacks_return_type_annotation_implicitly_has_an_1_return_type;
break;
case SyntaxKind.MappedType:
error(declaration, Diagnostics.Mapped_object_type_implicitly_has_an_any_template_type);
if (noImplicitAny) {
error(declaration, Diagnostics.Mapped_object_type_implicitly_has_an_any_template_type);
}
return;
default:
diagnostic = Diagnostics.Variable_0_implicitly_has_an_1_type;
}
error(declaration, diagnostic, declarationNameToString(getNameOfDeclaration(declaration)), typeAsString);
errorOrSuggestion(noImplicitAny, declaration, diagnostic, declarationNameToString(getNameOfDeclaration(declaration)), typeAsString);
}

function reportErrorsFromWidening(declaration: Declaration, type: Type) {
if (produceDiagnostics && noImplicitAny && type.flags & TypeFlags.ContainsWideningType) {
// Report implicit any error within type if possible, otherwise report error on declaration
if (!reportWideningErrorsInType(type)) {
reportImplicitAnyError(declaration, type);
reportImplicitAny(declaration, type);
}
}
}
Expand Down Expand Up @@ -22319,15 +22319,11 @@ namespace ts {
isTypeAssertion(initializer) ? type : getWidenedLiteralType(type);
if (isInJSFile(declaration)) {
if (widened.flags & TypeFlags.Nullable) {
if (noImplicitAny) {
reportImplicitAnyError(declaration, anyType);
}
reportImplicitAny(declaration, anyType);
return anyType;
}
else if (isEmptyArrayLiteralType(widened)) {
if (noImplicitAny) {
reportImplicitAnyError(declaration, anyArrayType);
}
reportImplicitAny(declaration, anyArrayType);
return anyArrayType;
}
}
Expand Down Expand Up @@ -23318,8 +23314,8 @@ namespace ts {
checkSourceElement(node.typeParameter);
checkSourceElement(node.type);

if (noImplicitAny && !node.type) {
reportImplicitAnyError(node, anyType);
if (!node.type) {
reportImplicitAny(node, anyType);
}

const type = <MappedType>getTypeFromMappedTypeNode(node);
Expand Down Expand Up @@ -24343,8 +24339,8 @@ namespace ts {
if (produceDiagnostics && !getEffectiveReturnTypeNode(node)) {
// Report an implicit any error if there is no body, no explicit return type, and node is not a private method
// in an ambient context
if (noImplicitAny && nodeIsMissing(body) && !isPrivateWithinAmbient(node)) {
reportImplicitAnyError(node, anyType);
if (nodeIsMissing(body) && !isPrivateWithinAmbient(node)) {
reportImplicitAny(node, anyType);
}

if (functionFlags & FunctionFlags.Generator && nodeIsPresent(body)) {
Expand Down
2 changes: 1 addition & 1 deletion src/testRunner/unittests/tsserverProjectSystem.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4981,7 +4981,7 @@ namespace ts.projectSystem {
checkErrorMessage(session, "suggestionDiag", {
file: file.path,
diagnostics: [
createDiagnostic({ line: 1, offset: 12 }, { line: 1, offset: 13 }, Diagnostics._0_is_declared_but_its_value_is_never_read, ["p"], "suggestion", /*reportsUnnecssary*/ true)
createDiagnostic({ line: 1, offset: 12 }, { line: 1, offset: 13 }, Diagnostics._0_is_declared_but_its_value_is_never_read, ["p"], "suggestion", /*reportsUnnecessary*/ true),
],
});
checkCompleteEvent(session, 2, expectedSequenceId);
Expand Down
7 changes: 3 additions & 4 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc1.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@
/////** @type {number} */
////var [|x|];

verify.getSuggestionDiagnostics([{
message: "JSDoc types may be moved to TypeScript types.",
code: 80004,
}]);
verify.getSuggestionDiagnostics([
{ message: "JSDoc types may be moved to TypeScript types.", code: 80004 },
{ message: "Variable 'x' implicitly has an 'any' type.", code: 7005 }]);

verify.codeFix({
description: "Annotate with type from JSDoc",
Expand Down
13 changes: 8 additions & 5 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc3.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@
//// * @param alpha - the other best parameter
//// * @param {*} beta - I have no idea how this got here
//// */
////function [|f|](x, y, z: string, alpha, beta) {
////function [|f|]([|x|], [|y|], z: string, [|alpha|], [|beta|]) {
//// x; y; z; alpha; beta;
////}

verify.getSuggestionDiagnostics([{
message: "JSDoc types may be moved to TypeScript types.",
code: 80004,
}]);
const [r0, r1, r2, r3, r4] = test.ranges();
verify.getSuggestionDiagnostics([
{message: "JSDoc types may be moved to TypeScript types.", code: 80004, range: r0},
{message: "Parameter 'x' implicitly has an 'any' type.", code: 7006, range: r1 },
{message: "Parameter 'y' implicitly has an 'any' type.", code: 7006, range: r2 },
{message: "Parameter 'alpha' implicitly has an 'any' type.", code: 7006, range: r3 },
{message: "Parameter 'beta' implicitly has an 'any' type.", code: 7006, range: r4 }]);

verify.codeFix({
description: "Annotate with type from JSDoc",
Expand Down
5 changes: 5 additions & 0 deletions tests/cases/fourslash/codeFixUnusedIdentifier_suggestion.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@

const [r0, r1] = test.ranges();
verify.getSuggestionDiagnostics([
{
message: "Parameter 'p' implicitly has an 'any' type.",
range: r0,
code: 7006,
},
{
message: "'p' is declared but its value is never read.",
range: r0,
Expand Down
11 changes: 10 additions & 1 deletion tests/cases/fourslash/noSuggestionDiagnosticsOnParseError.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,13 @@
////export {};
////const a = 1 d;

verify.getSuggestionDiagnostics([]);
// Only give suggestions for nodes that do NOT have parse errors
verify.getSuggestionDiagnostics([{
message: "Variable 'd' implicitly has an 'any' type.",
code: 7005,
range: {
fileName: "/a.ts",
pos: 23,
end: 24,
}
}]);
8 changes: 4 additions & 4 deletions tests/cases/fourslash/refactorConvertToEs6Module_export_named.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
////exports.a3 = x => { x; };
////exports.a4 = x => x;

verify.getSuggestionDiagnostics([{
message: "File is a CommonJS module; it may be converted to an ES6 module.",
code: 80001,
}]);
const [r0, r1, r2] = test.ranges();
verify.getSuggestionDiagnostics([
{ message: "File is a CommonJS module; it may be converted to an ES6 module.", code: 80001, range: r0 },
]);

verify.codeFix({
description: "Convert to ES6 module",
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/unusedLocalsInFunction2.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
//// x+1;
////}

verify.rangeAfterCodeFix("var x;");
verify.rangeAfterCodeFix("var x;", undefined, undefined, 0);

0 comments on commit ec0e8cb

Please sign in to comment.