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

fix: off-by-one error in token range (regression in #15). #244

Merged
merged 4 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/new-chairs-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@0no-co/graphqlsp": patch
---

fix case where the hover-information would target the wrong TypeScript node by one character
4 changes: 2 additions & 2 deletions packages/graphqlsp/src/ast/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ export const getToken = (

let foundToken: Token | undefined = undefined;
for (let line = 0; line < input.length; line++) {
const lPos = cPos;
const lPos = cPos - 1;
const stream = new CharacterStream(input[line] + '\n');
while (!stream.eol()) {
const token = parser.token(stream, state);
const string = stream.current();

if (
lPos + stream.getStartOfToken() <= cursorPosition &&
lPos + stream.getStartOfToken() + 1 <= cursorPosition &&
lPos + stream.getCurrentPosition() >= cursorPosition
) {
foundToken = {
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/fixture-project-tada/fixtures/fragment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,14 @@ export const PokemonFields = graphql(`
}
`);

// prettier-ignore
export const Regression190 = graphql(`
fragment pokemonFields on Pokemon {
id
name
fleeRate

}
`);

export const Pokemon = () => {};
6 changes: 6 additions & 0 deletions test/e2e/fixture-project/fixtures/simple.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,11 @@ const PostsQuery = gql`
}
`;

const Regression190 = gql`
Copy link
Member

Choose a reason for hiding this comment

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

Does this also fail for graphql() function invocations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, yeah. Want me to add that to the fixture as well? I pushed a fix btw!

Copy link
Member

Choose a reason for hiding this comment

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

Would love a fixture in the tada one as we're slowly moving more towards the function invocation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify, Regression190 succeeds on main, but Regression15 does not. Regression190 is there to ensure that I didn't break #190 again while fixing #15.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would love a fixture in the tada one as we're slowly moving more towards the function invocation

Sure! One sec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added!

query AllPosts {

}
`;

const sql = (x: string | TemplateStringsArray) => x;
const x = sql`'{}'`;
76 changes: 74 additions & 2 deletions test/e2e/graphqlsp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,15 @@ describe('simple', () => {
]);
}, 7500);

it('Gives quick-info when hovering', async () => {
it('Gives quick-info when hovering start (#15)', async () => {
server.send({
seq: 9,
type: 'request',
command: 'quickinfo',
arguments: {
file: testFile,
line: 5,
offset: 7,
Copy link
Member

Choose a reason for hiding this comment

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

I assume 7 still works, just clarifying

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, yeah, I just changed it to make it a tricker case

offset: 5,
},
});

Expand All @@ -135,4 +135,76 @@ describe('simple', () => {
`Query.posts: [Post]\n\nList out all posts`
);
}, 7500);

it('Handles empty line (#190)', async () => {
server.send({
seq: 10,
type: 'request',
command: 'completionInfo',
arguments: {
file: testFile,
line: 14,
offset: 3,
includeExternalModuleExports: true,
includeInsertTextCompletions: true,
triggerKind: 1,
},
});

await server.waitForResponse(
response =>
response.type === 'response' && response.command === 'completionInfo'
);

const res = server.responses
.reverse()
.find(
resp => resp.type === 'response' && resp.command === 'completionInfo'
);

expect(res).toBeDefined();
expect(typeof res?.body.entries).toEqual('object');
const defaultAttrs = { kind: 'var', kindModifiers: 'declare' };
expect(res?.body.entries).toEqual([
{
...defaultAttrs,
name: 'post',
sortText: '0post',
labelDetails: { detail: ' Post' },
},
{
...defaultAttrs,
name: 'posts',
sortText: '1posts',
labelDetails: { detail: ' [Post]', description: 'List out all posts' },
},
{
...defaultAttrs,
name: '__typename',
sortText: '2__typename',
labelDetails: {
detail: ' String!',
description: 'The name of the current Object type at runtime.',
},
},
{
...defaultAttrs,
name: '__schema',
sortText: '3__schema',
labelDetails: {
detail: ' __Schema!',
description: 'Access the current type schema of this server.',
},
},
{
...defaultAttrs,
name: '__type',
sortText: '4__type',
labelDetails: {
detail: ' __Type',
description: 'Request the type information of a single type.',
},
},
]);
}, 7500);
});
190 changes: 190 additions & 0 deletions test/e2e/tada.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,4 +367,194 @@ List out all Pokémon, optionally in pages`
]
`);
}, 30000);

it('gives quick-info at start of word (#15)', async () => {
server.send({
seq: 11,
type: 'request',
command: 'quickinfo',
arguments: {
file: outfileCombinations,
line: 7,
offset: 5,
},
});

await server.waitForResponse(
response =>
response.type === 'response' && response.command === 'quickinfo'
);

const res = server.responses
.reverse()
.find(resp => resp.type === 'response' && resp.command === 'quickinfo');

expect(res).toBeDefined();
expect(typeof res?.body).toEqual('object');
expect(res?.body.documentation).toEqual(`Pokemon.name: String!`);
}, 30000);

it('gives suggestions with empty line (#190)', async () => {
server.send({
seq: 12,
type: 'request',
command: 'completionInfo',
arguments: {
file: outfileCombinations,
line: 19,
offset: 3,
includeExternalModuleExports: true,
includeInsertTextCompletions: true,
triggerKind: 1,
},
});

await server.waitForResponse(
response =>
response.type === 'response' && response.command === 'completionInfo'
);

const res = server.responses
.reverse()
.find(
resp => resp.type === 'response' && resp.command === 'completionInfo'
);

expect(res).toBeDefined();
expect(typeof res?.body.entries).toEqual('object');
expect(res?.body.entries).toMatchInlineSnapshot(`
[
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " AttacksConnection",
},
"name": "attacks",
"sortText": "0attacks",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " [EvolutionRequirement]",
},
"name": "evolutionRequirements",
"sortText": "2evolutionRequirements",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " [Pokemon]",
},
"name": "evolutions",
"sortText": "3evolutions",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"description": "Likelihood of an attempt to catch a Pokémon to fail.",
"detail": " Float",
},
"name": "fleeRate",
"sortText": "4fleeRate",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " PokemonDimension",
},
"name": "height",
"sortText": "5height",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " ID!",
},
"name": "id",
"sortText": "6id",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"description": "Maximum combat power a Pokémon may achieve at max level.",
"detail": " Int",
},
"name": "maxCP",
"sortText": "7maxCP",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"description": "Maximum health points a Pokémon may achieve at max level.",
"detail": " Int",
},
"name": "maxHP",
"sortText": "8maxHP",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " String!",
},
"name": "name",
"sortText": "9name",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " [PokemonType]",
},
"name": "resistant",
"sortText": "10resistant",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " [PokemonType]",
},
"name": "types",
"sortText": "11types",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " [PokemonType]",
},
"name": "weaknesses",
"sortText": "12weaknesses",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"detail": " PokemonDimension",
},
"name": "weight",
"sortText": "13weight",
},
{
"kind": "var",
"kindModifiers": "declare",
"labelDetails": {
"description": "The name of the current Object type at runtime.",
"detail": " String!",
},
"name": "__typename",
"sortText": "14__typename",
},
]
`);
}, 30000);
});
Loading