Skip to content
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

Remove remnants from "LSIF Typed" #191

Merged
merged 2 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module.exports = {
parserOptions: {
project: 'tsconfig.json',
},
ignorePatterns: ['temp', 'lsif.ts', 'snapshots'],
ignorePatterns: ['temp', 'scip.ts', 'snapshots'],
rules: {
'no-sync': 'off',
},
Expand Down
2 changes: 1 addition & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
dist
node_modules
src/lsif.ts
src/scip.ts
snapshots/output
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,17 @@
"homepage": "https://github.com/sourcegraph/scip-typescript#readme",
"dependencies": {
"commander": "^9.2.0",
"google-protobuf": "^3.20.0",
"google-protobuf": "^3.20.1",
"pretty-ms": "^7.0.1",
"progress": "^2.0.3",
"typescript": "^4.5.4"
"typescript": "^4.8.4"
},
"devDependencies": {
"@sourcegraph/eslint-config": "0.32.0",
"@sourcegraph/prettierrc": "3.0.3",
"@sourcegraph/tsconfig": "4.0.1",
"@types/diff": "5.0.2",
"@types/google-protobuf": "3.15.5",
"@types/google-protobuf": "3.15.6",
"@types/node": "17.0.14",
"@types/pretty-ms": "5.0.1",
"@types/progress": "2.0.5",
Expand Down
2 changes: 1 addition & 1 deletion snapshots/input/syntax/src/issue-45.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export namespace example {
class Server {
// This overloaded method reproduces the following issue https://github.com/sourcegraph/lsif-typescript/issues/45
// This overloaded method reproduces the following issue https://github.com/sourcegraph/scip-typescript/issues/45
addListener(name: 'a'): void
addListener(name: 'b'): void
}
Expand Down
2 changes: 1 addition & 1 deletion snapshots/output/syntax/src/issue-45.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
class Server {
// ^^^^^^ definition syntax 1.0.0 src/`issue-45.d.ts`/example/Server#
// documentation ```ts\nclass Server\n```
// This overloaded method reproduces the following issue https://github.com/sourcegraph/lsif-typescript/issues/45
// This overloaded method reproduces the following issue https://github.com/sourcegraph/scip-typescript/issues/45
addListener(name: 'a'): void
// ^^^^^^^^^^^ definition syntax 1.0.0 src/`issue-45.d.ts`/example/Server#addListener().
// documentation ```ts\n(method) addListener(name: "a"): void\n```
Expand Down
2 changes: 1 addition & 1 deletion src/.gitattributes
Original file line number Diff line number Diff line change
@@ -1 +1 @@
lsif.ts linguist-generated=true
scip.ts linguist-generated=true
6 changes: 3 additions & 3 deletions src/CommandLineOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import ts from 'typescript'

import packageJson from '../package.json'

import * as lsif from './lsif'
import * as scip from './scip'

/** Configuration options to index a multi-project workspace. */
export interface MultiProjectOptions {
Expand All @@ -22,7 +22,7 @@ export interface MultiProjectOptions {
export interface ProjectOptions extends MultiProjectOptions {
projectRoot: string
projectDisplayName: string
writeIndex: (index: lsif.lib.codeintel.lsiftyped.Index) => void
writeIndex: (index: scip.scip.Index) => void
}

/** Cached values */
Expand All @@ -42,7 +42,7 @@ export function mainCommand(
.name('scip-typescript')
.version(packageJson.version)
.description(
'LSIF indexer for TypeScript and JavaScript\nFor usage examples, see https://github.com/sourcegraph/scip-typescript/blob/main/README.md'
'SCIP indexer for TypeScript and JavaScript\nFor usage examples, see https://github.com/sourcegraph/scip-typescript/blob/main/README.md'
)
command
.command('index')
Expand Down
10 changes: 5 additions & 5 deletions src/Descriptor.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import * as lsif from './lsif'
import * as scip from './scip'

type Descriptor = lsif.lib.codeintel.lsiftyped.Descriptor
const Descriptor = lsif.lib.codeintel.lsiftyped.Descriptor
type Suffix = lsif.lib.codeintel.lsiftyped.Descriptor.Suffix
const Suffix = lsif.lib.codeintel.lsiftyped.Descriptor.Suffix
type Descriptor = scip.scip.Descriptor
const Descriptor = scip.scip.Descriptor
type Suffix = scip.scip.Descriptor.Suffix
const Suffix = scip.scip.Descriptor.Suffix
Comment on lines +3 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can we somehow avoid this being scip.scip. It's not a big deal, but it looks a little gross lol.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree it's not nice and I'd +1 if we can solve it somehow. It's not a regression from this PR though, it was gross before as well


export function packageDescriptor(name: string): Descriptor {
return new Descriptor({ name, suffix: Suffix.Package })
Expand Down
94 changes: 45 additions & 49 deletions src/FileIndexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,23 @@ import {
typeParameterDescriptor,
} from './Descriptor'
import { Input } from './Input'
import * as lsif from './lsif'
import { LsifSymbol } from './LsifSymbol'
import { lsiftyped } from './main'
import { Packages } from './Packages'
import { Range } from './Range'
import * as scip from './scip'
import { ScipSymbol } from './ScipSymbol'
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Following JS/TS conventions, I think this should be called SCIPSymbol, similar to HTML and URL in browser APIs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I recall seeing in relation to XMLHttpRequest that the convention is that only three-letter acronym should remain uppercase. I really dislike SCIPSymbol, it's easier IMO to read ScipSymbol so I prefer to keep it

import * as ts_inline from './TypeScriptInternal'

type Descriptor = lsif.lib.codeintel.lsiftyped.Descriptor
type Relationship = lsif.lib.codeintel.lsiftyped.Relationship

export class FileIndexer {
private localCounter = new Counter()
private propertyCounters: Map<string, Counter> = new Map()
private localSymbolTable: Map<ts.Node, LsifSymbol> = new Map()
private localSymbolTable: Map<ts.Node, ScipSymbol> = new Map()
private workingDirectoryRegExp: RegExp
constructor(
public readonly checker: ts.TypeChecker,
public readonly options: ProjectOptions,
public readonly input: Input,
public readonly document: lsif.lib.codeintel.lsiftyped.Document,
public readonly globalSymbolTable: Map<ts.Node, LsifSymbol>,
public readonly document: scip.scip.Document,
public readonly globalSymbolTable: Map<ts.Node, ScipSymbol>,
public readonly packages: Packages,
public readonly sourceFile: ts.SourceFile
) {
Expand All @@ -45,21 +41,21 @@ export class FileIndexer {
this.visit(this.sourceFile)
}
private emitSourceFileOccurrence(): void {
const symbol = this.lsifSymbol(this.sourceFile)
const symbol = this.scipSymbol(this.sourceFile)
if (symbol.isEmpty()) {
return
}
this.document.occurrences.push(
new lsif.lib.codeintel.lsiftyped.Occurrence({
new scip.scip.Occurrence({
range: [0, 0, 0],
symbol: symbol.value,
symbol_roles: lsiftyped.SymbolRole.Definition,
symbol_roles: scip.scip.SymbolRole.Definition,
})
)
const moduleName =
this.sourceFile.moduleName || path.basename(this.sourceFile.fileName)
this.document.symbols.push(
new lsiftyped.SymbolInformation({
new scip.scip.SymbolInformation({
symbol: symbol.value,
documentation: ['```ts\nmodule "' + moduleName + '"\n```'],
})
Expand Down Expand Up @@ -106,24 +102,24 @@ export class FileIndexer {
let role = 0
const isDefinition = this.declarationName(node.parent) === node
if (isDefinition) {
role |= lsiftyped.SymbolRole.Definition
role |= scip.scip.SymbolRole.Definition
}
for (const declaration of sym?.declarations || []) {
const lsifSymbol = this.lsifSymbol(declaration)
const scipSymbol = this.scipSymbol(declaration)

if (lsifSymbol.isEmpty()) {
if (scipSymbol.isEmpty()) {
// Skip empty symbols
continue
}
this.document.occurrences.push(
new lsif.lib.codeintel.lsiftyped.Occurrence({
new scip.scip.Occurrence({
range,
symbol: lsifSymbol.value,
symbol: scipSymbol.value,
symbol_roles: role,
})
)
if (isDefinition) {
this.addSymbolInformation(node, sym, declaration, lsifSymbol)
this.addSymbolInformation(node, sym, declaration, scipSymbol)
this.handleShorthandPropertyDefinition(declaration, range)
this.handleObjectBindingPattern(node, range)
// Only emit one symbol for definitions sites, see https://github.com/sourcegraph/lsif-typescript/issues/45
Expand Down Expand Up @@ -153,14 +149,14 @@ export class FileIndexer {
const tpe = this.checker.getTypeAtLocation(node.parent.parent)
const property = tpe.getProperty(node.getText())
for (const declaration of property?.declarations || []) {
const lsifSymbol = this.lsifSymbol(declaration)
if (lsifSymbol.isEmpty()) {
const scipSymbol = this.scipSymbol(declaration)
if (scipSymbol.isEmpty()) {
continue
}
this.document.occurrences.push(
new lsif.lib.codeintel.lsiftyped.Occurrence({
new scip.scip.Occurrence({
range,
symbol: lsifSymbol.value,
symbol: scipSymbol.value,
})
)
}
Expand Down Expand Up @@ -190,14 +186,14 @@ export class FileIndexer {
return
}
for (const symbol of valueSymbol?.declarations || []) {
const lsifSymbol = this.lsifSymbol(symbol)
if (lsifSymbol.isEmpty()) {
const scipSymbol = this.scipSymbol(symbol)
if (scipSymbol.isEmpty()) {
continue
}
this.document.occurrences.push(
new lsif.lib.codeintel.lsiftyped.Occurrence({
new scip.scip.Occurrence({
range,
symbol: lsifSymbol.value,
symbol: scipSymbol.value,
})
)
}
Expand All @@ -210,7 +206,7 @@ export class FileIndexer {
node: ts.Node,
sym: ts.Symbol,
declaration: ts.Node,
symbol: LsifSymbol
symbol: ScipSymbol
): void {
const documentation = [
'```ts\n' +
Expand All @@ -223,7 +219,7 @@ export class FileIndexer {
}

this.document.symbols.push(
new lsiftyped.SymbolInformation({
new scip.scip.SymbolInformation({
symbol: symbol.value,
documentation,
relationships: this.relationships(declaration, symbol),
Expand All @@ -233,15 +229,15 @@ export class FileIndexer {

private relationships(
declaration: ts.Node,
declarationSymbol: LsifSymbol
): Relationship[] {
const relationships: Relationship[] = []
declarationSymbol: ScipSymbol
): scip.scip.Relationship[] {
const relationships: scip.scip.Relationship[] = []
const isAddedSymbol = new Set<string>()
const pushImplementation = (
node: ts.NamedDeclaration,
isReferences: boolean
): void => {
const symbol = this.lsifSymbol(node)
const symbol = this.scipSymbol(node)
if (symbol.isEmpty()) {
return
}
Expand All @@ -255,7 +251,7 @@ export class FileIndexer {
}
isAddedSymbol.add(symbol.value)
relationships.push(
new lsiftyped.Relationship({
new scip.scip.Relationship({
symbol: symbol.value,
is_implementation: true,
is_reference: isReferences,
Expand Down Expand Up @@ -309,19 +305,19 @@ export class FileIndexer {
return undefined
}

private lsifSymbol(node: ts.Node): LsifSymbol {
const fromCache: LsifSymbol | undefined =
private scipSymbol(node: ts.Node): ScipSymbol {
const fromCache: ScipSymbol | undefined =
this.globalSymbolTable.get(node) || this.localSymbolTable.get(node)
if (fromCache) {
return fromCache
}
if (ts.isBlock(node)) {
return LsifSymbol.empty()
return ScipSymbol.empty()
}
if (ts.isSourceFile(node)) {
const package_ = this.packages.symbol(node.fileName)
if (!package_) {
return this.cached(node, LsifSymbol.empty())
return this.cached(node, ScipSymbol.empty())
}
return this.cached(node, package_)
}
Expand All @@ -337,8 +333,8 @@ export class FileIndexer {
}
return this.cached(
node,
LsifSymbol.global(
this.lsifSymbol(node.getSourceFile()),
ScipSymbol.global(
this.scipSymbol(node.getSourceFile()),
metaDescriptor(`${node.name.getText()}${counter.next()}`)
)
)
Expand All @@ -362,7 +358,7 @@ export class FileIndexer {
const tpe = this.checker.getTypeOfSymbolAtLocation(props, node)
const property = tpe.getProperty(node.name.text)
for (const decl of property?.declarations || []) {
return this.lsifSymbol(decl)
return this.scipSymbol(decl)
}
} catch {
// TODO: https://github.com/sourcegraph/lsif-typescript/issues/34
Expand All @@ -373,25 +369,25 @@ export class FileIndexer {
}
}

const owner = this.lsifSymbol(node.parent)
const owner = this.scipSymbol(node.parent)
if (owner.isEmpty() || owner.isLocal()) {
return this.newLocalSymbol(node)
}

if (isAnonymousContainerOfSymbols(node)) {
return this.cached(node, this.lsifSymbol(node.parent))
return this.cached(node, this.scipSymbol(node.parent))
}

if (ts.isImportSpecifier(node) || ts.isImportClause(node)) {
const tpe = this.checker.getTypeAtLocation(node)
for (const declaration of tpe.symbol?.declarations || []) {
return this.lsifSymbol(declaration)
return this.scipSymbol(declaration)
}
}

const desc = this.descriptor(node)
if (desc) {
return this.cached(node, LsifSymbol.global(owner, desc))
return this.cached(node, ScipSymbol.global(owner, desc))
}

// Fallback case: generate a local symbol. It's not a bug when this case
Expand All @@ -401,16 +397,16 @@ export class FileIndexer {
return this.newLocalSymbol(node)
}

private newLocalSymbol(node: ts.Node): LsifSymbol {
const symbol = LsifSymbol.local(this.localCounter.next())
private newLocalSymbol(node: ts.Node): ScipSymbol {
const symbol = ScipSymbol.local(this.localCounter.next())
this.localSymbolTable.set(node, symbol)
return symbol
}
private cached(node: ts.Node, symbol: LsifSymbol): LsifSymbol {
private cached(node: ts.Node, symbol: ScipSymbol): ScipSymbol {
this.globalSymbolTable.set(node, symbol)
return symbol
}
private descriptor(node: ts.Node): Descriptor | undefined {
private descriptor(node: ts.Node): scip.scip.Descriptor | undefined {
if (
ts.isInterfaceDeclaration(node) ||
ts.isEnumDeclaration(node) ||
Expand Down
Loading