-
Notifications
You must be signed in to change notification settings - Fork 51
fix(language-server): handle Windows file URIs and excluded schema directories #1997
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises' | ||
| import { tmpdir } from 'node:os' | ||
| import path from 'node:path' | ||
| import { afterEach, describe, expect, test } from 'vitest' | ||
| import { TextDocument } from 'vscode-languageserver-textdocument' | ||
| import { URI } from 'vscode-uri' | ||
| import { PrismaSchema } from '../lib/Schema' | ||
|
|
||
| const tempDirectories: string[] = [] | ||
|
|
||
| afterEach(async () => { | ||
| await Promise.all(tempDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))) | ||
| }) | ||
|
|
||
| describe('PrismaSchema', () => { | ||
| test('excludes schema files from configured directories', async () => { | ||
| const root = await mkdtemp(path.join(tmpdir(), 'prisma-language-server-')) | ||
| tempDirectories.push(root) | ||
|
|
||
| const activeDirectory = path.join(root, 'active') | ||
| const excludedDirectory = path.join(root, 'excluded') | ||
|
|
||
| await mkdir(activeDirectory) | ||
| await mkdir(excludedDirectory) | ||
|
|
||
| const activeSchemaPath = path.join(activeDirectory, 'schema.prisma') | ||
| const excludedSchemaPath = path.join(excludedDirectory, 'schema.prisma') | ||
|
|
||
| const activeContent = ` | ||
| model Active { | ||
| id Int @id | ||
| } | ||
| ` | ||
|
|
||
| const excludedContent = ` | ||
| model Excluded { | ||
| id Int @id | ||
| } | ||
| ` | ||
|
|
||
| await writeFile(activeSchemaPath, activeContent) | ||
| await writeFile(excludedSchemaPath, excludedContent) | ||
|
|
||
| const activeDocument = TextDocument.create(URI.file(activeSchemaPath).toString(), 'prisma', 1, activeContent) | ||
|
|
||
| const excludedDocument = TextDocument.create(URI.file(excludedSchemaPath).toString(), 'prisma', 1, excludedContent) | ||
|
|
||
| const schema = await PrismaSchema.load( | ||
| { | ||
| currentDocument: activeDocument, | ||
| allDocuments: [activeDocument, excludedDocument], | ||
| }, | ||
| root, | ||
| { | ||
| excludedSchemaDirectories: ['excluded'], | ||
| }, | ||
| ) | ||
|
|
||
| expect(schema.documents).toHaveLength(1) | ||
| expect(schema.documents[0].uri).toBe(activeDocument.uri) | ||
| expect(schema.documents[0].content).toContain('model Active') | ||
| expect(schema.documents[0].content).not.toContain('model Excluded') | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,14 @@ import { prismaSchemaWasm } from '.' | |
| import { handleFormatPanic, handleWasmError } from './internals' | ||
| import { PrismaSchema } from '../Schema' | ||
| import { TextDocument } from 'vscode-languageserver-textdocument' | ||
| import { URI } from 'vscode-uri' | ||
|
|
||
| export function isSameFileUri(leftUri: string, rightUri: string, platform = process.platform): boolean { | ||
| const leftPath = URI.parse(leftUri).fsPath | ||
| const rightPath = URI.parse(rightUri).fsPath | ||
|
|
||
| return platform === 'win32' ? leftPath.toLocaleLowerCase() === rightPath.toLocaleLowerCase() : leftPath === rightPath | ||
|
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
file='packages/language-server/src/lib/prisma-schema-wasm/format.ts'
printf '%s\n' '--- target file ---'
cat -n "$file"
printf '%s\n' '--- package/runtime declarations ---'
rg -n --glob 'package.json' --glob '*.yml' --glob '*.yaml' --glob '*.md' '"(engines|node|Node|Windows|win32)"' . | head -120
printf '%s\n' '--- related symbols ---'
rg -n 'isSameFileUri|formatResult|toLocaleLowerCase|toLowerCase' packages/language-serverRepository: prisma/language-tools Length of output: 4842 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- package engines ---'
cat -n package.json | sed -n '1,24p'
printf '%s\n' '--- format tests ---'
cat -n packages/language-server/src/__test__/format.test.ts
printf '%s\n' '--- shared Windows path comparison ---'
cat -n packages/language-server/src/lib/Schema.ts | sed -n '70,108p'
printf '%s\n' '--- available Node semantic probe ---'
if command -v node >/dev/null 2>&1; then
node - <<'JS'
console.log(JSON.stringify({
node: process.version,
locale: Intl.DateTimeFormat().resolvedOptions().locale,
invariant: 'I'.toLocaleLowerCase('en-US'),
turkishI: 'I'.toLocaleLowerCase('tr'),
turkishLowerI: 'i'.toLocaleLowerCase('tr'),
defaultI: 'I'.toLocaleLowerCase(),
defaultLowerI: 'i'.toLocaleLowerCase(),
invariantPairEqual: 'I'.toLocaleLowerCase('en-US') === 'i'.toLocaleLowerCase('en-US'),
turkishPairEqual: 'I'.toLocaleLowerCase('tr') === 'i'.toLocaleLowerCase('tr')
}))
JS
else
echo 'node unavailable'
fiRepository: prisma/language-tools Length of output: 4998 🌐 Web query:
💡 Result: In JavaScript and Node.js, String.prototype.toLocaleLowerCase performs locale-specific case mapping based on the environment's default locale if no locale is specified [1][2]. On Windows systems where the system locale is set to Turkish (tr-TR), calling this method on characters like 'I' will produce the Turkish dotless 'ı' instead of the standard Unicode 'i' [1][3][2]. Because toLocaleLowerCase is locale-dependent, its results can vary between different systems or environments (e.g., development vs. CI servers) [1][4]. For filesystem operations on Windows, this behavior is generally problematic for path normalization and comparison. Windows filesystems (NTFS, FAT32, exFAT) are inherently case-insensitive but preserve case [5][6][7]. However, they do not use Turkish locale-specific case mapping to resolve paths [4]. If you use toLocaleLowerCase to normalize paths for comparison on a system with a Turkish locale, you may generate incorrect or "canonical" path strings that do not match the actual files on disk [4]. To perform reliable, invariant filesystem path comparisons on Windows, you should: 1. Avoid toLocaleLowerCase for path normalization [4]. 2. Use standard String.prototype.toLowerCase or a custom invariant mapping if strictly required, as these follow standard Unicode case folding rather than locale-sensitive rules [1][2]. 3. Recognize that because Windows is case-insensitive, paths are effectively treated as equivalent regardless of case [5][7]. For robust path comparison, it is often safer to rely on OS-level APIs or tools that resolve paths to their actual disk representation (e.g., realpath) rather than performing manual string case manipulation [7][4]. In summary, do not use toLocaleLowerCase for path-related tasks on Windows, as the Turkish locale issue can cause path resolution failures that break cross-platform consistency [4]. Use case-insensitive comparison logic that is independent of the host's regional settings [8][4]. Citations:
Use locale-independent case conversion for Windows paths.
🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| export default function format( | ||
| schema: PrismaSchema, | ||
|
|
@@ -21,12 +29,16 @@ export default function format( | |
| } | ||
|
|
||
| const result = prismaSchemaWasm.format(JSON.stringify(schema), JSON.stringify(options)) | ||
|
|
||
| // tuples of [filePath, content] | ||
| const formattedFiles = JSON.parse(result) as Array<[string, string]> | ||
| const formatResult = formattedFiles.find(([uri]) => uri === initiatingDocument.uri) | ||
|
|
||
| const formatResult = formattedFiles.find(([uri]) => isSameFileUri(uri, initiatingDocument.uri)) | ||
|
|
||
| if (!formatResult) { | ||
| return initiatingDocument.getText() | ||
| } | ||
|
|
||
| return formatResult[1] | ||
| } catch (e) { | ||
| const err = e as Error | ||
|
|
||
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pass the matched workspace-folder root to
PrismaSchema.load.getDocumentSettingsloadsprismasettings for the document URI, soexcludedSchemaDirectoriescan contain workspace-relative paths.loadSchemapassesundefined, which makesPrismaSchema.loadresolve each relative path fromprocess.cwd(). If the workspace root differs fromprocess.cwd(), the configured directory is not excluded, and its schema files can contribute diagnostics. Pass the matchingInitializeParams.workspaceFoldersroot, or the single-workspacerootUri, as the filesystemconfigRoot. Add coverage with different process and workspace roots.🤖 Prompt for AI Agents