Skip to content

Allow JSDoc casts of parenthesized expressions in JS #17211

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

Closed
wants to merge 10 commits into from
Closed
2 changes: 1 addition & 1 deletion issue_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

```ts
// A *self-contained* demonstration of the problem follows...

// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.
```

**Expected behavior:**
Expand Down
30 changes: 24 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6892,7 +6892,7 @@ namespace ts {
return globalFunctionType;
case "Array":
case "array":
return !node.typeArguments || !node.typeArguments.length ? createArrayType(anyType) : undefined;
return !node.typeArguments || !node.typeArguments.length ? anyArrayType : undefined;
case "Promise":
case "promise":
return !node.typeArguments || !node.typeArguments.length ? createPromiseType(anyType) : undefined;
Expand Down Expand Up @@ -16256,15 +16256,19 @@ namespace ts {
}

function checkAssertion(node: AssertionExpression) {
const exprType = getRegularTypeOfObjectLiteral(getBaseTypeOfLiteralType(checkExpression(node.expression)));
return checkAssertionWorker(node, node.type, node.expression);
}

checkSourceElement(node.type);
const targetType = getTypeFromTypeNode(node.type);
function checkAssertionWorker(errNode: Node, type: TypeNode, expression: UnaryExpression | Expression, checkMode?: CheckMode) {
const exprType = getRegularTypeOfObjectLiteral(getBaseTypeOfLiteralType(checkExpression(expression, checkMode)));

checkSourceElement(type);
const targetType = getTypeFromTypeNode(type);

if (produceDiagnostics && targetType !== unknownType) {
const widenedType = getWidenedType(exprType);
if (!isTypeComparableTo(targetType, widenedType)) {
checkTypeComparableTo(exprType, targetType, node, Diagnostics.Type_0_cannot_be_converted_to_type_1);
checkTypeComparableTo(exprType, targetType, errNode, Diagnostics.Type_0_cannot_be_converted_to_type_1);
}
}
return targetType;
Expand Down Expand Up @@ -17735,6 +17739,20 @@ namespace ts {
return type;
}
Copy link
Member

Choose a reason for hiding this comment

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

why not combine these predicates?


Copy link
Member

Choose a reason for hiding this comment

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

can you use flatMap instead? filter should always return an array so you don't need the if (isArray ... check in flatten.

function checkParenthesizedExpression(node: ParenthesizedExpression, checkMode?: CheckMode): Type {
if (isInJavaScriptFile(node)) {
if (node.jsDoc) {
const typecasts = flatten<JSDocTag>(map(node.jsDoc, doc => filter(doc.tags, tag => tag.kind === SyntaxKind.JSDocTypeTag)));
if (typecasts && typecasts.length) {
// We should have already issued an error if there were multiple type jsdocs
const cast = typecasts[0] as JSDocTypeTag;
return checkAssertionWorker(cast, cast.typeExpression.type, node.expression, checkMode);
}
}
}
return checkExpression(node.expression, checkMode);
}

function checkExpressionWorker(node: Expression, checkMode: CheckMode): Type {
switch (node.kind) {
case SyntaxKind.Identifier:
Expand Down Expand Up @@ -17774,7 +17792,7 @@ namespace ts {
case SyntaxKind.TaggedTemplateExpression:
return checkTaggedTemplateExpression(<TaggedTemplateExpression>node);
case SyntaxKind.ParenthesizedExpression:
return checkExpression((<ParenthesizedExpression>node).expression, checkMode);
return checkParenthesizedExpression(<ParenthesizedExpression>node, checkMode);
case SyntaxKind.ClassExpression:
return checkClassExpression(<ClassExpression>node);
case SyntaxKind.FunctionExpression:
Expand Down
27 changes: 13 additions & 14 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ namespace ts {
}
}

export function parseCommandLine(commandLine: string[], readFile?: (path: string) => string): ParsedCommandLine {
export function parseCommandLine(commandLine: string[], readFile?: (path: string) => string | undefined): ParsedCommandLine {
const options: CompilerOptions = {};
const fileNames: string[] = [];
const errors: Diagnostic[] = [];
Expand Down Expand Up @@ -878,15 +878,9 @@ namespace ts {
* Read tsconfig.json file
* @param fileName The path to the config file
*/
export function readConfigFile(fileName: string, readFile: (path: string) => string): { config?: any; error?: Diagnostic } {
let text = "";
try {
text = readFile(fileName);
}
catch (e) {
return { config: {}, error: createCompilerDiagnostic(Diagnostics.Cannot_read_file_0_Colon_1, fileName, e.message) };
}
return parseConfigFileTextToJson(fileName, text);
export function readConfigFile(fileName: string, readFile: (path: string) => string | undefined): { config?: any; error?: Diagnostic } {
const textOrDiagnostic = tryReadFile(fileName, readFile);
return typeof textOrDiagnostic === "string" ? parseConfigFileTextToJson(fileName, textOrDiagnostic) : { config: {}, error: textOrDiagnostic };
}

/**
Expand All @@ -906,15 +900,20 @@ namespace ts {
* Read tsconfig.json file
* @param fileName The path to the config file
*/
export function readJsonConfigFile(fileName: string, readFile: (path: string) => string): JsonSourceFile {
let text = "";
export function readJsonConfigFile(fileName: string, readFile: (path: string) => string | undefined): JsonSourceFile {
const textOrDiagnostic = tryReadFile(fileName, readFile);
return typeof textOrDiagnostic === "string" ? parseJsonText(fileName, textOrDiagnostic) : <JsonSourceFile>{ parseDiagnostics: [textOrDiagnostic] };
}

function tryReadFile(fileName: string, readFile: (path: string) => string | undefined): string | Diagnostic {
let text: string | undefined;
try {
text = readFile(fileName);
}
catch (e) {
return <JsonSourceFile>{ parseDiagnostics: [createCompilerDiagnostic(Diagnostics.Cannot_read_file_0_Colon_1, fileName, e.message)] };
return createCompilerDiagnostic(Diagnostics.Cannot_read_file_0_Colon_1, fileName, e.message);
}
return parseJsonText(fileName, text);
return text === undefined ? createCompilerDiagnostic(Diagnostics.The_specified_path_does_not_exist_Colon_0, fileName) : text;
}

function commandLineOptionsToMap(options: CommandLineOption[]) {
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ namespace ts {
/**
* Stable sort of an array. Elements equal to each other maintain their relative position in the array.
*/
export function stableSort<T>(array: ReadonlyArray<T>, comparer: (x: T, y: T) => Comparison = compareValues) {
export function stableSort<T>(array: ReadonlyArray<T>, comparer: Comparer<T> = compareValues) {
return array
.map((_, i) => i) // create array of indices
.sort((x, y) => comparer(array[x], array[y]) || compareValues(x, y)) // sort indices by value then position
Expand Down Expand Up @@ -852,14 +852,16 @@ namespace ts {
return result;
}

export type Comparer<T> = (a: T, b: T) => Comparison;

/**
* Performs a binary search, finding the index at which 'value' occurs in 'array'.
* If no such index is found, returns the 2's-complement of first index at which
* number[index] exceeds number.
* @param array A sorted array whose first element must be no larger than number
* @param number The value to be searched for in the array.
*/
export function binarySearch<T>(array: ReadonlyArray<T>, value: T, comparer?: (v1: T, v2: T) => number, offset?: number): number {
export function binarySearch<T>(array: ReadonlyArray<T>, value: T, comparer?: Comparer<T>, offset?: number): number {
if (!array || array.length === 0) {
return -1;
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4342,7 +4342,7 @@ namespace ts {
parseExpected(SyntaxKind.OpenParenToken);
node.expression = allowInAnd(parseExpression);
parseExpected(SyntaxKind.CloseParenToken);
return finishNode(node);
return addJSDocComment(finishNode(node));
}

function parseSpreadElement(): Expression {
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/sys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace ts {
newLine: string;
useCaseSensitiveFileNames: boolean;
write(s: string): void;
readFile(path: string, encoding?: string): string;
readFile(path: string, encoding?: string): string | undefined;
getFileSize?(path: string): number;
writeFile(path: string, data: string, writeByteOrderMark?: boolean): void;
/**
Expand Down Expand Up @@ -97,7 +97,7 @@ namespace ts {
directoryExists(path: string): boolean;
createDirectory(path: string): void;
resolvePath(path: string): string;
readFile(path: string): string;
readFile(path: string): string | undefined;
writeFile(path: string, contents: string): void;
getDirectories(path: string): string[];
readDirectory(path: string, extensions?: ReadonlyArray<string>, basePaths?: ReadonlyArray<string>, excludeEx?: string, includeFileEx?: string, includeDirEx?: string): string[];
Expand Down Expand Up @@ -204,7 +204,7 @@ namespace ts {
const platform: string = _os.platform();
const useCaseSensitiveFileNames = isFileSystemCaseSensitive();

function readFile(fileName: string, _encoding?: string): string {
function readFile(fileName: string, _encoding?: string): string | undefined {
if (!fileExists(fileName)) {
return undefined;
}
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2349,7 +2349,7 @@ namespace ts {
*/
fileExists(path: string): boolean;

readFile(path: string): string;
readFile(path: string): string | undefined;
}

export interface WriteFileCallback {
Expand Down Expand Up @@ -3856,7 +3856,7 @@ namespace ts {
fileExists(fileName: string): boolean;
// readFile function is used to read arbitrary text files on disk, i.e. when resolution procedure needs the content of 'package.json'
// to determine location of bundled typings for node module
readFile(fileName: string): string;
readFile(fileName: string): string | undefined;
trace?(s: string): void;
directoryExists?(directoryName: string): boolean;
realpath?(path: string): string;
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,8 @@ namespace ts {
const commentRanges = (node.kind === SyntaxKind.Parameter ||
node.kind === SyntaxKind.TypeParameter ||
node.kind === SyntaxKind.FunctionExpression ||
node.kind === SyntaxKind.ArrowFunction) ?
node.kind === SyntaxKind.ArrowFunction ||
node.kind === SyntaxKind.ParenthesizedExpression) ?
concatenate(getTrailingCommentRanges(text, node.pos), getLeadingCommentRanges(text, node.pos)) :
getLeadingCommentRangesOfNodeFromText(node, text);
// True if the comment starts with '/**' but not if it is '/**/'
Expand Down Expand Up @@ -3925,7 +3926,7 @@ namespace ts {
*/
export function validateLocaleAndSetLanguage(
locale: string,
sys: { getExecutingFilePath(): string, resolvePath(path: string): string, fileExists(fileName: string): boolean, readFile(fileName: string): string },
sys: { getExecutingFilePath(): string, resolvePath(path: string): string, fileExists(fileName: string): boolean, readFile(fileName: string): string | undefined },
errors?: Diagnostic[]) {
const matchResult = /^([a-z]+)([_\-]([a-z]+))?$/.exec(locale.toLowerCase());

Expand Down
6 changes: 3 additions & 3 deletions src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ namespace Harness {
getCurrentDirectory(): string;
useCaseSensitiveFileNames(): boolean;
resolvePath(path: string): string;
readFile(path: string): string;
readFile(path: string): string | undefined;
writeFile(path: string, contents: string): void;
directoryName(path: string): string;
getDirectories(path: string): string[];
Expand Down Expand Up @@ -719,7 +719,7 @@ namespace Harness {
}
});

export function readFile(file: string) {
export function readFile(file: string): string | undefined {
const response = Http.getFileFromServerSync(serverRoot + file);
if (response.status === 200) {
return response.responseText;
Expand Down Expand Up @@ -976,7 +976,7 @@ namespace Harness {
useCaseSensitiveFileNames: () => useCaseSensitiveFileNames,
getNewLine: () => newLine,
fileExists: fileName => fileMap.has(toPath(fileName)),
readFile: (fileName: string): string => {
readFile(fileName: string): string | undefined {
const file = fileMap.get(toPath(fileName));
if (ts.endsWith(fileName, "json")) {
// strip comments
Expand Down
4 changes: 2 additions & 2 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ namespace Harness.LanguageService {
depth,
(p) => this.virtualFileSystem.getAccessibleFileSystemEntries(p));
}
readFile(path: string): string {
readFile(path: string): string | undefined {
const snapshot = this.getScriptSnapshot(path);
return snapshot.getText(0, snapshot.getLength());
}
Expand Down Expand Up @@ -619,7 +619,7 @@ namespace Harness.LanguageService {
this.writeMessage(message);
}

readFile(fileName: string): string {
readFile(fileName: string): string | undefined {
if (fileName.indexOf(Harness.Compiler.defaultLibFileName) >= 0) {
fileName = Harness.Compiler.defaultLibFileName;
}
Expand Down
2 changes: 1 addition & 1 deletion src/harness/projectsRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ class ProjectRunner extends RunnerBase {
return Harness.IO.fileExists(getFileNameInTheProjectTest(fileName));
}

function readFile(fileName: string): string {
function readFile(fileName: string): string | undefined {
return Harness.IO.readFile(getFileNameInTheProjectTest(fileName));
}

Expand Down
2 changes: 1 addition & 1 deletion src/harness/unittests/moduleResolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ namespace ts {
else {
return { readFile, fileExists: path => map.has(path) };
}
function readFile(path: string): string {
function readFile(path: string): string | undefined {
const file = map.get(path);
return file && file.content;
}
Expand Down
2 changes: 1 addition & 1 deletion src/harness/unittests/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace ts.server {
newLine: "\n",
useCaseSensitiveFileNames: true,
write(s): void { lastWrittenToHost = s; },
readFile(): string { return void 0; },
readFile: () => undefined,
writeFile: noop,
resolvePath(): string { return void 0; },
fileExists: () => false,
Expand Down
2 changes: 1 addition & 1 deletion src/harness/virtualFileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ namespace Utils {
}
}

readFile(path: string): string {
readFile(path: string): string | undefined {
const value = this.traversePath(path);
if (value && value.isFile()) {
return value.content.content;
Expand Down
Loading