Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration tool null safety detection fails when tests use path imports #44394

Closed
mit-mit opened this issue Dec 4, 2020 · 2 comments
Closed
Assignees
Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). NNBD Issues related to NNBD Release P3 A lower priority bug or feature request

Comments

@mit-mit
Copy link
Member

mit-mit commented Dec 4, 2020

If a test imports the library under test with a path dependency, then the migration tool fails to identify it as the library being migrated, and fails. While these is not the recommended style for imports in tests, seems like we shouldn't fail though? cc @stereotype441

<package>/test/<package>_test.dart:

import '../lib/lix.dart';

...

Try to migrate:

$ dart migrate
...
Analyzing project...
[-----------------------------------------------------------------------------------------------------------------------------------------|]Bad state: Error: package has unmigrated dependencies.

Before migrating your package, we recommend ensuring that every library it
imports (either directly or indirectly) has been migrated to null safety, so
that you will be able to run your unit tests in sound null checking mode.  You
are currently importing the following non-null-safe libraries:

  file:///Users/mit/dev/null_safety_demos/calculate_lix/lib/lix.dart
@mit-mit mit-mit added the NNBD Issues related to NNBD Release label Dec 4, 2020
@stereotype441 stereotype441 added area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). P3 A lower priority bug or feature request labels Dec 4, 2020
@stereotype441 stereotype441 self-assigned this Sep 14, 2022
@stereotype441
Copy link
Member

I'm unable to reproduce this. I believe there have been changes made to the analyzer to address this sort of problem since the sisue was filed in December of 2020, so I think it's most likely that the issue was fixed by those changes.

I'm adding a unit test to the migration tool to ensure that the fix doesn't regress (https://dart-review.googlesource.com/c/sdk/+/259200). Once that lands I'll close the issue. I see that a lot of people have given this bug a "thumbs up". If any of you are still experiencing this problem, please feel free to re-open.

copybara-service bot pushed a commit that referenced this issue Sep 14, 2022
This issue (Migration tool null safety detection fails when tests use
path imports) was reported in December of 2020 and I can't reproduce
it.  I believe there have been changes in analyzer package resolution
since then, aimed precisely at helping users who use this sort of
improper path import.  So it is likely that those changes fixed this
bug.

I'm adding a test case to ensure that the bug isn't accidentally
un-fixed by future changes.

Bug: #44394
Change-Id: I81490e545aa41196c3c69bc4d74cd481079d59ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259200
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
@stereotype441
Copy link
Member

Unit test has landed, so closing this issue as irreproducible. Once again, please feel free to re-open if you're still experiencing this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migration (deprecated) Deprecated: this label is no longer actively used (was: issues with the `dart migrate` tool). NNBD Issues related to NNBD Release P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

2 participants