Add support for context links to dart fix

[pdblasi-google]

There are plenty of reasons that a fix isn't supported by dart fix:

• The fix is too complex
• The original API has to many usage variations
• dart fix doesn't support a fix yet

In these cases, and possibly as an addition to automatible cases, it'd be nice to be able to provide a context link that could drive users to a migration doc.

Proposal

Add a new context field to the transform object. This field will accept a list of contextLink objects consisting of a title and uri. The titles of the links would be displayed in the context menu. They would show as an external link (with the appropriate icon and semantics) and clicking on them would open the link in a browser.

When fixes are applied through the terminal, either nothing would happen, or fixes with context would have the location of the error and the contextLink printed to stdout.

To support fixes that we can only provide context for, the requirements of transform would expand to allow:

• changes
• oneOf
• context
• changes and context, or
• oneOf and context

Examples

Adding context to an automated fix

version: 1
transforms:
  - title: 'Migrate to collate'
    date: 2023-03-29
    element:
      uris: [ 'flutter_test.dart' ]
      method: 'display'
      inClass: 'AnimationSheetBuilder'
    context:
      - title: 'Migration guide'
        uri: 'https://docs.flutter.dev/release/breaking-changes/animation-sheet-builder-display'
      - title: 'Deprecation PR'
        uri: 'https://github.com/flutter/flutter/pull/83337'
    changes:
      - kind: 'rename'
        newName: 'collate'
      - kind: 'removeParameter'
        name: 'key'
      - kind: 'addParameter'
        index: 0
        name: 'cellsPerRow'
        style: 'required_positional'
        argumentValue:
          expression: '1'

Context Menu

Adding context to a un-automated fix

version: 1
transforms:
  - title: 'Remove sheetSize'
    date: 2023-03-29
    element:
      uris: [ 'flutter_test.dart' ]
      method: 'sheetSize'
      inClass: 'AnimationSheetBuilder'
    context:
      - title: 'Migration guide'
        uri: 'https://docs.flutter.dev/release/breaking-changes/animation-sheet-builder-display'
      - title: 'Deprecation PR'
        uri: 'https://github.com/flutter/flutter/pull/83337'

Context Menu

[bwilkerson]

I like that idea. The more context we can provide, whether the fix is automated or not, the better for the user.

We might also be able to display that information with the diagnostic so that it's also visible for users that don't run dart fix or think to check the light-bulk menu. I think that's also worth investigating.

[keertip]

@DanTup , could you take a look at the above screenshots? Is this something we can surface in VS Code? Can we show links to urls as part of a CodeAction/Command?

[keertip]

@jwren, can we display documentation links along with the quick fix in IntelliJ/Android Studio? Does the API allow for this type of information to be sent and displayed?

[jwren]

We have an API for such notifications to be displayed in IJ, I have started but not finished, wiring up the required JSON.

[DanTup]

@keertip

could you take a look at the above screenshots? Is this something we can surface in VS Code? Can we show links to urls as part of a CodeAction/Command?

We can't add nested actions to items in the code actions menu like that, we can only supply a flat list (the groupings of fixes/refactors/etc. are hard-coded in VS Code based on the code action kinds). We could add additional items that open the docs, but they would likely be further down the menu (eg. not in the "Quick Fixes" section because they're not fixes).

I also don't think we can include markdown links in diagnostics (microsoft/vscode#54272). We can attach a URL to a diagnostic, but it's attached to the error code (that's the part that becomes clickable) and there's only one.

A question about the links though... If they are about the deprecation rather than the automated fix (the initial comment says some things won't have automated fixes?), might it be better to have them on the Deprecated class? That way they could easily be used by anyone without needing to create fix yaml files and the info is alongside the normal deprecation message?

[bwilkerson]

... might it be better to have them on the Deprecated class?

That would be better than nothing, but the problem with putting information on the annotation (and one of the major reasons why we have data files) is that when the deprecated API is removed the information about how to fix it disappears along with it.

Being able to put the URL in the data file has some advantages if we can figure out how to allow the user to follow the link.