| # Refactors |
| |
| LSP: [textDocument_codeActions][] request |
| Legacy: `edit.getAssists`, `edit.getAvailableRefactorings`, `edit.getFixes`, |
| and `edit.getRefactoring` requests |
| |
| Refactors are actions that users can select that make changes to their code. |
| |
| ## Kinds |
| |
| While users tend to think of all refactors as essentially being the same, |
| internally we divide them into three kinds. This is partially due to |
| implementation considerations, but more importantly because there are |
| implications in terms of the UX. |
| |
| The kinds are based on two characteristics of the refactor: |
| |
| <dl> |
| <dt>edit creation time</dt> |
| <dd> |
| <p> |
| An _eager refactor_ is one in which the code changes are computed before the |
| code action is returned. If code edits can't be computed, then the code action |
| isn't returned. This ensures that if the user selects the action it will |
| successfully apply the edits. |
| </p><p> |
| A _lazy refactor_ is one in which the code action is returned without |
| computing the code edits. Typically some minimal set of checks will be |
| performed so that the code action is only shown when the refactor makes sense, |
| but it's still possible for the refactor to fail after the user has selected |
| it. In those cases we display a message telling the user why it failed. |
| </p> |
| </dd> |
| <dt>availability</dt> |
| <dd> |
| Some refactors are only available when there is a diagnostic indicating a |
| problem. Others are available whenever the editor's selection is on the |
| appropriate tokens. |
| </dd> |
| </dl> |
| |
| The three kinds are described below. |
| |
| ### Fixes |
| |
| Fixes are code changes that are designed to resolve a problem in the code that |
| is indicated by a diagnostic. The changes are always computed eagerly. |
| |
| Fixes can be initiated to |
| - fix a single diagnostic at a single location in a single file |
| - fix all of the locations in a single file where a single diagnostic is |
| reported |
| - fix all of the locations of all diagnostics in all files in the workspace |
| (via `dart fix` and in LSP-based IDEs) |
| |
| ### Assists |
| |
| Assists are code changes that are available even when there is no diagnostic. |
| The changes are always computed eagerly. |
| |
| Assists can only be initiated at a single location in a single file. |
| |
| ### Global Refactors |
| |
| Global refactors are code changes that might involve changes to multiple |
| libraries and possibly across multiple packages, when those packages are all |
| open in the IDE's workspace. Global refactors are always computed lazily. |
| |
| Global refactors can only be initiated at a single location in a single file. |
| |
| Note that in many contexts (such as the issue tracker) we use the term |
| 'refactor' to sometimes mean any kind of refactor and sometimes to mean a global |
| refactor. In this document we'll use the longer name for clarity. |
| |
| ## Preserving semantics |
| |
| There is no rule against refactors that change the semantics of the code. Some |
| refactors are only useful _because_ they change the semantics. It could be |
| argued that most of the fixes are semantics changing: taking the code from being |
| broken to being compilable. This section discusses the criteria we use to decide |
| when it's appropriate for a refactor to be semantic preserving and when it's |
| reasonable for it to change the semantics. |
| |
| ### User expectations |
| |
| One question that should be asked is how likely it is that a user would |
| reasonably expect the semantics to be preserved. For example, it's reasonable |
| for a user to assume that a refactor that converts a switch statement into a |
| switch expression would preserve the semantics of the switch. On the other hand, |
| it's reasonable for a user to expect that a refactor that changes a method to be |
| marked as `async` and changes the return type to be a `Future` would change the |
| semantics of the code by doing so. |
| |
| ### Subtle vs. obvious changes |
| |
| If a refactor is going to change the semantics of the code then it ought to be |
| obvious to the user that the semantics have changed. The more subtle the change |
| to the semantics, the less appropriate it is for the semantics to change. For |
| example, the assist that converts a method to be marked as `async` changes the |
| semantics, but the change is easy to see because the return type is changed and |
| a new keyword is added. On the other hand, a change that impacts the lookup |
| scope in such a way that some identifiers are resolved to different targets |
| without any indication that this is the case is probably too subtle. |
| |
| If a fix is being applied at a single location, then the semantic changes will |
| generally be more obvious. If a fix is being applied across a large code base, |
| then the semantic changes might easily not be noticed because the affected files |
| might not be open. |
| |
| ## Producing broken code |
| |
| There are few, if any, valid reasons for a refactor to produce code that doesn't |
| compile. There are a couple of known exceptions: |
| |
| - Some refactors will work on code that is already broken, in which case |
| it's reasonable for the result to also be broken, as long as it isn't broken |
| worse. But it usually isn't reasonable for a refactor to introduce new |
| diagnostics into the code. |
| |
| - If the client allows the server to notify the user of the situation and the |
| user indicates that they want to proceed, then it makes sense to proceed with |
| the refactor. |
| |
| ## Generated code |
| |
| Most refactors produce code as part of their function. This section describes |
| the practices that should be followed when generating code. |
| |
| One of the best ways to adhere to these practices is to use the utilities |
| defined in the `DartFileEditBuilder` and `DartEditBuilder` classes when |
| generating code. |
| |
| ### Style |
| |
| The generated code should be as close as possible to what the user would likely |
| write. For example, if we believe that users are more likely to write methods |
| with block bodies than with expression bodies, then we should produce block |
| bodies when generating a method. Similarly, if we believe that most users use |
| `async` for methods that return a `Future`, then we should include the modifier |
| when generating such a method. |
| |
| The generated code should, where possible, follow the style that is enforced by |
| the enabled lints. For example, if the lint is enabled that enforces the use of |
| one delimiter for string literals, then any string literals that are generated |
| should use that delimiter. |
| |
| The generated code should be formatted similar to what the formatter produces. |
| Reasonable attempts are made to generate code with appropriate indentation and |
| space characters between tokens, but generated code does not need to include |
| line-wrapping, intentionally added or omitted commas, etc. If we get the |
| ability to run the formatter over portions of a file, then we should use the |
| formatter on generated code. |
| |
| ### Incomplete code |
| |
| Sometimes it's necessary to generate incomplete code. For example, when |
| generating a method based on an example invocation, the server can infer the |
| number and types of the parameters and even the return type, but can't know how |
| to implement the method. |
| |
| When incomplete code is generated, it should be as obvious as possible that the |
| code is incomplete. There are two ways to do this: generating code that has a |
| compilation error or generating a `TODO` comment as part of the generated code. |
| For example, if a method that returns a value is being generated, then we could |
| choose to generate code that returns `null`. However, if the return type is |
| nullable, then there won't be any signal that the method is incomplete. It would |
| be better to generate code that doesn't return a value. |
| |
| As a counter-example, when the server generates an override of a concrete method |
| it adds a super-invocation of the overridden method. If the method returns a |
| value then we return the value from the overridden method. That leaves the code |
| with no diagnostics, so we add a `TODO` comment. It also leaves the code being |
| semantically equivalent. That might seem like it violates the advice above, but |
| technically the code isn't incomplete, even though it usually isn't what the |
| user intends to write. |
| |
| [textDocument_codeActions]: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeActions |