refactor(diagnostics): use structured action data (#156)

Author: 9romise

packages/language-service/src/plugins/diagnostics/actions.ts

@@ -1,20 +1,19 @@
 import type { CodeAction, CodeActionKind, Diagnostic } from '@volar/language-service'
+import type { DiagnosticActionData } from './types'
 import { ADD_TO_IGNORE_COMMAND } from 'npmx-shared/commands'
 import { ConfigurationTarget } from 'npmx-shared/constants'
-
-type MatchGroups = NonNullable<RegExpExecArray['groups']>
+import { displayName } from 'npmx-shared/meta'
 
 interface CodeActionDiagnosticContext {
   code: string
+  data: DiagnosticActionData
   documentUri: string
   diagnostic: Diagnostic
-  groups: MatchGroups
 }
 
 type ActionBuilder = (context: CodeActionDiagnosticContext) => CodeAction[]
 
 interface DiagnosticStrategy {
-  pattern: RegExp
   actionBuilders: ActionBuilder[]
 }
 
@@ -24,12 +23,12 @@ const ignoreScopes = [
 ]
 
 function quickFix(
-  resolveReplacement: (groups: MatchGroups) => string | undefined,
+  resolveReplacement: (data: DiagnosticActionData) => string | undefined,
   formatTitle: (replacement: string) => string,
   isPreferred = false,
 ): ActionBuilder {
   return (context) => {
-    const replacement = resolveReplacement(context.groups)
+    const replacement = resolveReplacement(context.data)
     if (!replacement)
       return []
 
@@ -50,9 +49,9 @@ function quickFix(
   }
 }
 
-function ignore(resolvePackageId: (groups: MatchGroups) => string | undefined): ActionBuilder {
+function ignore(resolvePackageId: (data: DiagnosticActionData) => string | undefined): ActionBuilder {
   return (context) => {
-    const packageId = resolvePackageId(context.groups)
+    const packageId = resolvePackageId(context.data)
     if (!packageId)
       return []
 
@@ -72,37 +71,61 @@ function ignore(resolvePackageId: (groups: MatchGroups) => string | undefined):
   }
 }
 
-export const strategies: Partial<Record<string, DiagnosticStrategy>> = {
+function resolveActionData(diagnostic: Diagnostic): DiagnosticActionData | undefined {
+  const data: unknown = diagnostic.data
+  if (typeof data !== 'object' || data === null)
+    return
+
+  return {
+    packageId: 'packageId' in data && typeof data.packageId === 'string' ? data.packageId : undefined,
+    packageName: 'packageName' in data && typeof data.packageName === 'string' ? data.packageName : undefined,
+    targetVersion: 'targetVersion' in data && typeof data.targetVersion === 'string' ? data.targetVersion : undefined,
+  }
+}
+
+const strategies: Partial<Record<string, DiagnosticStrategy>> = {
   upgrade: {
-    pattern: /^"(?<packageName>\S+)" can be upgraded to (?<targetVersion>[^"\s]+)\.$/,
     actionBuilders: [
-      quickFix((g) => g.targetVersion, (replacement) => `Upgrade to ${replacement}`),
-      ignore((g) => {
-        const targetVersion = g.targetVersion
-        if (!targetVersion)
+      quickFix((data) => data.targetVersion, (replacement) => `Upgrade to ${replacement}`),
+      ignore((data) => {
+        const { packageName, targetVersion } = data
+        if (!packageName || !targetVersion)
           return
 
-        return `${g.packageName}@${targetVersion}`
+        return `${packageName}@${targetVersion}`
       }),
     ],
   },
   vulnerability: {
-    pattern: /^"(?<packageId>\S+)" has .+ vulnerabilit(?:y|ies)\.(?: Upgrade to (?<targetVersion>\S+) to fix\.)?$/,
     actionBuilders: [
-      quickFix((g) => g.targetVersion, (replacement) => `Upgrade to ${replacement} to fix vulnerabilities`, true),
-      ignore((g) => g.packageId),
+      quickFix((data) => data.targetVersion, (replacement) => `Upgrade to ${replacement} to fix vulnerabilities`, true),
+      ignore((data) => data.packageId),
     ],
   },
   deprecation: {
-    pattern: /^"(?<packageId>\S+)" has been deprecated/,
     actionBuilders: [
-      ignore((g) => g.packageId),
+      ignore((data) => data.packageId),
     ],
   },
   replacement: {
-    pattern: /^"(?<packageName>\S+)"/,
     actionBuilders: [
-      ignore((g) => g.packageName),
+      ignore((data) => data.packageName),
     ],
   },
 }
+
+export function createCodeActions(documentUri: string, diagnostics: readonly Diagnostic[]): CodeAction[] {
+  return diagnostics.flatMap((diagnostic) => {
+    if (diagnostic.source !== displayName || !diagnostic.code)
+      return []
+
+    const code = String(diagnostic.code)
+    const strategy = strategies[code]
+    const data = resolveActionData(diagnostic)
+    if (!strategy || !data)
+      return []
+
+    const actionContext = { code, data, documentUri, diagnostic }
+    return strategy.actionBuilders.flatMap((build) => build(actionContext))
+  })
+}

packages/language-service/src/plugins/diagnostics/index.ts

@@ -1,12 +1,11 @@
-import type { CodeActionKind, LanguageServicePlugin, LanguageServicePluginInstance } from '@volar/language-service'
+import type { CodeActionKind, Diagnostic, LanguageServicePlugin, LanguageServicePluginInstance } from '@volar/language-service'
 import type { IWorkspaceState } from '../../types'
 import type { DiagnosticContext, DiagnosticRule } from './types'
 import { isDependencyFile } from 'npmx-language-core/utils'
 import { displayName } from 'npmx-shared/meta'
-import { Diagnostic } from 'vscode-languageserver-types'
 import { URI } from 'vscode-uri'
 import { getConfig } from '../../config'
-import { strategies } from './actions'
+import { createCodeActions } from './actions'
 import { checkDeprecation } from './rules/deprecation'
 import { checkDistTag } from './rules/dist-tag'
 import { checkEngineMismatch } from './rules/engine-mismatch'
@@ -112,26 +111,7 @@ export function create(workspaceState: IWorkspaceState): LanguageServicePlugin {
         },
 
         provideCodeActions(document, _range, codeActionContext) {
-          return codeActionContext.diagnostics.flatMap((diagnostic) => {
-            if (diagnostic.source !== displayName)
-              return []
-
-            if (!diagnostic.code)
-              return []
-
-            const code = String(diagnostic.code)
-            const strategy = strategies[code]
-            if (!strategy)
-              return []
-
-            const groups = strategy.pattern.exec(Diagnostic.getMessageString(diagnostic))?.groups
-            if (!groups)
-              return []
-
-            const actionContext = { code, documentUri: document.uri, diagnostic, groups }
-
-            return strategy.actionBuilders.flatMap((build) => build(actionContext))
-          })
+          return createCodeActions(document.uri, codeActionContext.diagnostics)
         },
       }
     },

packages/language-service/src/plugins/diagnostics/rules/deprecation.test.ts

@@ -25,6 +25,7 @@ describe('checkDeprecation', () => {
 
     expect(result).toMatchObject({
       code: 'deprecation',
+      data: { packageId: 'lodash@1.0.0' },
     })
     expect(result!.message).toMatchInlineSnapshot('""lodash@1.0.0" has been deprecated: old notice"')
   })
@@ -34,6 +35,7 @@ describe('checkDeprecation', () => {
 
     expect(result).toMatchObject({
       code: 'deprecation',
+      data: { packageId: 'lodash@1.2.0' },
     })
     expect(result!.message).toMatchInlineSnapshot('""lodash@1.2.0" has been deprecated: new notice"')
   })

packages/language-service/src/plugins/diagnostics/rules/deprecation.ts

@@ -16,11 +16,14 @@ export const checkDeprecation: DiagnosticRule = async ({ dep, pkg }, ignoreList)
   if (checkIgnored({ ignoreList, name: resolvedName, version: resolvedVersion }))
     return
 
+  const packageId = formatPackageId(resolvedName, resolvedVersion)
+
   return {
     range: specRange,
-    message: `"${formatPackageId(resolvedName, resolvedVersion)}" has been deprecated: ${versionInfo.deprecated}`,
+    message: `"${packageId}" has been deprecated: ${versionInfo.deprecated}`,
     severity: 1 satisfies typeof DiagnosticSeverity.Error,
     code: 'deprecation',
+    data: { packageId },
     codeDescription: { href: npmxPackageUrl(resolvedName, resolvedSpec) },
     tags: [2 satisfies typeof DiagnosticTag.Deprecated],
   }

packages/language-service/src/plugins/diagnostics/rules/replacement.test.ts

@@ -15,6 +15,9 @@ describe('checkReplacement', () => {
         "codeDescription": {
           "href": "https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/padStart",
         },
+        "data": {
+          "packageName": "left-pad",
+        },
         "message": ""left-pad" can be replaced with String.prototype.padStart.",
         "range": [
           0,

packages/language-service/src/plugins/diagnostics/rules/replacement.ts

@@ -48,6 +48,7 @@ export const checkReplacement: DiagnosticRule = async ({ dep: { nameRange, resol
     message: description,
     severity: 2 satisfies typeof DiagnosticSeverity.Warning,
     code: 'replacement',
+    data: { packageName: resolvedName },
     ...(link && { codeDescription: { href: link } }),
   }
 }

packages/language-service/src/plugins/diagnostics/rules/upgrade.test.ts

@@ -2,7 +2,7 @@ import type { PackageInfo } from 'npmx-language-core/api/package'
 import type { DependencyInfo } from 'npmx-language-core/workspace'
 import { describe, expect, it } from 'vitest'
 import { createContext } from './__tests__/utils'
-import { resolveUpgrade } from './upgrade'
+import { checkUpgrade, resolveUpgrade } from './upgrade'
 
 const distTags: Record<string, string> = {
   latest: '2.7.0',
@@ -22,8 +22,19 @@ async function createOptions(version: string): Promise<[DependencyInfo, PackageI
 }
 
 describe('resolveUpgrade', () => {
-  it('should flag when latest is greater than current version', async () => {
-    expect(resolveUpgrade(...await createOptions('^1.0.0'), [])).toBe('^2.7.0')
+  it('returns structured action data', async () => {
+    await expect(checkUpgrade(createContext({
+      name: 'vite',
+      version: '^1.0.0',
+      distTags,
+      versionsMeta,
+    }), [])).resolves.toMatchObject({
+      code: 'upgrade',
+      data: {
+        packageName: 'vite',
+        targetVersion: '^2.7.0',
+      },
+    })
   })
 
   it.each([

packages/language-service/src/plugins/diagnostics/rules/upgrade.ts

@@ -56,6 +56,10 @@ export const checkUpgrade: DiagnosticRule = async ({ dep, pkg }, ignoreList) =>
     severity: 4 satisfies typeof DiagnosticSeverity.Hint,
     message: `"${dep.resolvedName}" can be upgraded to ${targetVersion}.`,
     code: 'upgrade',
+    data: {
+      packageName: dep.resolvedName,
+      targetVersion,
+    },
     codeDescription: { href: npmxPackageUrl(dep.resolvedName, targetVersion) },
   }
 }

packages/language-service/src/plugins/diagnostics/rules/vulnerability.test.ts

@@ -10,6 +10,7 @@ describe('checkVulnerability', () => {
   it('should flag version with critical vulnerability', async () => {
     expect(await checkVulnerability(createVulnerabilityContext('pkg-crit'), [])).toMatchObject({
       code: 'vulnerability',
+      data: { packageId: 'pkg-crit@1.0.0' },
       message: expect.stringContaining('1 critical'),
     })
   })
@@ -23,6 +24,10 @@ describe('checkVulnerability', () => {
 
   it('should include fix suggestion when fixedIn is available', async () => {
     expect(await checkVulnerability(createVulnerabilityContext('pkg-fix'), [])).toMatchObject({
+      data: {
+        packageId: 'pkg-fix@1.0.0',
+        targetVersion: '1.2.0',
+      },
       message: expect.stringContaining('Upgrade to 1.2.0 to fix.'),
     })
   })

packages/language-service/src/plugins/diagnostics/rules/vulnerability.ts

@@ -59,15 +59,18 @@ export const checkVulnerability: DiagnosticRule = async ({ dep }, ignoreList) =>
     return
 
   const fixedInVersion = getBiggestFixedInVersion(vulnerablePackages)
-  const messageSuffix = fixedInVersion
-    ? ` Upgrade to ${formatUpgradeVersion(dep, fixedInVersion)} to fix.`
+  const packageId = formatPackageId(resolvedName, resolvedVersion)
+  const targetVersion = fixedInVersion ? formatUpgradeVersion(dep, fixedInVersion) : undefined
+  const messageSuffix = targetVersion
+    ? ` Upgrade to ${targetVersion} to fix.`
     : ''
 
   return {
     range: specRange,
-    message: `"${formatPackageId(resolvedName, resolvedVersion)}" has ${messageParts.join(', ')} ${messageParts.length === 1 ? 'vulnerability' : 'vulnerabilities'}.${messageSuffix}`,
+    message: `"${packageId}" has ${messageParts.join(', ')} ${messageParts.length === 1 ? 'vulnerability' : 'vulnerabilities'}.${messageSuffix}`,
     severity,
     code: 'vulnerability',
+    data: { packageId, targetVersion },
     codeDescription: { href: npmxPackageUrl(resolvedName, resolvedSpec) },
   }
 }