Skip to content

Issue "Cannot find name did-you-mean" errors as suggestions in plain JS #44271

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 18 commits into from
Jun 15, 2021
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
4 changes: 2 additions & 2 deletions src/compiler/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ namespace ts {
const cachedDiagnostics = state.semanticDiagnosticsPerFile.get(path);
// Report the bind and check diagnostics from the cache if we already have those diagnostics present
if (cachedDiagnostics) {
return filterSemanticDiagnotics(cachedDiagnostics, state.compilerOptions);
Copy link
Member Author

Choose a reason for hiding this comment

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

driveby typo fix

return filterSemanticDiagnostics(cachedDiagnostics, state.compilerOptions);
}
}

Expand All @@ -702,7 +702,7 @@ namespace ts {
if (state.semanticDiagnosticsPerFile) {
state.semanticDiagnosticsPerFile.set(path, diagnostics);
}
return filterSemanticDiagnotics(diagnostics, state.compilerOptions);
return filterSemanticDiagnostics(diagnostics, state.compilerOptions);
}

export type ProgramBuildInfoFileId = number & { __programBuildInfoFileIdBrand: any };
Expand Down
63 changes: 46 additions & 17 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1076,15 +1076,19 @@ namespace ts {
return diagnostic;
}

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

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

function addErrorOrSuggestion(isError: boolean, diagnostic: DiagnosticWithLocation) {
function addErrorOrSuggestion(isError: boolean, diagnostic: Diagnostic) {
if (isError) {
diagnostics.add(diagnostic);
}
Expand Down Expand Up @@ -1708,8 +1712,8 @@ namespace ts {
nameArg: __String | Identifier | undefined,
isUse: boolean,
excludeGlobals = false,
suggestedNameNotFoundMessage?: DiagnosticMessage): Symbol | undefined {
return resolveNameHelper(location, name, meaning, nameNotFoundMessage, nameArg, isUse, excludeGlobals, getSymbol, suggestedNameNotFoundMessage);
issueSuggestions?: boolean): Symbol | undefined {
return resolveNameHelper(location, name, meaning, nameNotFoundMessage, nameArg, isUse, excludeGlobals, getSymbol, issueSuggestions);
}

function resolveNameHelper(
Expand All @@ -1720,8 +1724,7 @@ namespace ts {
nameArg: __String | Identifier | undefined,
isUse: boolean,
excludeGlobals: boolean,
lookup: typeof getSymbol,
suggestedNameNotFoundMessage?: DiagnosticMessage): Symbol | undefined {
lookup: typeof getSymbol, issueSuggestions?: boolean): Symbol | undefined {
const originalLocation = location; // needed for did-you-mean error reporting, which gathers candidates starting from the original location
let result: Symbol | undefined;
let lastLocation: Node | undefined;
Expand Down Expand Up @@ -2058,15 +2061,19 @@ namespace ts {
!checkAndReportErrorForUsingNamespaceModuleAsValue(errorLocation, name, meaning) &&
!checkAndReportErrorForUsingValueAsType(errorLocation, name, meaning)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

driveby upgrade to ?.

let suggestion: Symbol | undefined;
if (suggestedNameNotFoundMessage && suggestionCount < maximumSuggestionCount) {
if (issueSuggestions && suggestionCount < maximumSuggestionCount) {
suggestion = getSuggestedSymbolForNonexistentSymbol(originalLocation, name, meaning);
const isGlobalScopeAugmentationDeclaration = suggestion && suggestion.valueDeclaration && isAmbientModule(suggestion.valueDeclaration) && isGlobalScopeAugmentation(suggestion.valueDeclaration);
const isGlobalScopeAugmentationDeclaration = suggestion?.valueDeclaration && isAmbientModule(suggestion.valueDeclaration) && isGlobalScopeAugmentation(suggestion.valueDeclaration);
if (isGlobalScopeAugmentationDeclaration) {
suggestion = undefined;
}
if (suggestion) {
const suggestionName = symbolToString(suggestion);
const diagnostic = error(errorLocation, suggestedNameNotFoundMessage, diagnosticName(nameArg!), suggestionName);
const isUncheckedJS = isUncheckedJSSuggestion(originalLocation, suggestion, /*excludeClasses*/ false);
const message = isUncheckedJS ? Diagnostics.Could_not_find_name_0_Did_you_mean_1 : Diagnostics.Cannot_find_name_0_Did_you_mean_1;
const diagnostic = createError(errorLocation, message, diagnosticName(nameArg!), suggestionName);
addErrorOrSuggestion(!isUncheckedJS, diagnostic);

if (suggestion.valueDeclaration) {
addRelatedInfo(
diagnostic,
Expand Down Expand Up @@ -21939,7 +21946,7 @@ namespace ts {
node,
!isWriteOnlyAccess(node),
/*excludeGlobals*/ false,
Diagnostics.Cannot_find_name_0_Did_you_mean_1) || unknownSymbol;
/*issueSuggestions*/ true) || unknownSymbol;
}
return links.resolvedSymbol;
}
Expand Down Expand Up @@ -27453,7 +27460,8 @@ namespace ts {
if (!prop) {
const indexInfo = !isPrivateIdentifier(right) && (assignmentKind === AssignmentKind.None || !isGenericObjectType(leftType) || isThisTypeParameter(leftType)) ? getIndexInfoOfType(apparentType, IndexKind.String) : undefined;
if (!(indexInfo && indexInfo.type)) {
if (isJSLiteralType(leftType)) {
const isUncheckedJS = isUncheckedJSSuggestion(node, leftType.symbol, /*excludeClasses*/ true);
if (!isUncheckedJS && isJSLiteralType(leftType)) {
return anyType;
}
if (leftType.symbol === globalThisSymbol) {
Expand All @@ -27466,7 +27474,7 @@ namespace ts {
return anyType;
}
if (right.escapedText && !checkAndReportErrorForExtendingInterface(node)) {
reportNonexistentProperty(right, isThisTypeParameter(leftType) ? apparentType : leftType);
reportNonexistentProperty(right, isThisTypeParameter(leftType) ? apparentType : leftType, isUncheckedJS);
}
return errorType;
}
Expand Down Expand Up @@ -27499,6 +27507,26 @@ namespace ts {
return getFlowTypeOfAccessExpression(node, prop, propType, right, checkMode);
}

/**
* Determines whether a did-you-mean error should be a suggestion in an unchecked JS file.
* Only applies to unchecked JS files without checkJS, // @ts-check or // @ts-nocheck
* It does not suggest when the suggestion:
* - Is from a global file that is different from the reference file, or
* - (optionally) Is a class, or is a this.x property access expression
*/
function isUncheckedJSSuggestion(node: Node | undefined, suggestion: Symbol | undefined, excludeClasses: boolean): boolean {
const file = getSourceFileOfNode(node);
if (file) {
if (compilerOptions.checkJs === undefined && file.checkJsDirective === undefined && (file.scriptKind === ScriptKind.JS || file.scriptKind === ScriptKind.JSX)) {
const declarationFile = forEach(suggestion?.declarations, getSourceFileOfNode);
return !(file !== declarationFile && !!declarationFile && isGlobalSourceFile(declarationFile))
&& !(excludeClasses && suggestion && suggestion.flags & SymbolFlags.Class)
&& !(!!node && excludeClasses && isPropertyAccessExpression(node) && node.expression.kind === SyntaxKind.ThisKeyword);
}
}
return false;
}

function getFlowTypeOfAccessExpression(node: ElementAccessExpression | PropertyAccessExpression | QualifiedName, prop: Symbol | undefined, propType: Type, errorNode: Node, checkMode: CheckMode | undefined) {
// Only compute control flow type if this is a property access expression that isn't an
// assignment target, and the referenced property was declared as a variable, property,
Expand Down Expand Up @@ -27630,7 +27658,7 @@ namespace ts {
return getIntersectionType(x);
}

function reportNonexistentProperty(propNode: Identifier | PrivateIdentifier, containingType: Type) {
function reportNonexistentProperty(propNode: Identifier | PrivateIdentifier, containingType: Type, isUncheckedJS: boolean) {
let errorInfo: DiagnosticMessageChain | undefined;
let relatedInfo: Diagnostic | undefined;
if (!isPrivateIdentifier(propNode) && containingType.flags & TypeFlags.Union && !(containingType.flags & TypeFlags.Primitive)) {
Expand Down Expand Up @@ -27663,7 +27691,8 @@ namespace ts {
const suggestion = getSuggestedSymbolForNonexistentProperty(propNode, containingType);
if (suggestion !== undefined) {
const suggestedName = symbolName(suggestion);
errorInfo = chainDiagnosticMessages(errorInfo, Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2, missingProperty, container, suggestedName);
const message = isUncheckedJS ? Diagnostics.Property_0_may_not_exist_on_type_1_Did_you_mean_2 : Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2;
errorInfo = chainDiagnosticMessages(errorInfo, message, missingProperty, container, suggestedName);
relatedInfo = suggestion.valueDeclaration && createDiagnosticForNode(suggestion.valueDeclaration, Diagnostics._0_is_declared_here, suggestedName);
}
else {
Expand All @@ -27679,7 +27708,7 @@ namespace ts {
if (relatedInfo) {
addRelatedInfo(resultDiagnostic, relatedInfo);
}
diagnostics.add(resultDiagnostic);
addErrorOrSuggestion(!isUncheckedJS, resultDiagnostic);
}

function containerSeemsToBeEmptyDomElement(containingType: Type) {
Expand Down Expand Up @@ -34594,7 +34623,7 @@ namespace ts {

const rootName = getFirstIdentifier(typeName);
const meaning = (typeName.kind === SyntaxKind.Identifier ? SymbolFlags.Type : SymbolFlags.Namespace) | SymbolFlags.Alias;
const rootSymbol = resolveName(rootName, rootName.escapedText, meaning, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined, /*isRefernce*/ true);
const rootSymbol = resolveName(rootName, rootName.escapedText, meaning, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined, /*isReference*/ true);
if (rootSymbol
&& rootSymbol.flags & SymbolFlags.Alias
&& symbolIsValue(rootSymbol)
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2442,10 +2442,18 @@
"category": "Error",
"code": 2567
},
"Property '{0}' may not exist on type '{1}'. Did you mean '{2}'?": {
"category": "Error",
"code": 2568
},
"Type '{0}' is not an array type or a string type. Use compiler option '--downlevelIteration' to allow iterating of iterators.": {
"category": "Error",
"code": 2569
},
"Could not find name '{0}'. Did you mean '{1}'?": {
"category": "Error",
"code": 2570
},
"Object is of type 'unknown'.": {
"category": "Error",
"code": 2571
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1896,7 +1896,7 @@ namespace ts {

function getSemanticDiagnosticsForFile(sourceFile: SourceFile, cancellationToken: CancellationToken | undefined): readonly Diagnostic[] {
return concatenate(
filterSemanticDiagnotics(getBindAndCheckDiagnosticsForFile(sourceFile, cancellationToken), options),
filterSemanticDiagnostics(getBindAndCheckDiagnosticsForFile(sourceFile, cancellationToken), options),
getProgramDiagnostics(sourceFile)
);
}
Expand All @@ -1918,8 +1918,8 @@ namespace ts {
const isCheckJs = isCheckJsEnabledForFile(sourceFile, options);
const isTsNoCheck = !!sourceFile.checkJsDirective && sourceFile.checkJsDirective.enabled === false;
// By default, only type-check .ts, .tsx, 'Deferred' and 'External' files (external files are added by plugins)
const includeBindAndCheckDiagnostics = !isTsNoCheck && (sourceFile.scriptKind === ScriptKind.TS || sourceFile.scriptKind === ScriptKind.TSX ||
sourceFile.scriptKind === ScriptKind.External || isCheckJs || sourceFile.scriptKind === ScriptKind.Deferred);
const includeBindAndCheckDiagnostics = !isTsNoCheck && (sourceFile.scriptKind === ScriptKind.TS || sourceFile.scriptKind === ScriptKind.TSX
|| sourceFile.scriptKind === ScriptKind.External || isCheckJs || sourceFile.scriptKind === ScriptKind.Deferred);
const bindDiagnostics: readonly Diagnostic[] = includeBindAndCheckDiagnostics ? sourceFile.bindDiagnostics : emptyArray;
const checkDiagnostics = includeBindAndCheckDiagnostics ? typeChecker.getDiagnostics(sourceFile, cancellationToken) : emptyArray;

Expand Down Expand Up @@ -3891,7 +3891,7 @@ namespace ts {
}

/*@internal*/
export function filterSemanticDiagnotics(diagnostic: readonly Diagnostic[], option: CompilerOptions): readonly Diagnostic[] {
export function filterSemanticDiagnostics(diagnostic: readonly Diagnostic[], option: CompilerOptions): readonly Diagnostic[] {
return filter(diagnostic, d => !d.skippedOn || !option[d.skippedOn]);
}

Expand Down
2 changes: 2 additions & 0 deletions src/services/codefixes/fixSpelling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ namespace ts.codefix {
const fixId = "fixSpelling";
const errorCodes = [
Diagnostics.Property_0_does_not_exist_on_type_1_Did_you_mean_2.code,
Diagnostics.Property_0_may_not_exist_on_type_1_Did_you_mean_2.code,
Diagnostics.Cannot_find_name_0_Did_you_mean_1.code,
Diagnostics.Could_not_find_name_0_Did_you_mean_1.code,
Diagnostics.Cannot_find_name_0_Did_you_mean_the_instance_member_this_0.code,
Diagnostics.Cannot_find_name_0_Did_you_mean_the_static_member_1_0.code,
Diagnostics._0_has_no_exported_member_named_1_Did_you_mean_2.code,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ class B extends A {
* @type object
*/
this.bar = super.arguments.foo;
>this.bar = super.arguments.foo : any
>this.bar = super.arguments.foo : error
>this.bar : any
>this : this
>bar : any
>super.arguments.foo : any
>super.arguments.foo : error
>super.arguments : { bar: {}; }
>super : A
>arguments : { bar: {}; }
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/jsObjectsMarkedAsOpenEnded.types
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class C {

this.member.a = 0;
>this.member.a = 0 : 0
>this.member.a : any
>this.member.a : error
>this.member : {}
>this : this
>member : {}
Expand All @@ -48,7 +48,7 @@ var obj = {

obj.property.a = 0;
>obj.property.a = 0 : 0
>obj.property.a : any
>obj.property.a : error
>obj.property : {}
>obj : { property: {}; }
>property : {}
Expand Down
95 changes: 95 additions & 0 deletions tests/baselines/reference/spellingUncheckedJS.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
=== tests/cases/conformance/salsa/spellingUncheckedJS.js ===
export var inModule = 1
>inModule : Symbol(inModule, Decl(spellingUncheckedJS.js, 0, 10))

inmodule.toFixed()

function f() {
>f : Symbol(f, Decl(spellingUncheckedJS.js, 1, 18))

var locals = 2 + true
>locals : Symbol(locals, Decl(spellingUncheckedJS.js, 4, 7))

locale.toFixed()
// @ts-expect-error
localf.toExponential()
// @ts-expect-error
"this is fine"
}
class Classe {
>Classe : Symbol(Classe, Decl(spellingUncheckedJS.js, 10, 1))

non = 'oui'
>non : Symbol(Classe.non, Decl(spellingUncheckedJS.js, 11, 14))

methode() {
>methode : Symbol(Classe.methode, Decl(spellingUncheckedJS.js, 12, 15))

// no error on 'this' references
return this.none
>this : Symbol(Classe, Decl(spellingUncheckedJS.js, 10, 1))
}
}
class Derivee extends Classe {
>Derivee : Symbol(Derivee, Decl(spellingUncheckedJS.js, 17, 1))
>Classe : Symbol(Classe, Decl(spellingUncheckedJS.js, 10, 1))

methode() {
>methode : Symbol(Derivee.methode, Decl(spellingUncheckedJS.js, 18, 30))

// no error on 'super' references
return super.none
>super : Symbol(Classe, Decl(spellingUncheckedJS.js, 10, 1))
}
}


var object = {
>object : Symbol(object, Decl(spellingUncheckedJS.js, 26, 3), Decl(spellingUncheckedJS.js, 29, 15), Decl(spellingUncheckedJS.js, 30, 18))

spaaace: 3
>spaaace : Symbol(spaaace, Decl(spellingUncheckedJS.js, 26, 14))
}
object.spaaaace // error on read
>object : Symbol(object, Decl(spellingUncheckedJS.js, 26, 3), Decl(spellingUncheckedJS.js, 29, 15), Decl(spellingUncheckedJS.js, 30, 18))

object.spaace = 12 // error on write
>object : Symbol(object, Decl(spellingUncheckedJS.js, 26, 3), Decl(spellingUncheckedJS.js, 29, 15), Decl(spellingUncheckedJS.js, 30, 18))

object.fresh = 12 // OK
>object : Symbol(object, Decl(spellingUncheckedJS.js, 26, 3), Decl(spellingUncheckedJS.js, 29, 15), Decl(spellingUncheckedJS.js, 30, 18))

other.puuuce // OK, from another file
>other : Symbol(other, Decl(other.js, 3, 3))

new Date().getGMTDate() // OK, from another file
>Date : Symbol(Date, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.scripthost.d.ts, --, --))

// No suggestions for globals from other files
const atoc = setIntegral(() => console.log('ok'), 500)
>atoc : Symbol(atoc, Decl(spellingUncheckedJS.js, 36, 5))
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))

AudioBuffin // etc
Jimmy
>Jimmy : Symbol(Jimmy, Decl(other.js, 0, 3))

Jon

=== tests/cases/conformance/salsa/other.js ===
var Jimmy = 1
>Jimmy : Symbol(Jimmy, Decl(other.js, 0, 3))

var John = 2
>John : Symbol(John, Decl(other.js, 1, 3))

Jon // error, it's from the same file
var other = {
>other : Symbol(other, Decl(other.js, 3, 3))

puuce: 4
>puuce : Symbol(puuce, Decl(other.js, 3, 13))
}

Loading