Skip to content
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
55 changes: 51 additions & 4 deletions packages/language-server/src/workspace.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { Connection, LanguageServer } from '@volar/language-server'
import type { CatalogsInfo, Engines } from 'npmx-language-core/types'
import type { DependencyInfo, PackageManager, WorkspaceAdapter } from 'npmx-language-core/workspace'
import type { ClientFeatures, IWorkspaceState } from 'npmx-language-service/types'
import { access, realpath as fsRealpath, readFile } from 'node:fs/promises'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect browser-related package configuration and Node built-in imports in source.
fd -a -t f 'package.json' . -x sh -c '
  printf "\n== %s ==\n" "$1"
  rg -n "\"browser\"|\"exports\"|\"main\"|\"module\"|\"types\"" "$1" || true
' sh {} \;

rg -n --glob '*.{ts,tsx}' "from 'node:|from \"node:" packages/language-server/src

Repository: npmx-dev/vscode-npmx

Length of output: 1543


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== repository guidance =='
find /tmp/coderabbit-repo-knowledge/npmx-dev-vscode-npmx-74af7664 -type f -name '*.md' -print \
  | sort
printf '%s\n' '== package and source context =='
cat -n packages/language-server/package.json
sed -n '1,35p' packages/language-server/src/workspace.ts
printf '%s\n' '== matching source imports =='
rg -n --glob '*.{ts,tsx}' "from ['\"]node:" packages/language-server/src

Repository: npmx-dev/vscode-npmx

Length of output: 4070


Remove the Node built-in import from src/.

packages/language-server/src/workspace.ts imports node:fs/promises, which violates the repository’s browser-compatibility boundary for **/src/**/*.{ts,tsx}. Move file-system access behind a host-specific adapter.

Source: Coding guidelines

import { CACHE_MAX_AGE_MAXIMUM, DEPENDENCY_FILE_GLOB, PACKAGE_JSON_BASENAME } from 'npmx-language-core/constants'
import { isDependencyFile, isPackageManifest } from 'npmx-language-core/utils'
import { isDependencyFile, isPackageManifest, normalizeCatalogName } from 'npmx-language-core/utils'
import { WorkspaceContext } from 'npmx-language-core/workspace'
import { DEFAULT_CLIENT_FEATURES } from 'npmx-language-service/types'
import { defineCachedFunction } from 'ocache'
Expand Down Expand Up @@ -163,16 +164,62 @@ export class WorkspaceState implements IWorkspaceState {
return bestMatch
}

async getWorkspaceContext(uriString: string): Promise<WorkspaceContext | undefined> {
async #getWorkspaceContext(uriString: string): Promise<WorkspaceContext | undefined> {
const folderUri = this.#getWorkspaceFolderUri(uriString)
if (!folderUri)
return

return await this.#getWorkspaceContextByFolder(folderUri)
}

async findCatalogDependency(uriString: string, dependency: DependencyInfo) {
const ctx = await this.#getWorkspaceContext(uriString)
if (!ctx?.workspaceFilePath)
return

const workspaceFileInfo = await ctx.loadWorkspaceFileInfo(ctx.workspaceFilePath)
const targetDependency = workspaceFileInfo?.dependencies.find((candidate) =>
candidate.rawName === dependency.resolvedName
&& candidate.categoryName != null
&& dependency.categoryName != null
&& normalizeCatalogName(candidate.categoryName) === normalizeCatalogName(dependency.categoryName),
)
if (!targetDependency)
return

return { dependency: targetDependency, path: ctx.workspaceFilePath }
}

async findInstalledPackageManifestPath(uriString: string, packageName: string): Promise<string | undefined> {
const ctx = await this.#getWorkspaceContext(uriString)
if (!ctx)
return

const uri = URI.parse(uriString)
if (uri.scheme !== 'file' || !isPackageManifest(uri.path))
return

return ctx.findInstalledPackageManifestPath(uri.path, packageName)
}

async getCatalogs(uriString: string): Promise<CatalogsInfo | undefined> {
return (await this.#getWorkspaceContext(uriString))?.getCatalogs()
}

async getPackageEngines(uriString: string): Promise<Engines | undefined> {
const ctx = await this.#getWorkspaceContext(uriString)
if (!ctx)
return

const uri = URI.parse(uriString)
if (uri.scheme !== 'file' || !isPackageManifest(uri.path))
return

return (await ctx.loadPackageManifestInfo(uri.path))?.engines
}

async getResolvedDependencies(uriString: string): Promise<DependencyInfo[] | undefined> {
const ctx = await this.getWorkspaceContext(uriString)
const ctx = await this.#getWorkspaceContext(uriString)
if (!ctx)
return

Expand All @@ -194,7 +241,7 @@ export class WorkspaceState implements IWorkspaceState {
}

async getResolvedDependenciesForContainingPackage(uriString: string): Promise<DependencyInfo[] | undefined> {
const ctx = await this.getWorkspaceContext(uriString)
const ctx = await this.#getWorkspaceContext(uriString)
if (!ctx)
return

Expand Down
33 changes: 6 additions & 27 deletions packages/language-service/src/plugins/catalog.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { CompletionItemKind, CompletionList, LanguageServicePlugin, LanguageServicePluginInstance, LocationLink } from '@volar/language-service'
import type { DependencyInfo } from 'npmx-language-core/workspace'
import type { IWorkspaceState } from '../types'
import { isPackageManifest, normalizeCatalogName } from 'npmx-language-core/utils'
import { isPackageManifest } from 'npmx-language-core/utils'
import { URI } from 'vscode-uri'
import { getDocumentByUri, getResolvedDependencySpecAtOffset } from '../utils/document'

Expand Down Expand Up @@ -33,13 +33,6 @@ export function create(workspaceState: IWorkspaceState): LanguageServicePlugin {
return getCatalogDependencyAtOffset(dependencies, offset)
}

function matchesCatalogDependency(candidate: DependencyInfo, dependency: DependencyInfo): boolean {
return candidate.rawName === dependency.resolvedName
&& candidate.categoryName != null
&& dependency.categoryName != null
&& normalizeCatalogName(candidate.categoryName) === normalizeCatalogName(dependency.categoryName)
}

return {
name: 'npmx-catalog',
capabilities: {
Expand All @@ -61,11 +54,7 @@ export function create(workspaceState: IWorkspaceState): LanguageServicePlugin {
if (!dependency)
return

const workspaceContext = await workspaceState.getWorkspaceContext(document.uri)
if (!workspaceContext)
return

const catalogs = await workspaceContext.getCatalogs()
const catalogs = await workspaceState.getCatalogs(document.uri)
if (!catalogs)
return

Expand Down Expand Up @@ -96,26 +85,16 @@ export function create(workspaceState: IWorkspaceState): LanguageServicePlugin {
if (!dependency)
return

const workspaceContext = await workspaceState.getWorkspaceContext(document.uri)
if (!workspaceContext?.workspaceFilePath)
return

const workspaceFileInfo = await workspaceContext.loadWorkspaceFileInfo(workspaceContext.workspaceFilePath)
if (!workspaceFileInfo)
return

const targetDependency = workspaceFileInfo.dependencies.find((candidate) =>
matchesCatalogDependency(candidate, dependency),
)
if (!targetDependency)
const target = await workspaceState.findCatalogDependency(document.uri, dependency)
if (!target)
return

const workspaceFileUri = dependencyFileUri.with({ path: workspaceContext.workspaceFilePath })
const workspaceFileUri = dependencyFileUri.with({ path: target.path })
const workspaceDocument = await getDocumentByUri(context, workspaceFileUri)
if (!workspaceDocument)
return

const [targetStart, targetEnd] = targetDependency.specRange
const [targetStart, targetEnd] = target.dependency.specRange
const originStart = document.positionAt(dependency.specRange[0])
const originEnd = document.positionAt(dependency.specRange[1])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface CreateContextOptions {
}

export function createContext(options: CreateContextOptions): DiagnosticContext {
const { name, version, distTags = {}, versionsMeta = {}, category = 'dependencies' } = options
const { name, version, distTags = {}, versionsMeta = {}, engines, category = 'dependencies' } = options
const { protocol, resolvedName, resolvedSpec, resolvedProtocol } = resolveDependencySpec(name, version)
const pkg = { distTags, versionsMeta } as PackageInfo

Expand All @@ -33,5 +33,26 @@ export function createContext(options: CreateContextOptions): DiagnosticContext
resolvedVersion: async () => resolveExactVersion(pkg, resolvedSpec),
packageInfo: async () => (pkg),
}
return { uri: 'file:///package.json', dep, pkg } as DiagnosticContext
const workspace: DiagnosticContext['workspace'] = {
async findCatalogDependency() {
return undefined
},
async findInstalledPackageManifestPath() {
return undefined
},
async getCatalogs() {
return undefined
},
getClientFeatures: () => ({ catalogInlayHints: true, iconStyle: 'emoji' }),
async getPackageEngines() {
return engines
},
async getResolvedDependencies() {
return undefined
},
async getResolvedDependenciesForContainingPackage() {
return undefined
},
}
return { uri: 'file:///package.json', dep, pkg, workspace }
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, it } from 'vitest'
import { resolveEngineMismatches } from './engine-mismatch'
import { createContext } from './__tests__/utils'
import { checkEngineMismatch, resolveEngineMismatches } from './engine-mismatch'

describe('resolveEngineMismatches', () => {
it('should flag when engine ranges do not overlap', () => {
Expand Down Expand Up @@ -62,3 +63,19 @@ describe('resolveEngineMismatches', () => {
)).toEqual([])
})
})

describe('checkEngineMismatch', () => {
it('reads package engines through the workspace interface', async () => {
await expect(checkEngineMismatch(
createContext({
name: 'foo',
version: '1.0.0',
engines: { node: '>=20' },
versionsMeta: {
'1.0.0': { engines: { node: '>=22' } },
},
}),
[],
)).resolves.toMatchObject({ code: 'engine-mismatch' })
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ import type { DiagnosticSeverity } from '@volar/language-service'
import type { Engines } from 'npmx-language-core/types'
import type { DiagnosticRule } from '../types'
import { npmxPackageUrl } from 'npmx-language-core/links'
import { formatPackageId, isPackageManifest } from 'npmx-language-core/utils'
import { formatPackageId } from 'npmx-language-core/utils'
import { isRangeSubset, parseRange, rangesIntersect } from 'verkit'
import { URI } from 'vscode-uri'

interface EngineMismatch {
engine: string
Expand Down Expand Up @@ -46,19 +45,14 @@ export function resolveEngineMismatches(
}

export const checkEngineMismatch: DiagnosticRule = async ({ uri, dep, pkg, workspace }) => {
const path = URI.parse(uri).path

if (!isPackageManifest(path))
return
if (dep.category !== 'dependencies')
return

const resolvedVersion = await dep.resolvedVersion()
if (!resolvedVersion)
return

const wsCtx = await workspace.getWorkspaceContext(uri)
const engines = (await wsCtx?.loadPackageManifestInfo(path))?.engines
const engines = await workspace.getPackageEngines(uri)
if (!engines)
return

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { IWorkspaceState } from '../types'
import { WorkspaceContext } from 'npmx-language-core/workspace'
import { describe, expect, it } from 'vitest'
import { TextDocument } from 'vscode-languageserver-textdocument'
import { URI } from 'vscode-uri'
import { createDependencyInfo } from '../test-utils/dependency'
import { DEFAULT_CLIENT_FEATURES } from '../types'
import { provideInstalledPackageDefinition } from './installed-package-definition'
Expand All @@ -26,9 +27,18 @@ async function createWorkspaceState(
const workspaceContext = await WorkspaceContext.create('/repo', adapter)

return {
async findCatalogDependency() {
return undefined
},
async findInstalledPackageManifestPath(uri, packageName) {
return workspaceContext.findInstalledPackageManifestPath(URI.parse(uri).path, packageName)
},
async getCatalogs() {
return undefined
},
getClientFeatures: () => DEFAULT_CLIENT_FEATURES,
async getWorkspaceContext() {
return workspaceContext
async getPackageEngines() {
return undefined
},
async getResolvedDependencies() {
return dependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,8 @@ export async function provideInstalledPackageDefinition(
if (!dependency)
return

const workspaceContext = await workspaceState.getWorkspaceContext(document.uri)
if (!workspaceContext)
return

const targetPath = await workspaceContext.findInstalledPackageManifestPath(
packageManifestUri.path,
const targetPath = await workspaceState.findInstalledPackageManifestPath(
document.uri,
dependency.rawName,
)
if (!targetPath)
Expand Down
11 changes: 9 additions & 2 deletions packages/language-service/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { DependencyInfo, WorkspaceContext } from 'npmx-language-core/workspace'
import type { CatalogsInfo, Engines } from 'npmx-language-core/types'
import type { DependencyInfo } from 'npmx-language-core/workspace'

export type IconStyle = 'codicon' | 'emoji'

Expand All @@ -14,7 +15,13 @@ export const DEFAULT_CLIENT_FEATURES: ClientFeatures = {

export interface IWorkspaceState {
getClientFeatures: () => ClientFeatures
getWorkspaceContext: (uri: string) => Promise<WorkspaceContext | undefined>
getCatalogs: (uri: string) => Promise<CatalogsInfo | undefined>
findCatalogDependency: (uri: string, dependency: DependencyInfo) => Promise<{
dependency: DependencyInfo
path: string
} | undefined>
getPackageEngines: (uri: string) => Promise<Engines | undefined>
getResolvedDependencies: (uri: string) => Promise<DependencyInfo[] | undefined>
getResolvedDependenciesForContainingPackage: (uri: string) => Promise<DependencyInfo[] | undefined>
findInstalledPackageManifestPath: (uri: string, packageName: string) => Promise<string | undefined>
}