Skip to content

Commit b761096

Browse files
billtibillti
committed
Merge pull request #6905 from Microsoft/portJsDocFixes
Port Salsa fixes
2 parents 232b322 + 73a8ace commit b761096

14 files changed

+233
-26
lines changed

‎src/compiler/binder.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,7 +1384,7 @@ namespace ts {
13841384
// Export assignment in some sort of block construct
13851385
bindAnonymousDeclaration(node, SymbolFlags.Alias, getDeclarationName(node));
13861386
}
1387-
else if (boundExpression.kind === SyntaxKind.Identifier) {
1387+
else if (boundExpression.kind === SyntaxKind.Identifier && node.kind === SyntaxKind.ExportAssignment) {
13881388
// An export default clause with an identifier exports all meanings of that identifier
13891389
declareSymbol(container.symbol.exports, container.symbol, node, SymbolFlags.Alias, SymbolFlags.PropertyExcludes | SymbolFlags.AliasExcludes);
13901390
}
@@ -1435,7 +1435,8 @@ namespace ts {
14351435
// Declare a 'member' in case it turns out the container was an ES5 class
14361436
if (container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.FunctionDeclaration) {
14371437
container.symbol.members = container.symbol.members || {};
1438-
declareSymbol(container.symbol.members, container.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes);
1438+
// It's acceptable for multiple 'this' assignments of the same identifier to occur
1439+
declareSymbol(container.symbol.members, container.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property);
14391440
}
14401441
}
14411442

@@ -1444,8 +1445,16 @@ namespace ts {
14441445

14451446
// Look up the function in the local scope, since prototype assignments should
14461447
// follow the function declaration
1447-
const classId = <Identifier>(<PropertyAccessExpression>(<PropertyAccessExpression>node.left).expression).expression;
1448-
const funcSymbol = container.locals[classId.text];
1448+
const leftSideOfAssignment = node.left as PropertyAccessExpression;
1449+
const classPrototype = leftSideOfAssignment.expression as PropertyAccessExpression;
1450+
const constructorFunction = classPrototype.expression as Identifier;
1451+
1452+
// Fix up parent pointers since we're going to use these nodes before we bind into them
1453+
leftSideOfAssignment.parent = node;
1454+
constructorFunction.parent = classPrototype;
1455+
classPrototype.parent = leftSideOfAssignment;
1456+
1457+
const funcSymbol = container.locals[constructorFunction.text];
14491458
if (!funcSymbol || !(funcSymbol.flags & SymbolFlags.Function)) {
14501459
return;
14511460
}
@@ -1456,7 +1465,7 @@ namespace ts {
14561465
}
14571466

14581467
// Declare the method/property
1459-
declareSymbol(funcSymbol.members, funcSymbol, <PropertyAccessExpression>node.left, SymbolFlags.Property, SymbolFlags.PropertyExcludes);
1468+
declareSymbol(funcSymbol.members, funcSymbol, leftSideOfAssignment, SymbolFlags.Property, SymbolFlags.PropertyExcludes);
14601469
}
14611470

14621471
function bindCallExpression(node: CallExpression) {

‎src/compiler/checker.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2845,7 +2845,7 @@ namespace ts {
28452845
}
28462846
// Handle module.exports = expr
28472847
if (declaration.kind === SyntaxKind.BinaryExpression) {
2848-
return links.type = checkExpression((<BinaryExpression>declaration).right);
2848+
return links.type = getUnionType(map(symbol.declarations, (decl: BinaryExpression) => checkExpressionCached(decl.right)));
28492849
}
28502850
if (declaration.kind === SyntaxKind.PropertyAccessExpression) {
28512851
// Declarations only exist for property access expressions for certain
@@ -10356,9 +10356,11 @@ namespace ts {
1035610356

1035710357
function getReturnTypeFromJSDocComment(func: SignatureDeclaration | FunctionDeclaration): Type {
1035810358
const returnTag = getJSDocReturnTag(func);
10359-
if (returnTag) {
10359+
if (returnTag && returnTag.typeExpression) {
1036010360
return getTypeFromTypeNode(returnTag.typeExpression.type);
1036110361
}
10362+
10363+
return undefined;
1036210364
}
1036310365

1036410366
function createPromiseType(promisedType: Type): Type {
@@ -10433,7 +10435,8 @@ namespace ts {
1043310435
}
1043410436
else {
1043510437
error(func, Diagnostics.No_best_common_type_exists_among_return_expressions);
10436-
return unknownType;
10438+
// Defer to unioning the return types so we get a) downstream errors earlier and b) better Salsa experience
10439+
return getUnionType(types);
1043710440
}
1043810441
}
1043910442

‎src/compiler/parser.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4051,7 +4051,7 @@ namespace ts {
40514051
setDecoratorContext(/*val*/ true);
40524052
}
40534053

4054-
return finishNode(node);
4054+
return addJSDocComment(finishNode(node));
40554055
}
40564056

40574057
function parseOptionalIdentifier() {
@@ -4335,13 +4335,13 @@ namespace ts {
43354335
const labeledStatement = <LabeledStatement>createNode(SyntaxKind.LabeledStatement, fullStart);
43364336
labeledStatement.label = <Identifier>expression;
43374337
labeledStatement.statement = parseStatement();
4338-
return finishNode(labeledStatement);
4338+
return addJSDocComment(finishNode(labeledStatement));
43394339
}
43404340
else {
43414341
const expressionStatement = <ExpressionStatement>createNode(SyntaxKind.ExpressionStatement, fullStart);
43424342
expressionStatement.expression = expression;
43434343
parseSemicolon();
4344-
return finishNode(expressionStatement);
4344+
return addJSDocComment(finishNode(expressionStatement));
43454345
}
43464346
}
43474347

@@ -4778,7 +4778,7 @@ namespace ts {
47784778
parseExpected(SyntaxKind.ConstructorKeyword);
47794779
fillSignature(SyntaxKind.ColonToken, /*yieldContext*/ false, /*awaitContext*/ false, /*requireCompleteParameterList*/ false, node);
47804780
node.body = parseFunctionBlockOrSemicolon(/*isGenerator*/ false, /*isAsync*/ false, Diagnostics.or_expected);
4781-
return finishNode(node);
4781+
return addJSDocComment(finishNode(node));
47824782
}
47834783

47844784
function parseMethodDeclaration(fullStart: number, decorators: NodeArray<Decorator>, modifiers: ModifiersArray, asteriskToken: Node, name: PropertyName, questionToken: Node, diagnosticMessage?: DiagnosticMessage): MethodDeclaration {
@@ -4792,7 +4792,7 @@ namespace ts {
47924792
const isAsync = !!(method.flags & NodeFlags.Async);
47934793
fillSignature(SyntaxKind.ColonToken, /*yieldContext*/ isGenerator, /*awaitContext*/ isAsync, /*requireCompleteParameterList*/ false, method);
47944794
method.body = parseFunctionBlockOrSemicolon(isGenerator, isAsync, diagnosticMessage);
4795-
return finishNode(method);
4795+
return addJSDocComment(finishNode(method));
47964796
}
47974797

47984798
function parsePropertyDeclaration(fullStart: number, decorators: NodeArray<Decorator>, modifiers: ModifiersArray, name: PropertyName, questionToken: Node): ClassElement {

‎src/compiler/utilities.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1207,7 +1207,19 @@ namespace ts {
12071207
node.parent.parent.parent.kind === SyntaxKind.VariableStatement;
12081208

12091209
const variableStatementNode = isInitializerOfVariableDeclarationInStatement ? node.parent.parent.parent : undefined;
1210-
return variableStatementNode && variableStatementNode.jsDocComment;
1210+
if (variableStatementNode) {
1211+
return variableStatementNode.jsDocComment;
1212+
}
1213+
1214+
// Also recognize when the node is the RHS of an assignment expression
1215+
const isSourceOfAssignmentExpressionStatement =
1216+
node.parent && node.parent.parent &&
1217+
node.parent.kind === SyntaxKind.BinaryExpression &&
1218+
(node.parent as BinaryExpression).operatorToken.kind === SyntaxKind.EqualsToken &&
1219+
node.parent.parent.kind === SyntaxKind.ExpressionStatement;
1220+
if (isSourceOfAssignmentExpressionStatement) {
1221+
return node.parent.parent.jsDocComment;
1222+
}
12111223
}
12121224

12131225
return undefined;

‎src/services/services.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ namespace ts {
6262
export interface SourceFile {
6363
/* @internal */ version: string;
6464
/* @internal */ scriptSnapshot: IScriptSnapshot;
65-
/* @internal */ nameTable: Map<string>;
65+
/* @internal */ nameTable: Map<number>;
6666

6767
/* @internal */ getNamedDeclarations(): Map<Declaration[]>;
6868

@@ -808,7 +808,7 @@ namespace ts {
808808
public languageVersion: ScriptTarget;
809809
public languageVariant: LanguageVariant;
810810
public identifiers: Map<string>;
811-
public nameTable: Map<string>;
811+
public nameTable: Map<number>;
812812
public resolvedModules: Map<ResolvedModule>;
813813
public imports: LiteralExpression[];
814814
public moduleAugmentations: LiteralExpression[];
@@ -1957,8 +1957,6 @@ namespace ts {
19571957
const text = scriptSnapshot.getText(0, scriptSnapshot.getLength());
19581958
const sourceFile = createSourceFile(fileName, text, scriptTarget, setNodeParents);
19591959
setSourceFileFields(sourceFile, scriptSnapshot, version);
1960-
// after full parsing we can use table with interned strings as name table
1961-
sourceFile.nameTable = sourceFile.identifiers;
19621960
return sourceFile;
19631961
}
19641962

@@ -3834,7 +3832,7 @@ namespace ts {
38343832

38353833
if (isRightOfDot && isSourceFileJavaScript(sourceFile)) {
38363834
const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries);
3837-
addRange(entries, getJavaScriptCompletionEntries(sourceFile, uniqueNames));
3835+
addRange(entries, getJavaScriptCompletionEntries(sourceFile, location.pos, uniqueNames));
38383836
}
38393837
else {
38403838
if (!symbols || symbols.length === 0) {
@@ -3867,12 +3865,17 @@ namespace ts {
38673865

38683866
return { isMemberCompletion, isNewIdentifierLocation, entries };
38693867

3870-
function getJavaScriptCompletionEntries(sourceFile: SourceFile, uniqueNames: Map<string>): CompletionEntry[] {
3868+
function getJavaScriptCompletionEntries(sourceFile: SourceFile, position: number, uniqueNames: Map<string>): CompletionEntry[] {
38713869
const entries: CompletionEntry[] = [];
38723870
const target = program.getCompilerOptions().target;
38733871

38743872
const nameTable = getNameTable(sourceFile);
38753873
for (const name in nameTable) {
3874+
// Skip identifiers produced only from the current location
3875+
if (nameTable[name] === position) {
3876+
continue;
3877+
}
3878+
38763879
if (!uniqueNames[name]) {
38773880
uniqueNames[name] = name;
38783881
const displayName = getCompletionEntryDisplayName(name, target, /*performCharacterChecks*/ true);
@@ -5487,7 +5490,7 @@ namespace ts {
54875490

54885491
const nameTable = getNameTable(sourceFile);
54895492

5490-
if (lookUp(nameTable, internedName)) {
5493+
if (lookUp(nameTable, internedName) !== undefined) {
54915494
result = result || [];
54925495
getReferencesInNode(sourceFile, symbol, declaredName, node, searchMeaning, findInStrings, findInComments, result, symbolToIndex);
54935496
}
@@ -7525,7 +7528,7 @@ namespace ts {
75257528
}
75267529

75277530
/* @internal */
7528-
export function getNameTable(sourceFile: SourceFile): Map<string> {
7531+
export function getNameTable(sourceFile: SourceFile): Map<number> {
75297532
if (!sourceFile.nameTable) {
75307533
initializeNameTable(sourceFile);
75317534
}
@@ -7534,15 +7537,15 @@ namespace ts {
75347537
}
75357538

75367539
function initializeNameTable(sourceFile: SourceFile): void {
7537-
const nameTable: Map<string> = {};
7540+
const nameTable: Map<number> = {};
75387541

75397542
walk(sourceFile);
75407543
sourceFile.nameTable = nameTable;
75417544

75427545
function walk(node: Node) {
75437546
switch (node.kind) {
75447547
case SyntaxKind.Identifier:
7545-
nameTable[(<Identifier>node).text] = (<Identifier>node).text;
7548+
nameTable[(<Identifier>node).text] = nameTable[(<Identifier>node).text] === undefined ? node.pos : -1;
75467549
break;
75477550
case SyntaxKind.StringLiteral:
75487551
case SyntaxKind.NumericLiteral:
@@ -7554,7 +7557,7 @@ namespace ts {
75547557
node.parent.kind === SyntaxKind.ExternalModuleReference ||
75557558
isArgumentOfElementAccessExpression(node)) {
75567559

7557-
nameTable[(<LiteralExpression>node).text] = (<LiteralExpression>node).text;
7560+
nameTable[(<LiteralExpression>node).text] = nameTable[(<LiteralExpression>node).text] === undefined ? node.pos : -1;
75587561
}
75597562
break;
75607563
default:

‎tests/cases/fourslash/getJavaScriptCompletions16.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @allowNonTsExtensions: true
4+
// @Filename: file.js
5+
//// "use strict";
6+
////
7+
//// class Something {
8+
////
9+
//// /**
10+
//// * @param {number} a
11+
//// */
12+
//// constructor(a, b) {
13+
//// a/*body*/
14+
//// }
15+
////
16+
//// /**
17+
//// * @param {number} a
18+
//// */
19+
//// method(a) {
20+
//// a/*method*/
21+
//// }
22+
//// }
23+
//// let x = new Something(/*sig*/);
24+
25+
goTo.marker('body');
26+
edit.insert('.');
27+
verify.completionListContains('toFixed', undefined, undefined, 'method');
28+
edit.backspace();
29+
30+
goTo.marker('sig');
31+
verify.currentSignatureHelpIs('Something(a: number, b: any): Something');
32+
33+
goTo.marker('method');
34+
edit.insert('.');
35+
verify.completionListContains('toFixed', undefined, undefined, 'method');

‎tests/cases/fourslash/getJavaScriptCompletions18.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @allowNonTsExtensions: true
4+
// @Filename: file.js
5+
//// /**
6+
//// * @param {number} a
7+
//// * @param {string} b
8+
//// */
9+
//// exports.foo = function(a, b) {
10+
//// a/*a*/;
11+
//// b/*b*/
12+
//// };
13+
14+
goTo.marker('a');
15+
edit.insert('.');
16+
verify.completionListContains('toFixed', undefined, undefined, 'method');
17+
18+
19+
goTo.marker('b');
20+
edit.insert('.');
21+
verify.completionListContains('substr', undefined, undefined, 'method');

‎tests/cases/fourslash/getJavaScriptCompletions19.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @allowNonTsExtensions: true
4+
// @Filename: file.js
5+
//// function fn() {
6+
//// if (foo) {
7+
//// return 0;
8+
//// } else {
9+
//// return '0';
10+
//// }
11+
//// }
12+
//// let x = fn();
13+
//// if(typeof x === 'string') {
14+
//// x/*str*/
15+
//// } else {
16+
//// x/*num*/
17+
//// }
18+
19+
goTo.marker('str');
20+
edit.insert('.');
21+
verify.completionListContains('substr', undefined, undefined, 'method');
22+
23+
goTo.marker('num');
24+
edit.insert('.');
25+
verify.completionListContains('toFixed', undefined, undefined, 'method');

‎tests/cases/fourslash/getJavaScriptCompletions20.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @allowNonTsExtensions: true
4+
// @Filename: file.js
5+
//// /**
6+
//// * A person
7+
//// * @constructor
8+
//// * @param {string} name - The name of the person.
9+
//// * @param {number} age - The age of the person.
10+
//// */
11+
//// function Person(name, age) {
12+
//// this.name = name;
13+
//// this.age = age;
14+
//// }
15+
////
16+
////
17+
//// Person.getName = 10;
18+
//// Person.getNa/**/ = 10;
19+
20+
goTo.marker();
21+
verify.not.memberListContains('getNa');

‎tests/cases/fourslash/getJavaScriptQuickInfo7.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @allowNonTsExtensions: true
4+
// @Filename: file.js
5+
//// /**
6+
//// * This is a very cool function that is very nice.
7+
//// * @returns something
8+
//// * @param p anotherthing
9+
//// */
10+
//// function a1(p) {
11+
//// try {
12+
//// throw new Error('x');
13+
//// } catch (x) { x--; }
14+
//// return 23;
15+
//// }
16+
////
17+
//// x - /**/a1()
18+
19+
goTo.marker();
20+
verify.quickInfoExists();

0 commit comments

Comments
 (0)