-
Notifications
You must be signed in to change notification settings - Fork 24
implemented @deprecated diagnostics reporting #430
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/** @deprecated This class is deprecated */ | ||
class Foo {} | ||
|
||
/** @deprecated This function is deprecated */ | ||
function bar() {} | ||
|
||
/** | ||
* @deprecated This is a function that has | ||
* multiple lines and is also deprecated. Make | ||
* sure to reference {@link bar} for some reason | ||
*/ | ||
function car() {} | ||
|
||
function main() { | ||
new Foo() | ||
bar() | ||
car() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"name": "diagnostics", | ||
"version": "0.0.1" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
// < definition diagnostics 0.0.1 `index.ts`/ | ||
|
||
/** @deprecated This class is deprecated */ | ||
class Foo {} | ||
// ^^^ definition diagnostics 0.0.1 `index.ts`/Foo# | ||
|
||
/** @deprecated This function is deprecated */ | ||
function bar() {} | ||
// ^^^ definition diagnostics 0.0.1 `index.ts`/bar(). | ||
|
||
/** | ||
* @deprecated This is a function that has | ||
* multiple lines and is also deprecated. Make | ||
* sure to reference {@link bar} for some reason | ||
*/ | ||
function car() {} | ||
// ^^^ definition diagnostics 0.0.1 `index.ts`/car(). | ||
|
||
function main() { | ||
// ^^^^ definition diagnostics 0.0.1 `index.ts`/main(). | ||
new Foo() | ||
// ^^^ reference diagnostics 0.0.1 `index.ts`/Foo# | ||
// diagnostic Information: | ||
// > This class is deprecated | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be good to add some test cases for multiline messages with |
||
bar() | ||
//^^^ reference diagnostics 0.0.1 `index.ts`/bar(). | ||
//diagnostic Information: | ||
//> This function is deprecated | ||
car() | ||
//^^^ reference diagnostics 0.0.1 `index.ts`/car(). | ||
//diagnostic Information: | ||
//> This is a function that has | ||
//> multiple lines and is also deprecated. Make | ||
//> sure to reference {@link bar } for some reason | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -242,6 +242,12 @@ export class FileIndexer { | |
range, | ||
symbol: scipSymbol.value, | ||
symbol_roles: role, | ||
|
||
// Diagnostics should only be added for references to the symbol, not | ||
// the definition | ||
diagnostics: !isDefinitionNode | ||
? this.diagnosticsForSymbol(sym) | ||
: undefined, | ||
}) | ||
) | ||
if (isDefinitionNode) { | ||
|
@@ -718,6 +724,29 @@ export class FileIndexer { | |
} | ||
loop(node) | ||
} | ||
|
||
// Returns the scip diagnostics for a given typescript symbol | ||
private diagnosticsForSymbol( | ||
sym: ts.Symbol | ||
): scip.scip.Diagnostic[] | undefined { | ||
const jsDocTags = sym.getJsDocTags() | ||
|
||
const deprecatedTag = jsDocTags.find(tag => tag.name === 'deprecated') | ||
if (deprecatedTag) { | ||
return [ | ||
new scip.scip.Diagnostic({ | ||
severity: scip.scip.Severity.Information, | ||
code: 'DEPRECATED', | ||
// jsDocInfo.text is a tokenized representation of the tag text. | ||
// Concatenate the elements to get the full message | ||
message: deprecatedTag.text?.map(part => part.text).join(''), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't quite understand when we'll have multiple elements here, and why it's justified to join with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The result of Because the individual parts of this include spaces, we simply I've added a comment to explain this better (as well as added a test covering this tokenizing better) |
||
tags: [scip.scip.DiagnosticTag.Deprecated], | ||
}), | ||
] | ||
} | ||
|
||
return undefined | ||
} | ||
} | ||
|
||
function isAnonymousContainerOfSymbols(node: ts.Node): boolean { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a reference to
car()
to trigger the logic for multiline diagnostics formatting.