-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Analyzer doesn't properly handle parts generated using build_to:cache #51087
Comments
This comment was marked as outdated.
This comment was marked as outdated.
sdk/279395 contains a failing test. Please note that if the folder is I wonder whether the files outside |
I'm confused. The original post says that the part file contains:
Is that a typo, or does the part actually claim to be a part of itself? I would have expected the generated file to contain
|
@asashour Oh good catch! I didn't realize the issue didn't occur for files inside Sorry about that @bwilkerson, that is indeed a typo; it should be: part of 'foo.dart'; I'll upate the original post |
Does the same error message get generated? I ask because the error message indicates that analyzer thinks |
Yes, it does. Yeah, I was confused by that as well |
@scheglov Does the analyzer know how to handle files in (I haven't been able to reproduce the original issue. I get a different error message suggesting that my environment isn't complete.) |
Oh sorry, I forgot to mention that sometimes I would have to run "Reanalyze Dart Sources" in my IDE to get it to pick up on the changes after running a build. I can't recall exactly, but there may have been a few times where even that didn't work. That's one of the reasons I put up the CI runs in that reduced test case package |
Thank you for the reproduction as a unit test, @asashour! Yes, it looks inconsistent that we can resolve |
Bug #51087 Change-Id: I9ee382bb5abe05624e7b61783bf4f50214d67ab0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279395 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
…hToUri() Bug: #51087 Change-Id: I5f56dc5c82e22a71b3d871ae2ef51f2acfa5eee1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279562 Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
The fix is on bleeding edge. If you have a chance to try using an SDK built from bleeding edge, please let us know whether the problem still occurs. If we know that it's fixed we can investigate cherry picking into 2.19. Otherwise the change won't reach users until 3.0. |
I tested it with the package in the OP and it works with the analyzer. However, CFE ignores both scenarios, please see #51105 |
Could I request this be patched to a 2.18.8 release as well as 2.19 🙏 ? |
The CFE works fine, my testing for it was incorrect. So if this seems important, it should be probably cherry picked. |
In addition to @asashour's testing, I reran a build on that test package with a dev build of the Dart SDK that included this fix, and it passed! https://github.com/greglittlefield-wf/generated_part_issue_repro/actions/runs/3991654264/jobs/6902940045 Thanks for fixing the issue so quickly! If it could be cherry-picked into 2.19, that would be awesome. Like @robbecker-wf mentioned, 2.18 would've been ideal but we'll take what we can get 🙂. For more context on why 2.18, since our build fails on 2.19 due to our dependency graph still being on 1.x of the analyzer package, which throws when analysis hits constructor tearoffs in the Dart 2.19 core library. We'll just need to upgrade analyzer further. |
A 2.19 cherry pick for a dot release is absolutely on the table. |
…olver.pathToUri() Bug: #51087 Change-Id: I1d141ef918fa53a86f31d331f6586367dfb99953 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/279562 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280045 Commit-Queue: Kevin Chisholm <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]>
When a builder generates parts using build_to:cache instead of build_to:source, and the parts use URIs in their
part of
directives, the part files don't get recognized correctly and you get apart_of_different_library
error.This issue seems to be specific to the analyzer, and does not seem to impact running in the VM or compiling to JS files that depend on those generated parts.
For example, if you have a file
foo.dart
which pulls in a generated file:and the generated file looks like:
Then you get an error on the
part
URI in the foo.dart file that looks like:I put together a package with a simple builder that reproduces this issue, which can be seen on this line.
However, if the generated part points to the main library using its library name, there's no error. For example, if you have a file
foo.dart
which pulls in a generated file:and the generated file looks like:
In that case, everything seems to work fine. Example of that behavior here.
Impact
This impacts consumers of over_react, which uses build_to:cache for its generated code.
It'd be ideal if this issue were fixed in the Dart SDK, and released in a 2.x version. I'd be happy to attempt to contribute a fix if that would be helpful!
If that's not feasible, then the following workarounds come to mind:
While switching to build_to:source would be possible, it'd be no small mount of work to migrate consumers. I think ideally we'd stick with build_to:cache until Dart macros are available :).
Dart SDKs
I tested this issue in different stable Dart SDKs until I could narrow down where behavior changed.
In Dart <=2.15.1, this worked as expected.
In Dart >=2.16.0 <=2.17.7,
part
directives in libraries that point to generated parts haveuri_has_not_been_generated
errors, regardless of whether a part uses a URI or library.In Dart >=2.18.0,
part
directives in libraries that point to generated parts havepart_of_different_library
errors, but only if those parts uses URI in theirpart of
directives, and not if they use library names.This issue is still present the beta (2.19.0-444.4.beta) and dev (3.0.0-128.0.dev) channels as well.
You can see CI runs of that reduced test case package on different Dart SDKs here.
The text was updated successfully, but these errors were encountered: