implemented @deprecated diagnostics reporting #430
Conversation
|
@varungandhi-src this PR should be ready for review. Happy to punt on the idea if you'd prefer the scip-typescript indexer not providing this as well |
| new scip.scip.Diagnostic({ | ||
| severity: scip.scip.Severity.Information, | ||
| code: 'DEPRECATED', | ||
| message: deprecatedTag.text?.map(part => part.text).join(''), |
There was a problem hiding this comment.
I don't quite understand when we'll have multiple elements here, and why it's justified to join with '' instead of say ' '. Could you explain that? It'd be good to add a code comment for this.
| new Foo() | ||
| // ^^^ reference diagnostics 0.0.1 `index.ts`/Foo# | ||
| // diagnostic Information: | ||
| // > This class is deprecated |
There was a problem hiding this comment.
It'd be good to add some test cases for multiline messages with @deprecated to capture whether they work/don't work, and make sure that the final formatting is correct.
| class Foo {} | ||
| // ^^^ definition diagnostics 0.0.1 `index.ts`/Foo# | ||
| // diagnostic Information: | ||
| // > This class is deprecated |
There was a problem hiding this comment.
I don't have a strong feeling on this (we can change it later), but I'm wondering if it makes sense to emit this at the definition site as well. What does the Dart indexer do? I'm thinking of downstream functionality, and it generally doesn't make sense to flag warnings for definitions that are deprecated, but only at usage sites.
Problem/Solution
Currently the diagnostics field on the reported index file is always empty, regardless on if the typescript indexer detects any diagnostic issues
This information can be especially helpful analysis of a codebase, where questions like "how many deprecations are being used within this package" are asked. This is something that we rely pretty heavily on in scip-dart use cases (which fully indexes all analyzer results)
This PR updates the scip-typescript indexer to start indexing
@deprecatedjsdoc comments, and assigns the correctDeprecateddiagnostic tag. EventuallyDiagnosticTag.Unnecessaryshould be implemented, but this is less beneficial from my point of viewTest plan
A new
diagnosticssnapshot test was added. This include updating the snapshot generation to be in line with the results from the official scip clisnapshotcommand: sourcegraph/scip#226