Analyzer data-based fix request

[lrhn]

trafficstars

I'm trying to do an automated migration from one (now deprecated) API to another, and is stuck on thing I cannot express within the current features.

One legacy class named FakePlatform should be replaced by a new (and different) class also named FakePlatform from another (new) library. If name conflicts are not solvable, replacing it with the type alias FakePlatformAliasForMigration is OK. We can fix that later, as long as the code keeps running.

I've already migrated all member accesses to use the new API. What's left is the type itself, and constructor invocations.

Possible migrations that could solve my problem:

1. "replacedBy" being able to introduce a new import. This would replace a constructor on the legacy class with a differently named constructor on the new class and import the new class. Bonus points for allowing show/hide on the new import.
2. "replacedBy" being able to replace a class/type, or "renamed" working across libraries. Still needs to add an import for a new library. Bonus points of also being able to hide the name on the original import. (If there are multiple fixes in the same file, the import should only be added once, and if hiding, it shouldn't prevent further fixes, so atomic fixing.)
3. Replacing a library entirely, but only if a specific type is referenced from the original library. (Concretely, if you reference FakePlatform, you should import the new library testing.dart, not platform.dart.)
4. Or generally, allow any rule to add a new import, and refer to names from that. Having a prefix is fine.

I might be over-complicating. Anything that gets the job done would be fine. It doesn't have to be pretty, it just needs to keep running, then we can clean up the migrated code afterwards, maybe not until we remove the deprecated API.

[dart-github-bot]

Summary: The user needs to automatically migrate code from a deprecated API to a new one. The issue is that the new API uses a class with the same name as the deprecated class, but from a different library. The user wants the analyzer to automatically replace the deprecated class with the new one, including adding the necessary import statement.

[bwilkerson]

@keertip

[keertip]

@lrhn, can you point to a CL with the api changes?

[lrhn]

The code I'm trying to make migrate is in https://github.com/lrhn/platform.dart/tree/3.2

I have a script, tools/check_migration.dart, that tries to check that every deprecation is fixed.

[keertip]

We can add an import while making the fix, that is already supported. Take a look at the test cases here - https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/test/src/services/correction/fix/data_driven/platform_use_case_test.dart.

Are there any other use cases that might not be covered by these test cases?

[lrhn]

Have tried more, and have gotten close, but it seems like I'm not doing the direct thing that I want to do.

What I want to do is replace a deprecated class in an old library with a new class from a new library. Say, very concretely, FakePlatform from package:platform/platform.dart with FakePlatfrom from package:platform/testing.dart. (I'm OK with the new name being FakePlatformMigrationHelper if that avoids name conflicts. I can fix that when we remove the old names.)

I can't do that directly since replacedBy cannot replace classes, only methods, getters, setters and constructors (according to documentation, but also what I'm seeing).

Instead:

• I'm replaceing each constructor of FakeClass in the old library with a constructor of FakePlatformMigrationHelper from the new library, which allows me to add an import for the testing.dart library.
• And then I am renaming the occurrences of the class as a type to the new name. Which then "works" because I have a constructor call in the same file that adds the import.

A file with no constructor call won't migrate correctly, the rename doesn't add the import.

If the code uses a prefixed for FakePlatform, like prefix.FakePlatform f = prefix.FakePlatform.fromJson(...), then it's (I'd say incorrectly) fixed into prefix.FakePlaformMigrationHelper.nativeFromJson(...), but the new import doesn't have a prefix, so the reference is wrong. The replacedBy operation should probably remove the prefix, it's not a rename. (Using replaceTarget doesn't do anything. I tried. And replaceBy can't use code template, so no chance of doing something custom.) Renames also retain the prefix, so prefixes just don't work with what I'm doing.

What I would want is:

• Let replacedBy for constructors to remove a prefix on the source. (Or only do so if a uri is supplied?)
• Ability to use replacedBy on type declarations (classes is enough for this).
• Which replaces every occurrence of FakePlatform and prefix.FakePlatform that denote the old class with FakePlatformMigrationHelper and adds an import of the testing.dart library.
• Or even better, allow the added import to specify a prefix, and replace with $prefix.FakePlatformMigrationHelper.

Without the ability to replace a class with a class/type from a different library, which is the actual change, I can only try to hit some of the indirect ways to refer to the type, but it'll be fragile. (I'm guessing nobody uses prefixes, and you probably do use a constructor for the testing-classes if you refer to the type, so it'll likely work for most code, but it's relying on luck.)

In the longer term, I'd like to be able to change new Platform.native(args) to new Platform.fromNative(new NativePlatform(args)). I don't see any current way to do that either. (Just addBefore and addAfter could help. I don't need to know generics or anything, but a code template would be safer in general.)

[bwilkerson]

It would definitely be better to expand the capabilities of the replaceBy transform. The primary reason for the design of the transforms is to express the semantics of what's changed and allow the underlying implementation cover all the corner cases so that data file authors don't have to.

@keertip

[lrhn]

It can be tricky when the change is large. The change space can be pretty big.

Consider going from fakePlatform.copyWith(args) to FakePlatform.fromNative(fakePlatform.native!.copyWith(args)).

A code template could be the easy choice for a lot of this.

If a "method invocation" target captures the receiver expression (or namespace reference if static), methodName, type arguments and argument list, and you could write a template of

"FakePlatform.fromNative(@{receiver}.nativePlatform!.@{methodName}(@{arguments}))"

could let you build anyting around the parts of the match. Or instead of prefixing, I could have a helper getter:

"@{receiver}.nativePlatform!.@{methodName}(@{arguments}).toPlatform()"

Putting something after the arguments is what's hard right now. It's going to give long lines, should probably reformatt after fixing!

(And who doesn't want to implement a full-fledged template engine! :grin:)