Skip to content

Detect and prevent creation of bad Identifier #21581

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.

Sign up for GitHub

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
1 commit merged into from
Feb 5, 2018
Merged

Detect and prevent creation of bad Identifier #21581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5891,7 +5891,7 @@ namespace ts {

/** True if node is of a kind that may contain comment text. */
export function isJSDocCommentContainingNode(node: Node): boolean {
return node.kind === SyntaxKind.JSDocComment || isJSDocTag(node);
return node.kind === SyntaxKind.JSDocComment || isJSDocTag(node) || isJSDocTypeLiteral(node);
}

// TODO: determine what this does before making it public.
Expand Down
5 changes: 4 additions & 1 deletion src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,15 @@ namespace ts {
return sourceFile.text.substring(this.getStart(sourceFile), this.getEnd());
}

private addSyntheticNodes(nodes: Node[], pos: number, end: number): number {
private addSyntheticNodes(nodes: Push<Node>, pos: number, end: number): number {
scanner.setTextPos(pos);
while (pos < end) {
const token = scanner.scan();
const textPos = scanner.getTextPos();
if (textPos <= end) {
if (token === SyntaxKind.Identifier) {
Debug.fail(`Did not expect ${(ts as any).SyntaxKind[this.kind]} to have an Identifier in its trivia`);
}
nodes.push(createNode(token, pos, textPos, this));
}
pos = textPos;
Expand Down
6 changes: 0 additions & 6 deletions tests/cases/fourslash/findAllReferencesJsDocTypeLiteral.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,3 @@
//// function f(o) { return o.nested.[|great|]; }

verify.rangesReferenceEachOther();

Copy link
Copy Markdown
Author

@ghost ghost Feb 2, 2018

Choose a reason for hiding this comment

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

This was the test that broke under the new assertion. I'm just removing a comment here that does nothing (it only has two slashes).

Copy link
Copy Markdown
Member

@sandersn sandersn Feb 2, 2018

Choose a reason for hiding this comment

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

Yeah, I must have left it from earlier testing and checked it in by mistake. But it would probably be good to retain this as a parser test.

Copy link
Copy Markdown
Author

@ghost ghost Feb 5, 2018 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

By "this" I meant the above code. This below is a regular comment that doesn't test anything.

///**
// * @param {object} [|o|] - very important!
// * @param {string} o.x - a thing, its ok
// */
// function f([|o|]) { return [|o|].x; }