savetheclocktower/pulsar-refactor

New version of `refactor` service that supports errors & introspection

Opened this issue · 0 comments

The original refactor service, at least as documented by atom-ide-base, says that an attempted rename will resolve with either

  • a table of edits grouped by file, or
  • null.

I'm assuming that null is the response that covers all the error cases. But the equivalent rename action in the LSP spec allows a language server to send an error code and error message if the attempt fails, so it'd be nice to have a version of the service that can communicate this information to the UI.

Proposal

type RefactorError = {
  code: number,
  message: string,
  data?: unknown
}

type RefactorRenameTextEdit = { uri: IdeUri, edit: TextEdit }
type RefactorRenameCreateFile = { uri: IdeUri }
// …et cetera

type RefactorRenameResponse = {
  changes: 
}

type RefactorPrepareRenameResponse = { valid: true } | {
  range: Range,
  placeholder?: string
} | null
export interface RefactorProviderV2 {
  packageName?: string
  title?: string
  grammarScopes: string[]
  priority: number
  rename(editor: TextEditor, position: Point, newName: string): Promise<Map<IdeUri, TextEdit[]> | RefactorError>,
  prepareRename?(editor: TextEditor, position: Point): Promise<RefactorPrepareRenameResponse | RefactorError>
}

The title and packageName fields are desired so that a user can ask pulsar-refactor what its providers are and receive a human-readable list. Right now there's no easy way to describe an arbitrary provider to the user; we don't have the right context.

The LSP spec says that textDocument/prepareRename can return null, unlike textDocument/rename; this signals to the client that the requested rename operation is not valid at the given location. This makes it even more important to distinguish a null response — which is technically valid and non-exceptional — with true error responses.

All possible null responses would be made illegal for rename; atom-languageclient could enforce this by falling back to a generic error if the underlying language server returned null instead of an error. (Which might not even be possible.)

If I implement something like this, I should bump the major version of the refactor service.


I didn't realize until now that our refactor support only extends to edits made to existing files; if a refactor involves creating, deleting, or renaming files, the service doesn't envision those sorts of changes.

There's a whole set of types that LSP makes available for describing these edits, but that would involve basically using the exact types of LSP in an IDE service contract, which hasn't been done before. The alternative would be to invent my own. Either way, this is a major complexity leap from what we already have.

@savetheclocktower/atom-languageclient can handle lots of this via the ApplyEditAdapter, but then that takes the responsibility of performing the task out of the hands of this package.