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

Add support for SymbolInformation.signature #231

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

olafurpg
Copy link
Member

@olafurpg olafurpg commented Feb 6, 2024

Test plan

@olafurpg
Copy link
Member Author

olafurpg commented Feb 7, 2024

Changes I'd like to make on the SemanticDB types:

  • Remove Scope because we use symlinks in 99% of all cases, and there exists is a workaround for the <1% case (just emit a separate SymbolInformation). For repeated Scope we need to add dedicated message on a case-by-case basis. For example, it makes to add ParameterList
  • Replace ConstantType with nested constant types. The additional layer of nesting is just annoying to work with. I prefer to have ConstantStringType, ConstantIntType, etc. even if it makes the Type enum bigger (which is not a problem)
  • Still unsure, but I'm not 100% satisfied with the ad-hoc solution for primitive/built-in types (string,number,...). One one hand, I like reusing TypeRef with magic symbols because of simplicity. On the other hand, given most compilers have special encodings for primitives, maybe we should just add dedicated Type to represent them. One benefit of this approach is that you can write a generic helper function to detect string/number/int types across different languages without having to enumerate special string values.

@olafurpg
Copy link
Member Author

olafurpg commented Feb 9, 2024

  • On a second thought, having separate Constant message allows code reuse between types and terms. It's especially desirable to add terms for annotation arguments.
  • We should consider adding TypeSignature.value or something equivalent for basic type A = B signatures. The current lower/upper bound design is to support type A >: B in Scala, but we should not sacrifice ergonomics for the >99% use-case.

olafurpg added a commit to sourcegraph/scip-typescript that referenced this pull request Feb 13, 2024
Companion PRs:

* sourcegraph/scip#231
* sourcegraph/cody#3142

Previously, scip-typescript didn't emit any structured information about
signatures. This PR solves that problem, which unblocks new exciting
use-cases for SCIP.

Keeping this PR open for a while since we are not planning to merge
signature support to SCIP just yet. For now, the Cody repo can use this
branch instead.
olafurpg added a commit to sourcegraph/scip-typescript that referenced this pull request Feb 13, 2024
Companion PRs:

* sourcegraph/scip#231
* sourcegraph/cody#3142

Previously, scip-typescript didn't emit any structured information about
signatures. This PR solves that problem, which unblocks new exciting
use-cases for SCIP.

Keeping this PR open for a while since we are not planning to merge
signature support to SCIP just yet. For now, the Cody repo can use this
branch instead.
olafurpg added a commit to sourcegraph/scip-typescript that referenced this pull request Feb 13, 2024
Companion PRs:

* sourcegraph/scip#231
* sourcegraph/cody#3142

Previously, scip-typescript didn't emit any structured information about
signatures. This PR solves that problem, which unblocks new exciting
use-cases for SCIP.

Keeping this PR open for a while since we are not planning to merge
signature support to SCIP just yet. For now, the Cody repo can use this
branch instead.
olafurpg added a commit to sourcegraph/cody that referenced this pull request Feb 13, 2024
Until now, we have been manually writing Kotlin bindings for the agent
protocol. This worked fine when only a small number of people worked on
the protocol and the protocol was small, but has become increasingly
problematic as more people are actively working on the JetBrains plugin
and the size of the Agent protocol has grown. For example, if you make
a mistake in the bindings, you may end up spending a long time debugging
why something is not working because the error messages can be cryptic.

This PR adds a new script to automatically generate Kotlin bindings so
that we no longer need to rely on manually written bindings. Simplified,
the script works like this:

- Index the codebase with a fork of scip-typescript. This fork uses an
  extension of SCIP that adds `SymbolInformation.signature` (more
  details here sourcegraph/scip#231).
- Use the structured type signatures from the SCIP index, translate
  it into Kotlin code, and report errors if something in the protocol
  has no clean translation to Kotlin.
- Add the generated code to the Cody repo so that we can closely track
  what Cody PRs are influencing the protocol, and how they're
  influencing it.
olafurpg added a commit to sourcegraph/cody that referenced this pull request Feb 14, 2024
Until now, we have been manually writing Kotlin bindings for the agent
protocol. This worked fine when only a small number of people worked on
the protocol and the protocol was small, but has become increasingly
problematic as more people are actively working on the JetBrains plugin
and the size of the Agent protocol has grown. For example, if you make a
mistake in the bindings, you may end up spending a long time debugging
why something is not working because the error messages can be cryptic.

This PR adds a new script to automatically generate Kotlin bindings so
that we no longer need to rely on manually written bindings. Simplified,
the script works like this:

- Index the codebase with a fork of scip-typescript. This fork uses an
extension of SCIP that adds `SymbolInformation.signature` (more details
here sourcegraph/scip#231).
- Use the structured type signatures from the SCIP index, translate it
into Kotlin code, and report errors if something in the protocol has no
clean translation to Kotlin.
- Add the generated code to the Cody repo so that we can closely track
what Cody PRs are influencing the protocol, and how they're influencing
it.


## Test plan

Green CI

<!-- Required. See
https://sourcegraph.com/docs/dev/background-information/testing_principles.
-->
@mkondratek
Copy link

Hello Olaf 👋 I wanted to make use of the generated bindings and I found an issue:

export interface ChatHistory {
    [chatID: string]: TranscriptJSON
}

turns into

data class ChatHistory(
  val chatID: String? = null,
)

(and hence - I guess - TranscriptJSON is not being generated)

olafurpg added a commit to sourcegraph/scip-typescript that referenced this pull request Jun 12, 2024
Companion PRs:

* sourcegraph/scip#231
* sourcegraph/cody#3142

Previously, scip-typescript didn't emit any structured information about
signatures. This PR solves that problem, which unblocks new exciting
use-cases for SCIP.

Keeping this PR open for a while since we are not planning to merge
signature support to SCIP just yet. For now, the Cody repo can use this
branch instead.
olafurpg added a commit to sourcegraph/scip-typescript that referenced this pull request Jun 17, 2024
Companion PRs:

* sourcegraph/scip#231
* sourcegraph/cody#3142

Previously, scip-typescript didn't emit any structured information about
signatures. This PR solves that problem, which unblocks new exciting
use-cases for SCIP.

Keeping this PR open for a while since we are not planning to merge
signature support to SCIP just yet. For now, the Cody repo can use this
branch instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants