Skip to content

Commit 02df611

Browse files
authored
Filter out "parameter missing" errors when not connected, fix flaky test suite (#688)
1 parent f399ae4 commit 02df611

10 files changed

Lines changed: 115 additions & 22 deletions

File tree

.changeset/major-beds-wave.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@neo4j-cypher/language-support": patch
3+
"@neo4j-cypher/react-codemirror": patch
4+
"@neo4j-cypher/language-server": patch
5+
---
6+
7+
Hide parameter errors when disconnected

packages/language-server/src/linting.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {
22
clampUnsafePositions,
3+
isNotParamError,
34
SymbolTable,
5+
SyntaxDiagnostic,
46
} from '@neo4j-cypher/language-support';
57
import { Neo4jSchemaPoller } from '@neo4j-cypher/query-tools';
68
import debounce from 'lodash.debounce';
@@ -63,12 +65,6 @@ async function rawLintDocument(
6365
});
6466
const result = await lastSemanticJob;
6567

66-
//marks the entire text if any position is negative
67-
const positionSafeResult = clampUnsafePositions(
68-
result.diagnostics,
69-
document,
70-
);
71-
7268
// Pass the computed symbol tables to the parser
7369
if (result.symbolTables) {
7470
languageService.setSymbolsInfo(
@@ -80,14 +76,30 @@ async function rawLintDocument(
8076
);
8177
}
8278

83-
sendDiagnostics(positionSafeResult);
79+
sendDiagnostics(
80+
filterLintResult(
81+
result.diagnostics,
82+
dbSchema?.databaseNames?.length ? false : true,
83+
document,
84+
),
85+
);
8486
} catch (err) {
8587
if (!(err instanceof workerpool.Promise.CancellationError)) {
8688
console.error(err);
8789
}
8890
}
8991
}
9092

93+
function filterLintResult(
94+
diagnostics: SyntaxDiagnostic[],
95+
dontWarnOnParams: boolean,
96+
document: TextDocument,
97+
) {
98+
return dontWarnOnParams
99+
? clampUnsafePositions(diagnostics.filter(isNotParamError), document)
100+
: clampUnsafePositions(diagnostics, document);
101+
}
102+
91103
export const lintDocument: typeof rawLintDocument = debounce(
92104
rawLintDocument,
93105
600,

packages/language-support/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export type { ParsedCypherToken } from './syntaxHighlighting/syntaxHighlightingH
2727
export {
2828
lintCypherQuery,
2929
clampUnsafePositions,
30+
isNotParamError,
3031
} from './syntaxValidation/syntaxValidation.js';
3132
export type { SyntaxDiagnostic } from './syntaxValidation/syntaxValidation.js';
3233
export { testData } from './tests/testData.js';

packages/language-support/src/syntaxValidation/syntaxValidation.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,17 @@ function detectNonDeclaredLabel(
8787
return undefined;
8888
}
8989

90+
type GenericDiagnostic = { message: string };
91+
92+
export function isNotParamError<T extends GenericDiagnostic>(
93+
diagnostic: T,
94+
): boolean {
95+
return (
96+
!diagnostic.message.startsWith('Parameter ') ||
97+
!diagnostic.message.endsWith(' is not defined.')
98+
);
99+
}
100+
90101
export function clampUnsafePositions(
91102
diagnostics: SyntaxDiagnostic[],
92103
document: TextDocument,
@@ -802,6 +813,8 @@ function warningOnDeprecatedFunction(
802813
return warnings;
803814
}
804815

816+
export const paramMsgStart = 'Parameter ';
817+
805818
function errorOnUndeclaredParameters(
806819
parsingResult: ParsedStatement,
807820
dbSchema: DbSchema,

packages/language-support/src/tests/misc.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { TextDocument } from 'vscode-languageserver-textdocument';
22
import {
33
clampUnsafePositions,
4+
isNotParamError,
45
SyntaxDiagnostic,
56
} from '../syntaxValidation/syntaxValidation.js';
67
import { DiagnosticSeverity, Position } from 'vscode-languageserver-types';
8+
import { getDiagnosticsForQuery } from './syntaxValidation/helpers.js';
9+
import { testData } from './testData.js';
710

811
describe('Clean positions', () => {
912
test('Cleaning positions should mark the entire document when a position is negative.', () => {
@@ -47,6 +50,24 @@ describe('Clean positions', () => {
4750
});
4851
});
4952

53+
test('Can filter out missing parameter warnings when disconnected using isNotParamError', () => {
54+
const query =
55+
'MATCH (n: Person) WHERE n.name = $`missing param` and n.age = $`some param` RETURN n';
56+
57+
expect(
58+
getDiagnosticsForQuery({
59+
query,
60+
dbSchema: {
61+
...testData.mockSchema,
62+
parameters: {
63+
'some param': 21,
64+
},
65+
databaseNames: [],
66+
},
67+
}).filter(isNotParamError),
68+
).toEqual([]);
69+
});
70+
5071
function testPositionCoversDoc(
5172
text: string,
5273
endLine: number,

packages/react-codemirror/src/lang-cypher/syntaxValidation.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { DiagnosticSeverity, DiagnosticTag } from 'vscode-languageserver-types';
44
import workerpool from 'workerpool';
55
import type { CypherConfig } from './langCypher';
66
import type { LintWorker } from '@neo4j-cypher/lint-worker';
7+
import { isNotParamError } from '@neo4j-cypher/language-support';
78

89
const WorkerURL = new URL('./lintWorker.mjs', import.meta.url).pathname;
910

@@ -44,8 +45,9 @@ export const cypherLinter: (config: CypherConfig) => Extension = (config) =>
4445

4546
const a: Diagnostic[] = result.diagnostics.map((diagnostic) => {
4647
return {
47-
from: diagnostic.offsets.start,
48-
to: diagnostic.offsets.end,
48+
from: diagnostic.offsets.start >= 0 ? diagnostic.offsets.start : 0,
49+
to:
50+
diagnostic.offsets.end >= 0 ? diagnostic.offsets.end : query.length,
4951
severity:
5052
diagnostic.severity === DiagnosticSeverity.Error
5153
? 'error'
@@ -57,6 +59,9 @@ export const cypherLinter: (config: CypherConfig) => Extension = (config) =>
5759
: {}),
5860
};
5961
});
62+
if (!config.schema?.databaseNames?.length) {
63+
return a.filter(isNotParamError);
64+
}
6065
return a;
6166
} catch (err) {
6267
if (!String(err).includes('Worker terminated')) {

packages/vscode-extension/tests/helpers.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ export async function eventually(
4444
timeoutMs = 15000,
4545
backoffMs = 100,
4646
) {
47+
// Cap the backoff so the retry interval stays small. Without a cap the
48+
// exponential growth means we both poll very rarely near the end (a correct
49+
// state can appear and not be noticed for tens of seconds) and overshoot
50+
// `timeoutMs` in wall-clock time by ~2x, so mocha's own timeout fires first
51+
// and hides the real assertion error.
52+
const maxBackoffMs = 5000;
4753
let totalWait = 0;
4854
let wait = backoffMs;
4955
let succeeded = false;
@@ -53,16 +59,15 @@ export async function eventually(
5359
await assertion();
5460
succeeded = true;
5561
} catch (e) {
56-
totalWait += wait;
5762
if (totalWait > timeoutMs) {
5863
throw new Error(
5964
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
6065
`Timeout of ${timeoutMs} exceeded for test with last error: ${e}`,
6166
);
62-
} else {
63-
wait *= 2;
64-
await sleep(wait);
6567
}
68+
await sleep(wait);
69+
totalWait += wait;
70+
wait = Math.min(wait * 2, maxBackoffMs);
6671
}
6772
}
6873
}

packages/vscode-extension/tests/specs/api/lintSwitching.spec.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,21 @@ suite('Lint switching spec', () => {
2929
let showErrorMessageSpy: sinon.SinonSpy;
3030

3131
before(async () => {
32+
textDocument = await newUntitledFileWithContent(`MATCH (n) RETURN m`);
33+
await saveDefaultConnection();
34+
await testSyntaxValidation({
35+
docUri: textDocument.uri,
36+
expected: [
37+
new vscode.Diagnostic(
38+
new vscode.Range(
39+
new vscode.Position(0, 17),
40+
new vscode.Position(0, 18),
41+
),
42+
'Variable `m` not defined',
43+
vscode.DiagnosticSeverity.Error,
44+
),
45+
],
46+
});
3247
textDocument = await newUntitledFileWithContent(`
3348
UNWIND range(1, 100) AS i
3449
CALL (i) {
@@ -50,7 +65,6 @@ suite('Lint switching spec', () => {
5065
'switchWorkerOnLanguageServer',
5166
);
5267
sendNotificationSpy = sandbox.spy(mockLanguageClient, 'sendNotification');
53-
await saveDefaultConnection();
5468
sandbox.resetHistory();
5569
});
5670

@@ -81,7 +95,7 @@ suite('Lint switching spec', () => {
8195
}
8296
}
8397

84-
test('Switching the linter to an old one should show different errors', async () => {
98+
test('Switching the linter to an old one should show different errors', async function () {
8599
const linterVersion = '5.26';
86100
const stub = sandbox.stub(
87101
window,
@@ -178,7 +192,7 @@ suite('Lint switching spec', () => {
178192
checkLinterUpdatedInLanguageServer('Default');
179193
});
180194

181-
test('Switching the linter back to a new one should show different errors', async () => {
195+
test('Switching the linter back to a new one should show different errors', async function () {
182196
const linterVersion = '2025.06';
183197
const stub = sandbox.stub(
184198
window,

packages/vscode-extension/tests/specs/api/syntaxValidation.spec.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@ import {
2020
type InclusionTestArgs = {
2121
docUri: vscode.Uri;
2222
expected: vscode.Diagnostic[];
23+
timeoutMs?: number;
2324
};
2425

2526
export async function testSyntaxValidation({
2627
docUri,
2728
expected,
29+
timeoutMs,
2830
}: InclusionTestArgs) {
2931
await eventually(
3032
() =>
@@ -66,6 +68,7 @@ export async function testSyntaxValidation({
6668
reject(e);
6769
}
6870
}),
71+
timeoutMs,
6972
);
7073
}
7174

@@ -74,7 +77,8 @@ suite('Syntax validation spec', () => {
7477
await toggleLinting(true);
7578
});
7679

77-
test('Suggests replacements for deprecated functions/procedures', async () => {
80+
test('Suggests replacements for deprecated functions/procedures', async function () {
81+
this.timeout(60000);
7882
const textFile = 'deprecated-by.cypher';
7983
const docUri = getDocumentUri(textFile);
8084

@@ -100,10 +104,12 @@ suite('Syntax validation spec', () => {
100104
vscode.DiagnosticSeverity.Warning,
101105
),
102106
],
107+
timeoutMs: 40000,
103108
});
104109
});
105110

106-
test('Relints when database connected / disconnected', async () => {
111+
test('Relints when database connected / disconnected', async function () {
112+
this.timeout(60000);
107113
const textFile = 'deprecated-by.cypher';
108114
const docUri = getDocumentUri(textFile);
109115

@@ -125,16 +131,19 @@ suite('Syntax validation spec', () => {
125131
await testSyntaxValidation({
126132
docUri,
127133
expected: deprecationErrors,
134+
timeoutMs: 40000,
128135
});
129136
await disconnectDefault();
130137
await testSyntaxValidation({
131138
docUri,
132139
expected: [],
140+
timeoutMs: 40000,
133141
});
134142
await connectDefault();
135143
await testSyntaxValidation({
136144
docUri,
137145
expected: deprecationErrors,
146+
timeoutMs: 40000,
138147
});
139148
});
140149

@@ -342,7 +351,8 @@ suite('Syntax validation spec', () => {
342351
});
343352
});
344353

345-
test('Linting can be disabled with the config option', async () => {
354+
test('Linting can be disabled with the config option', async function () {
355+
this.timeout(60000);
346356
const textFile = 'deprecated-by.cypher';
347357
const docUri = getDocumentUri(textFile);
348358

@@ -368,12 +378,14 @@ suite('Syntax validation spec', () => {
368378
vscode.DiagnosticSeverity.Warning,
369379
),
370380
],
381+
timeoutMs: 40000,
371382
});
372383

373384
await toggleLinting(false);
374385
await testSyntaxValidation({
375386
docUri,
376387
expected: [],
388+
timeoutMs: 40000,
377389
});
378390
});
379391
});

packages/vscode-extension/tests/specs/api/versionSpecificLinting.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ suite('Neo4j version specific linting spec', () => {
3232
await connectDefault({ version: 'neo4j 2025' });
3333
});
3434

35-
async function testNeo4jSpecificLinting() {
35+
async function testNeo4jSpecificLinting(timeoutMs?: number) {
3636
const textFile = 'cypher-versioned.cypher';
3737
const docUri = getDocumentUri(textFile);
3838
await openDocument(docUri);
@@ -49,6 +49,7 @@ suite('Neo4j version specific linting spec', () => {
4949
vscode.DiagnosticSeverity.Error,
5050
),
5151
],
52+
timeoutMs,
5253
});
5354

5455
await connectDefault({ version: 'neo4j 5' });
@@ -64,15 +65,17 @@ suite('Neo4j version specific linting spec', () => {
6465
vscode.DiagnosticSeverity.Error,
6566
),
6667
],
68+
timeoutMs,
6769
});
6870

6971
await connectDefault({ version: 'neo4j 2025' });
7072
}
7173

72-
test('Linting depends on the specific neo4j version and works when having to download new linters', async () => {
74+
test('Linting depends on the specific neo4j version and works when having to download new linters', async function () {
75+
this.timeout(60000);
7376
const lintersFolder = getExtensionStoragePath();
7477
rmSync(lintersFolder, { force: true, recursive: true });
75-
await testNeo4jSpecificLinting();
78+
await testNeo4jSpecificLinting(40000);
7679
});
7780

7881
test('Linting depends on the specific neo4j version and works when linters are already present', async () => {

0 commit comments

Comments
 (0)