- 
                Notifications
    You must be signed in to change notification settings 
- Fork 317
[typespec-vscode] show NoTarget diagnostic message in vscode #8757
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
705643e
              4a58263
              2d30dbf
              f4d1265
              86040f0
              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,8 @@ | ||
| --- | ||
| # Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking | ||
| changeKind: fix | ||
| packages: | ||
| - "@typespec/compiler" | ||
| --- | ||
|  | ||
| Show NoTarget diagnostic message in vscode | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -3,41 +3,74 @@ import type { | |
| Range, | ||
| Diagnostic as VSDiagnostic, | ||
| } from "vscode-languageserver"; | ||
| import { Range as VSRange } from "vscode-languageserver"; | ||
| import type { TextDocument } from "vscode-languageserver-textdocument"; | ||
| import { DiagnosticSeverity } from "vscode-languageserver/node.js"; | ||
| import { | ||
| getDiagnosticTemplateInstantitationTrace, | ||
| getSourceLocation, | ||
| } from "../core/diagnostics.js"; | ||
| import { getTypeName } from "../core/helpers/type-name-utils.js"; | ||
| import { NodeSystemHost } from "../core/node-system-host.js"; | ||
| import type { Program } from "../core/program.js"; | ||
| import type { Node, SourceLocation } from "../core/types.js"; | ||
| import { Diagnostic } from "../core/types.js"; | ||
| import { Diagnostic, NoTarget } from "../core/types.js"; | ||
| import { isDefined } from "../utils/misc.js"; | ||
| import { getLocationInYamlScript } from "../yaml/diagnostics.js"; | ||
| import { parseYaml } from "../yaml/parser.js"; | ||
| import type { FileService } from "./file-service.js"; | ||
| import type { ServerSourceFile } from "./types.js"; | ||
|  | ||
| /** Convert TypeSpec Diagnostic to Lsp diagnostic. Each TypeSpec diagnostic could produce multiple lsp ones when it involve multiple locations. */ | ||
| export function convertDiagnosticToLsp( | ||
| export async function convertDiagnosticToLsp( | ||
| fileService: FileService, | ||
| program: Program, | ||
| document: TextDocument, | ||
| diagnostic: Diagnostic, | ||
| ): [VSDiagnostic, TextDocument][] { | ||
| const root = getVSLocation( | ||
| emitters?: string[] | undefined, | ||
| ): Promise<[VSDiagnostic, TextDocument][]> { | ||
| const emitterName = program.compilerOptions.emit?.find((emitName) => | ||
| diagnostic.message.includes(emitName), | ||
| ); | ||
| const root = await getVSLocation( | ||
| fileService, | ||
| getSourceLocation(diagnostic.target, { locateId: true }), | ||
| document, | ||
| program.compilerOptions.config, | ||
| emitterName, | ||
| ); | ||
| const relatedInformation: DiagnosticRelatedInformation[] = []; | ||
| if (root === NoTarget) { | ||
| let customMsg = ""; | ||
| if (emitters && emitters.includes(emitterName || "")) { | ||
| customMsg = ". It's configured in vscode settings."; | ||
| 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. " [...]", ". " may not be correct because you dont know the original message | ||
| } | ||
| 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. else { | ||
| return [ | ||
| [ | ||
| createLspDiagnostic({ | ||
| range: VSRange.create(0, 0, 0, 0), | ||
| message: diagnostic.message + customMsg, | ||
| severity: convertSeverity(diagnostic.severity), | ||
| code: diagnostic.code, | ||
| relatedInformation, | ||
| }), | ||
| document, | ||
| ], | ||
| ]; | ||
| } | ||
| if (root === undefined || !fileService.upToDate(root.document)) return []; | ||
|  | ||
| const instantiationNodes = getDiagnosticTemplateInstantitationTrace(diagnostic.target); | ||
| const relatedInformation: DiagnosticRelatedInformation[] = []; | ||
|  | ||
| const relatedDiagnostics: [VSDiagnostic, TextDocument][] = []; | ||
| if (instantiationNodes.length > 0) { | ||
| const items = instantiationNodes | ||
| .map((node) => getVSLocationWithTypeInfo(program, fileService, node, document)) | ||
| .filter(isDefined); | ||
| const items = ( | ||
| await Promise.all( | ||
| instantiationNodes.map((node) => | ||
| getVSLocationWithTypeInfo(program, fileService, node, document), | ||
| ), | ||
| ) | ||
| ).filter(isDefined); | ||
|  | ||
| for (const location of items) { | ||
| relatedInformation.push({ | ||
|  | @@ -127,11 +160,39 @@ interface VSLocation { | |
| readonly range: Range; | ||
| } | ||
|  | ||
| function getVSLocation( | ||
| async function getVSLocation( | ||
| fileService: FileService, | ||
| location: SourceLocation | undefined, | ||
| currentDocument: TextDocument, | ||
| ): VSLocation | undefined { | ||
| configFilePath: string | undefined = undefined, | ||
| emitterName: string | undefined = undefined, | ||
| ): Promise<VSLocation | typeof NoTarget | undefined> { | ||
| if (configFilePath && emitterName && emitterName.length > 0 && location === undefined) { | ||
| const [yamlScript] = parseYaml(await NodeSystemHost.readFile(configFilePath)); | ||
| 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. why do we need to load the yaml here? I think we did the same thing for the import error in tspconfig.yaml but didn't need this change. | ||
| const target = getLocationInYamlScript(yamlScript, ["emit", emitterName], "key"); | ||
| if (target.pos === 0) { | ||
| return NoTarget; | ||
| } | ||
|  | ||
| const docTspConfig = fileService.getOpenDocument(configFilePath); | ||
| if (!docTspConfig) { | ||
| return NoTarget; | ||
| } | ||
|  | ||
| const lineAndChar = target.file.getLineAndCharacterOfPosition(target.pos); | ||
| const range = VSRange.create( | ||
| lineAndChar.line, | ||
| lineAndChar.character, | ||
| lineAndChar.line, | ||
| lineAndChar.character + emitterName.length, | ||
| ); | ||
|  | ||
| return { | ||
| range, | ||
| document: docTspConfig, | ||
| }; | ||
| } | ||
|  | ||
| if (location === undefined) return undefined; | ||
| const document = getDocumentForLocation(fileService, location, currentDocument); | ||
| if (!document) { | ||
|  | @@ -148,18 +209,18 @@ interface VSLocationWithTypeInfo extends VSLocation { | |
| readonly typeName: string; | ||
| readonly node: Node; | ||
| } | ||
| function getVSLocationWithTypeInfo( | ||
| async function getVSLocationWithTypeInfo( | ||
| program: Program, | ||
| fileService: FileService, | ||
| node: Node, | ||
| document: TextDocument, | ||
| ): VSLocationWithTypeInfo | undefined { | ||
| const location = getVSLocation( | ||
| ): Promise<VSLocationWithTypeInfo | undefined> { | ||
| const location = await getVSLocation( | ||
| fileService, | ||
| getSourceLocation(node, { locateId: true }), | ||
| document, | ||
| ); | ||
| if (location === undefined) return undefined; | ||
| if (location === undefined || location === NoTarget) return undefined; | ||
| return { | ||
| ...location, | ||
| node, | ||
|  | ||
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.
emitters is confusing here, better to pass in the settings directly