Skip to content

Prevent detached diagnostics from running off the end of the file #55381

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 2 commits into from
Aug 15, 2023
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
12 changes: 6 additions & 6 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1803,7 +1803,7 @@ namespace Parser {
return sourceFile;

function reportPragmaDiagnostic(pos: number, end: number, diagnostic: DiagnosticMessage) {
parseDiagnostics.push(createDetachedDiagnostic(fileName, pos, end, diagnostic));
parseDiagnostics.push(createDetachedDiagnostic(fileName, sourceText, pos, end, diagnostic));
}
}

Expand Down Expand Up @@ -2118,7 +2118,7 @@ namespace Parser {
const lastError = lastOrUndefined(parseDiagnostics);
let result: DiagnosticWithDetachedLocation | undefined;
if (!lastError || start !== lastError.start) {
result = createDetachedDiagnostic(fileName, start, length, message, ...args);
result = createDetachedDiagnostic(fileName, sourceText, start, length, message, ...args);
parseDiagnostics.push(result);
}

Expand Down Expand Up @@ -2470,7 +2470,7 @@ namespace Parser {
if (lastError) {
addRelatedInfo(
lastError,
createDetachedDiagnostic(fileName, openPosition, 1, Diagnostics.The_parser_expected_to_find_a_1_to_match_the_0_token_here, tokenToString(openKind), tokenToString(closeKind))
createDetachedDiagnostic(fileName, sourceText, openPosition, 1, Diagnostics.The_parser_expected_to_find_a_1_to_match_the_0_token_here, tokenToString(openKind), tokenToString(closeKind))
Copy link
Member Author

Choose a reason for hiding this comment

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

This set of three detached diagnostics are the trouble; openPosition can totally be at the end of the file.

);
}
}
Expand Down Expand Up @@ -4486,7 +4486,7 @@ namespace Parser {
if (lastError && lastError.code === Diagnostics._0_expected.code) {
addRelatedInfo(
lastError,
createDetachedDiagnostic(fileName, openBracePosition, 1, Diagnostics.The_parser_expected_to_find_a_1_to_match_the_0_token_here, "{", "}")
createDetachedDiagnostic(fileName, sourceText, openBracePosition, 1, Diagnostics.The_parser_expected_to_find_a_1_to_match_the_0_token_here, "{", "}")
);
}
}
Expand Down Expand Up @@ -8326,7 +8326,7 @@ namespace Parser {
if (lastError && lastError.code === Diagnostics._0_expected.code) {
addRelatedInfo(
lastError,
createDetachedDiagnostic(fileName, openBracePosition, 1, Diagnostics.The_parser_expected_to_find_a_1_to_match_the_0_token_here, "{", "}")
createDetachedDiagnostic(fileName, sourceText, openBracePosition, 1, Diagnostics.The_parser_expected_to_find_a_1_to_match_the_0_token_here, "{", "}")
);
}
}
Expand Down Expand Up @@ -9429,7 +9429,7 @@ namespace Parser {
if (childTypeTag) {
const lastError = parseErrorAtCurrentToken(Diagnostics.A_JSDoc_typedef_comment_may_not_contain_multiple_type_tags);
if (lastError) {
addRelatedInfo(lastError, createDetachedDiagnostic(fileName, 0, 0, Diagnostics.The_tag_was_first_specified_here));
addRelatedInfo(lastError, createDetachedDiagnostic(fileName, sourceText, 0, 0, Diagnostics.The_tag_was_first_specified_here));
}
break;
}
Expand Down
21 changes: 11 additions & 10 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2150,19 +2150,16 @@ export function createDiagnosticForNodeArrayFromMessageChain(sourceFile: SourceF
return createFileDiagnosticFromMessageChain(sourceFile, start, nodes.end - start, messageChain, relatedInformation);
}

function assertDiagnosticLocation(file: SourceFile | undefined, start: number, length: number) {
function assertDiagnosticLocation(sourceText: string, start: number, length: number) {
Debug.assertGreaterThanOrEqual(start, 0);
Debug.assertGreaterThanOrEqual(length, 0);

if (file) {
Debug.assertLessThanOrEqual(start, file.text.length);
Debug.assertLessThanOrEqual(start + length, file.text.length);
}
Debug.assertLessThanOrEqual(start, sourceText.length);
Debug.assertLessThanOrEqual(start + length, sourceText.length);
}

/** @internal */
export function createFileDiagnosticFromMessageChain(file: SourceFile, start: number, length: number, messageChain: DiagnosticMessageChain, relatedInformation?: DiagnosticRelatedInformation[]): DiagnosticWithLocation {
assertDiagnosticLocation(file, start, length);
assertDiagnosticLocation(file.text, start, length);
return {
file,
start,
Expand Down Expand Up @@ -8152,8 +8149,12 @@ export function getLocaleSpecificMessage(message: DiagnosticMessage) {
}

/** @internal */
export function createDetachedDiagnostic(fileName: string, start: number, length: number, message: DiagnosticMessage, ...args: DiagnosticArguments): DiagnosticWithDetachedLocation {
assertDiagnosticLocation(/*file*/ undefined, start, length);
export function createDetachedDiagnostic(fileName: string, sourceText: string, start: number, length: number, message: DiagnosticMessage, ...args: DiagnosticArguments): DiagnosticWithDetachedLocation {
if ((start + length) > sourceText.length) {
length = sourceText.length - start;
}
Comment on lines +8153 to +8155
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the actual fix.


assertDiagnosticLocation(sourceText, start, length);
let text = getLocaleSpecificMessage(message);

if (some(args)) {
Expand Down Expand Up @@ -8222,7 +8223,7 @@ export function attachFileToDiagnostics(diagnostics: DiagnosticWithDetachedLocat

/** @internal */
export function createFileDiagnostic(file: SourceFile, start: number, length: number, message: DiagnosticMessage, ...args: DiagnosticArguments): DiagnosticWithLocation {
assertDiagnosticLocation(file, start, length);
assertDiagnosticLocation(file.text, start, length);

let text = getLocaleSpecificMessage(message);

Expand Down
21 changes: 21 additions & 0 deletions tests/baselines/reference/parseUnmatchedTypeAssertion.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
parseUnmatchedTypeAssertion.ts(1,2): error TS1109: Expression expected.
parseUnmatchedTypeAssertion.ts(1,12): error TS1141: String literal expected.
parseUnmatchedTypeAssertion.ts(1,12): error TS2304: Cannot find name 'obju2c77'.
parseUnmatchedTypeAssertion.ts(1,21): error TS1109: Expression expected.
parseUnmatchedTypeAssertion.ts(2,1): error TS1005: '{' expected.


==== parseUnmatchedTypeAssertion.ts (5 errors) ====
@<[[import(obju2c77,
~
!!! error TS1109: Expression expected.
~~~~~~~~
!!! error TS1141: String literal expected.
~~~~~~~~
!!! error TS2304: Cannot find name 'obju2c77'.

!!! error TS1109: Expression expected.


!!! error TS1005: '{' expected.
!!! related TS1007 parseUnmatchedTypeAssertion.ts:2:1: The parser expected to find a '}' to match the '{' token here.
8 changes: 8 additions & 0 deletions tests/baselines/reference/parseUnmatchedTypeAssertion.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//// [tests/cases/compiler/parseUnmatchedTypeAssertion.ts] ////

//// [parseUnmatchedTypeAssertion.ts]
@<[[import(obju2c77,


//// [parseUnmatchedTypeAssertion.js]
;
6 changes: 6 additions & 0 deletions tests/baselines/reference/parseUnmatchedTypeAssertion.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//// [tests/cases/compiler/parseUnmatchedTypeAssertion.ts] ////

=== parseUnmatchedTypeAssertion.ts ===
@<[[import(obju2c77,
>obju2c77 : Symbol(obju2c77)

9 changes: 9 additions & 0 deletions tests/baselines/reference/parseUnmatchedTypeAssertion.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//// [tests/cases/compiler/parseUnmatchedTypeAssertion.ts] ////

=== parseUnmatchedTypeAssertion.ts ===
@<[[import(obju2c77,
> : any
><[[import(obju2c77, : [[any]]

> : any

1 change: 1 addition & 0 deletions tests/cases/compiler/parseUnmatchedTypeAssertion.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@<[[import(obju2c77,