-
Notifications
You must be signed in to change notification settings - Fork 13
Enables linter switching feature #542
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
Changes from all commits
911e449
f89ce4a
3518841
7c111ea
eb20220
f38f0ba
90a5147
28e2e14
f6f5660
ecafab1
fbe8455
6af5505
6fe412e
3971f45
14533c1
75b7cb9
a36ec7e
a07fc10
c785b26
96ad295
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ | |
"cypher", | ||
"lint worker" | ||
], | ||
"version": "0.1.0-next.0", | ||
"version": "1.10.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From now onwards you'd need to bump this by hand I suppose |
||
"repository": { | ||
"type": "git", | ||
"url": "git://github.com/neo4j/cypher-language-support.git" | ||
|
@@ -34,7 +34,6 @@ | |
"@neo4j-cypher/language-support": "workspace:*", | ||
"@neo4j-cypher/query-tools": "workspace:*", | ||
"languageSupport-next.13": "npm:@neo4j-cypher/[email protected]", | ||
"languageSupport-next.8": "npm:@neo4j-cypher/[email protected]", | ||
"vscode-languageserver": "^8.1.0", | ||
"vscode-languageserver-types": "^3.17.3", | ||
"workerpool": "^9.0.4", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,28 +7,26 @@ import { | |
import axios from 'axios'; | ||
import { DbSchema as DbSchemaV1 } from 'languageSupport-next.13'; | ||
|
||
const oldLinter = '5.20.0'; | ||
|
||
// for older versions of the language support, the dbschema was not the same, | ||
// meaning old linters need conversion of the new schema | ||
export function convertDbSchema( | ||
originalSchema: DbSchemaV2, | ||
linterVersion: string, | ||
): DbSchemaV2 | DbSchemaV1 { | ||
let oldFunctions: Record<string, Neo4jFunction> = {}; | ||
let oldProcedures: Record<string, Neo4jProcedure> = {}; | ||
if (!originalSchema) { | ||
return originalSchema; | ||
} | ||
if ( | ||
originalSchema.functions['CYPHER 5'] && | ||
originalSchema.procedures['CYPHER 5'] | ||
Comment on lines
-24
to
-25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was unsafe, the |
||
) { | ||
oldFunctions = originalSchema.functions['CYPHER 5']; | ||
oldProcedures = originalSchema.procedures['CYPHER 5']; | ||
} | ||
|
||
if (compareMajorMinorVersions(linterVersion, oldLinter) <= 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was not correct. The check needs to be any version lower than 2025.01, which is when we changed the schema. |
||
if (compareMajorMinorVersions(linterVersion, '2025.01') < 0) { | ||
let oldFunctions: Record<string, Neo4jFunction> | undefined = undefined; | ||
let oldProcedures: Record<string, Neo4jProcedure> | undefined = undefined; | ||
if (originalSchema.functions && originalSchema.functions['CYPHER 5']) { | ||
oldFunctions = originalSchema.functions['CYPHER 5']; | ||
} | ||
|
||
if (originalSchema.procedures && originalSchema.procedures['CYPHER 5']) { | ||
oldProcedures = originalSchema.procedures['CYPHER 5']; | ||
} | ||
const dbSchemaOld: DbSchemaV1 = { | ||
...originalSchema, | ||
functions: oldFunctions, | ||
|
@@ -40,12 +38,30 @@ export function convertDbSchema( | |
} | ||
} | ||
|
||
export function serverVersionToLinter(serverVersion: string) { | ||
let candidate: string = 'Latest'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the term |
||
if (compareMajorMinorVersions(serverVersion, oldLinter) <= 0) { | ||
candidate = oldLinter; | ||
export function serverVersionToLinter(serverVersion: string): { | ||
linterVersion: string; | ||
notResolved?: boolean; | ||
notSupported?: boolean; | ||
} { | ||
// Extract only the major and minor | ||
const versionRegex = /^\d+\.\d+/; | ||
const linterVersion = serverVersion.match(versionRegex)?.[0]; | ||
|
||
// If we have a version lower than 5.23, use that linter | ||
if (compareMajorMinorVersions(serverVersion, '5.23') < 0) { | ||
return { linterVersion: '5.23', notSupported: true }; | ||
// Unfortunately 2025.01, 2025.02 and 2025.03 all return 5.27 | ||
// so we have to assume we are on the most modern database from all those | ||
// | ||
// This case should never happen though because we should have cleaned up the | ||
// version by the moment we call this method | ||
} else if (compareMajorMinorVersions(serverVersion, '5.27') === 0) { | ||
return { linterVersion: '2025.03' }; | ||
} else if (linterVersion) { | ||
return { linterVersion: linterVersion }; | ||
} | ||
return candidate; | ||
|
||
return { linterVersion: 'Default', notResolved: true }; | ||
} | ||
|
||
export function linterFileToServerVersion(fileName: string) { | ||
|
@@ -64,7 +80,7 @@ export type NpmRelease = { | |
}; | ||
|
||
export const npmTagToLinterVersion = (tag: string) => | ||
tag.match(/^neo4j-([\d.]+)$/)?.[1]; | ||
tag.match(/^neo4j-(\d+\.\d+)$/)?.[1]; | ||
|
||
export async function getTaggedRegistryVersions(): Promise<NpmRelease[]> { | ||
const registryUrl = 'https://registry.npmjs.org/@neo4j-cypher/lint-worker'; | ||
|
@@ -74,7 +90,7 @@ export async function getTaggedRegistryVersions(): Promise<NpmRelease[]> { | |
const taggedVersions: { tag: string; version: string }[] = []; | ||
if (data !== null && data['dist-tags'] !== null) { | ||
for (const [tag, version] of Object.entries(data['dist-tags'])) { | ||
if (typeof tag === 'string' && typeof version === 'string') { | ||
if (npmTagToLinterVersion(tag)) { | ||
taggedVersions.push({ tag, version }); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ | |
{ | ||
"command": "neo4j.switchLinter", | ||
"category": "Neo4j", | ||
"title": "Pick linter" | ||
"title": "Select Cypher linter version" | ||
}, | ||
{ | ||
"command": "neo4j.editConnection", | ||
|
@@ -160,8 +160,7 @@ | |
], | ||
"commandPalette": [ | ||
{ | ||
"command": "neo4j.switchLinter", | ||
"when": "neo4j:useVersionedLinters" | ||
"command": "neo4j.switchLinter" | ||
}, | ||
{ | ||
"command": "neo4j.deleteConnection", | ||
|
@@ -445,16 +444,6 @@ | |
], | ||
"default": true, | ||
"description": "Enable linting errors for Cypher queries" | ||
}, | ||
"neo4j.features.useVersionedLinters": { | ||
"scope": "window", | ||
"type": "boolean", | ||
"enum": [ | ||
true, | ||
false | ||
], | ||
"default": false, | ||
"description": "Enable neo4j version specifc linting" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've cleaned up this feature flag |
||
} | ||
} | ||
}, | ||
|
@@ -478,9 +467,9 @@ | |
"dev": "tsc -b && pnpm gen-textmate && concurrently 'node --experimental-strip-types esbuild-extension.mts' 'pnpm bundle-webview-controllers --watch' ", | ||
"clean": "rm -rf {dist,tsconfig.tsbuildinfo}", | ||
"package": "pnpm vsce package --pre-release --no-dependencies", | ||
"test:e2e": "DEBUG_VSCODE_TESTS=false && pnpm build:dev && pnpm test:apiAndUnit && pnpm test:webviews", | ||
"test:apiAndUnit": "DEBUG_VSCODE_TESTS=false && pnpm build:dev && rm -rf .vscode-test/user-data && node ./dist/tests/runApiAndUnitTests.js", | ||
"test:webviews": "DEBUG_VSCODE_TESTS=false && pnpm build:dev && wdio run ./dist/tests/runWebviewTests.js" | ||
"test:e2e": "pnpm build:dev && cross-env DEBUG_VSCODE_TESTS=false pnpm test:apiAndUnit && cross-env DEBUG_VSCODE_TESTS=false pnpm test:webviews", | ||
"test:apiAndUnit": "pnpm build:dev && rm -rf .vscode-test/user-data && cross-env DEBUG_VSCODE_TESTS=false node ./dist/tests/runApiAndUnitTests.js", | ||
"test:webviews": "pnpm build:dev && cross-env DEBUG_VSCODE_TESTS=false wdio run ./dist/tests/runWebviewTests.js" | ||
Comment on lines
+470
to
+472
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a bug here (which did not have any implications until now) and the export VARIABLE=value && job or you have to prepend setting the variable just before every job. |
||
}, | ||
"dependencies": { | ||
"@neo4j-cypher/language-server": "workspace:*", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR cleans up this flag