diff --git a/.changeset/stale-donkeys-grow.md b/.changeset/stale-donkeys-grow.md new file mode 100644 index 00000000..047a3120 --- /dev/null +++ b/.changeset/stale-donkeys-grow.md @@ -0,0 +1,5 @@ +--- +'@neo4j-cypher/language-support': patch +--- + +Fix incorrect completions when identifier overlaps with keyword diff --git a/packages/language-support/src/autocompletion/completionCoreCompletions.ts b/packages/language-support/src/autocompletion/completionCoreCompletions.ts index 0ba03efd..853fb45f 100644 --- a/packages/language-support/src/autocompletion/completionCoreCompletions.ts +++ b/packages/language-support/src/autocompletion/completionCoreCompletions.ts @@ -11,7 +11,12 @@ import CypherParser, { Expression2Context, } from '../generated-parser/CypherParser'; import { rulesDefiningVariables } from '../helpers'; -import { CypherTokenType, lexerSymbols, tokenNames } from '../lexerSymbols'; +import { + CypherTokenType, + lexerKeywords, + lexerSymbols, + tokenNames, +} from '../lexerSymbols'; import { EnrichedParsingResult, ParsingResult } from '../parserWrapper'; const uniq = (arr: T[]) => Array.from(new Set(arr)); @@ -282,7 +287,13 @@ export function completionCoreCompletion( // The identfier is finished when the last token is a SPACE or dot etc. etc. // this allows us to give completions that replace the current text => for example `RET` <- it's parsed as an identifier // The need for this caret movement is outlined in the documentation of antlr4-c3 in the section about caret position - if (tokens[caretIndex - 1]?.type === CypherLexer.IDENTIFIER) { + // When an identifer overlaps with a keyword, it's no longer treates as an identifier (although it's a valid identifier) + // So we need to move the caret back for keywords as well + const previousToken = tokens[caretIndex - 1]?.type; + if ( + previousToken === CypherLexer.IDENTIFIER || + lexerKeywords.includes(previousToken) + ) { caretIndex--; } diff --git a/packages/language-support/src/tests/autocompletion/databasename-completion.test.ts b/packages/language-support/src/tests/autocompletion/databasename-completion.test.ts index b9abad10..838b51a2 100644 --- a/packages/language-support/src/tests/autocompletion/databasename-completion.test.ts +++ b/packages/language-support/src/tests/autocompletion/databasename-completion.test.ts @@ -319,7 +319,7 @@ describe('can complete database names', () => { test('suggests parameter in options field of create constraint', () => { const query = - 'CREATE CONSTRAINT abc ON (n:person) ASSERT EXISTS n.name OPTIONS'; + 'CREATE CONSTRAINT abc ON (n:person) ASSERT EXISTS n.name OPTIONS '; testCompletions({ query, dbSchema, @@ -359,14 +359,14 @@ describe('can complete database names', () => { test('suggests parameters for user management', () => { const cases = [ - 'CREATE USER', - 'DROP USER', - 'ALTER USER', - 'RENAME USER', - 'SHOW USER', - 'ALTER CURRENT USER SET PASSWORD FROM', + 'CREATE USER ', + 'DROP USER ', + 'ALTER USER ', + 'RENAME USER ', + 'SHOW USER ', + 'ALTER CURRENT USER SET PASSWORD FROM ', 'ALTER CURRENT USER SET PASSWORD FROM $pw to ', - 'ALTER USER', + 'ALTER USER ', 'ALTER USER foo IF EXISTS SET PASSWORD ', ]; cases.forEach((query) => { @@ -386,11 +386,11 @@ describe('can complete database names', () => { test('suggests parameters for role management', () => { const cases = [ - 'CREATE ROLE', - 'DROP ROLE', - 'RENAME ROLE', - 'GRANT ROLE', - 'GRANT ROLE abc TO', + 'CREATE ROLE ', + 'DROP ROLE ', + 'RENAME ROLE ', + 'GRANT ROLE ', + 'GRANT ROLE abc TO ', ]; cases.forEach((query) => { testCompletions({ @@ -418,8 +418,8 @@ describe('can complete database names', () => { 'DEALLOCATE DATABASES FROM SERVERS "ab", ', ]; const optionsCases = [ - 'ENABLE SERVER "abc" OPTIONS', - 'ALTER SERVER "abc" SET OPTIONS', + 'ENABLE SERVER "abc" OPTIONS ', + 'ALTER SERVER "abc" SET OPTIONS ', ]; nameCases.forEach((query) => { diff --git a/packages/language-support/src/tests/autocompletion/misc-completions.test.ts b/packages/language-support/src/tests/autocompletion/misc-completions.test.ts index cd0b3e87..98006d82 100644 --- a/packages/language-support/src/tests/autocompletion/misc-completions.test.ts +++ b/packages/language-support/src/tests/autocompletion/misc-completions.test.ts @@ -23,6 +23,25 @@ describe('Misc auto-completion', () => { }); }); + test('Requires space to complete next keyword', () => { + const query = 'SHOW'; + + testCompletions({ + query, + expected: [{ label: 'SHOW', kind: CompletionItemKind.Keyword }], + excluded: [{ label: 'DATABASE', kind: CompletionItemKind.Keyword }], + }); + }); + + test('Completes keyword, even when current word is also valid keyword', () => { + const query = 'SHOW DATA'; + + testCompletions({ + query, + expected: [{ label: 'DATABASE', kind: CompletionItemKind.Keyword }], + }); + }); + test('Correctly completes DISTINCT', () => { const query = 'MATCH (n:Person)-[r:KNOWS]-(m:Person) RETURN '; diff --git a/packages/language-support/src/tests/autocompletion/variable-completion.test.ts b/packages/language-support/src/tests/autocompletion/variable-completion.test.ts index 6dc03b8c..ce9c4a2f 100644 --- a/packages/language-support/src/tests/autocompletion/variable-completion.test.ts +++ b/packages/language-support/src/tests/autocompletion/variable-completion.test.ts @@ -38,7 +38,7 @@ describe('unscoped variable completions', () => { }); test('suggests both variables after renaming variable', () => { - const query = 'MATCH (n:Person) WITH n as m RETURN'; + const query = 'MATCH (n:Person) WITH n as m RETURN '; testCompletions({ query, expected: [ @@ -49,7 +49,7 @@ describe('unscoped variable completions', () => { }); test('does not suggest variable when renaming variable', () => { - const query = 'MATCH (n:Person) WITH n as'; + const query = 'MATCH (n:Person) WITH n as '; testCompletions({ query, @@ -58,7 +58,7 @@ describe('unscoped variable completions', () => { }); test('does not suggest variables when unwinding ', () => { - const query = 'MATCH (n:Person) UNWIND [] as'; + const query = 'MATCH (n:Person) UNWIND [] as '; testCompletions({ query,