Add support for moving symbols across packages with dart fix

[natebosch]

Possibly a duplicate of #44892, or may also be considered an extension.

We'd like to move some members across packages, and stop exporting others. The user visible changes is that where they previously had a single import to package:test/test.dart they will now also need a dependency on package:matcher/expect.dart as well as an extra pubspec dependency.

If we can't target the diagnostic about the undefined name for the fix we would be OK with a migration where we mark the exports as deprecated for a while so that the fixes can be applied. We'd need support for deprecating exports in that case. #23067

[bwilkerson]

@pq @keertip

[devoncarew]

Do we know who the owner for this item is? We'd like to ship support for this - for tooling to help us roll out the new package:checks - for the next stable SDK release.

[bwilkerson]

@pq and @keertip are actively working on adding this support. We now support the case where the old export of expect has been removed, but not the case where the export has been marked as deprecated. The reason for that is the issue that was recently linked to this issue by @pq.

[devoncarew]

Ah, great to hear! I didn't know this was in progress.

[pq]

Yeah. #23067 is I think the blocker for @natebosch's use case.

[natebosch]

We are getting things in place to move these APIs from test.

I could add the deprecations soon, but I'd ideally like to have fix support in place when I do. Can this be prioritized?

[bwilkerson]

@jacob314 How does this prioritize relative to language feature support or analytics work?

[natebosch]

We can use dart-lang/test#1978 to test the implementation when we start on it.

The end result should be:

• Add import to package:matcher/expect.dart in any test with a deprecation diagnostic.
• (Rarely) remove an import to package:test/test.dart if there were no other APIs being used. (Could skip this and let dart fix fix the unused import in the next run?)
• Add a dependency to macher in the pubspec.yaml file.

I'm happy to add a dart fix config file to the PR whenever we have an idea of what the syntax will be for this type of move.

[bwilkerson]

• Add import to package:matcher/expect.dart in any test with a deprecation diagnostic.

That would be a good fix to have in general, but doesn't require data-driven fix support. Any time there's a deprecated_import_use we should be able to see which library it's declared in and add an explicit import. We might even have enough information in the element model to import it using the same URI as was in the export directive.

The fix doesn't currently exist.

• (Rarely) remove an import to package:test/test.dart if there were no other APIs being used. (Could skip this and let dart fix fix the unused import in the next run?)

Yes. We always leave import clean-up until the very last pass. It avoids a large number of possible bugs.

• Add a dependency to macher in the pubspec.yaml file.

That would be really nice. We've talked a few times about ways to update the pubspec.yaml file, but we don't currently have any support for doing so.

[natebosch]

Is this something we can start on soon? We are getting closer to being ready to deprecate the export of matcher from test

[natebosch]

but doesn't require data-driven fix support

Do we think it's sufficient for dart fix to only work during the intermediate period where we have the export but it is deprecated? Or should we also look for a way to express this in the data-driven config for it to keep working if the user jumps straight to a version without the export at all?

[jacob314]

I think it is important for the fix to still work after the migration period. Users will encounter documentation that omits the package:matcher import for a long time and LLMs will likely be confused and suggest code that omits the matcher import.

[mit-mit]

Hey there, is there a tentative timeline for this?

[pq]

@keertip: are you planning to look at this?

[keertip]

no, had not planned to.

[pq]

/fyi @srawlins @bwilkerson @jacob314

[srawlins]

I don't think we have any timeline for this. It is not in our queue for Q2, but I think we can prioritize it for Q3.

Correct me if I'm wrong but it sounds like the remaining AIs are:

• detect when an export has been deprecated as a triggering mechanism (from Add support for moving symbols across packages with dart fix #48997 (comment))
• add the import if needed (from Add support for moving symbols across packages with dart fix #48997 (comment))
• add a dependency (always dev_dependency?) to pubspec.yaml.

I'll note that we do not have a quick fix for depend_on_referenced_packages, and I'm not certain that it's possible. @bwilkerson you note above that:

We always leave import clean-up until the very last pass. It avoids a large number of possible bugs.

It sounds like it would be a natural step to add a step after import clean-up that adds and removes packages from dependencies and dev_dependencies. "Natural," but I don't want to imply "simple." Also... as far as dev_dependencies go, I think maybe you would only want to add dependencies, but never remove (they're not always there for use as imports).

[natebosch]

Correct me if I'm wrong but it sounds like the remaining AIs are:

I think we also want support for encoding this in dart_fix.yaml. #48997 (comment)

[bwilkerson]

It sounds like it would be a natural step to add a step after import clean-up that adds and removes packages from dependencies and dev_dependencies.

Yes, the clean-up-imports pass seems like a good time to update dependencies.

We'd need to collect all of the diagnostics used to find the imported packages for which a dependency needs to be added, then de-dup the list of dependencies so that we can build a consistent set of edits, but other than that it sounds reasonable.

[keertip]

Changes to dart fix to add missing dependencies to pubspec have landed.