Skip to content

[GH-43213] Fix duplicate comments printed in quick info section #43240

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
merged 7 commits into from
Mar 19, 2021
Merged

[GH-43213] Fix duplicate comments printed in quick info section #43240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
69933ea
Concat on unique values
hahmadia Mar 14, 2021
d82255e
more changes
hahmadia Mar 14, 2021
7ed1ba7
only push values to array if unique
hahmadia Mar 14, 2021
d8f8b1c
Merge branch 'master' into GH-43213
hahmadia Mar 19, 2021
1ed06f2
address review comments
hahmadia Mar 19, 2021
9645b98
Fix lint
hahmadia Mar 19, 2021
4a66d9a
removw foeachunique
hahmadia Mar 19, 2021
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: 9 additions & 3 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,17 @@ namespace ts {

let doc = JsDoc.getJsDocCommentsFromDeclarations(declarations);
if (checker && (doc.length === 0 || declarations.some(hasJSDocInheritDocTag))) {
forEachUnique(declarations, declaration => {
const inheritedDocs = findBaseOfDeclaration(checker, declaration, symbol => symbol.getDocumentationComment(checker));
const seenSymbols = new Set<Symbol>();
for (const declaration of declarations) {
const inheritedDocs = findBaseOfDeclaration(checker, declaration, symbol => {
if (!seenSymbols.has(symbol)) {
seenSymbols.add(symbol);
return symbol.getDocumentationComment(checker);
}
});
// TODO: GH#16312 Return a ReadonlyArray, avoid copying inheritedDocs
if (inheritedDocs) doc = doc.length === 0 ? inheritedDocs.slice() : inheritedDocs.concat(lineBreakPart(), doc);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need to write your own loop; findBaseOfDeclaration allows you to skip base declarations by returning undefined, so you can maintain seen: Set<Symbol> to allow you to skip a symbol that's been seen before.

Copy link
Member

Choose a reason for hiding this comment

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

Also, the outer forEachUnique might be redundant with this fix in place. Might be worth running tests once with it removed to see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GENUIS!

});
}
}
return doc;
}
Expand Down
18 changes: 18 additions & 0 deletions tests/cases/fourslash/server/quickinfoWrongComment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path="../fourslash.ts"/>

//// interface I {
//// /** The colour */
//// readonly colour: string
//// }
//// interface A extends I {
//// readonly colour: "red" | "green";
//// }
//// interface B extends I {
//// readonly colour: "yellow" | "green";
//// }
//// type F = A | B
//// const f: F = { colour: "green" }
//// f.colour/*1*/

goTo.marker("1")
verify.quickInfoIs("(property) colour: \"red\" | \"green\" | \"yellow\"", "The colour")