Don't suggest an export if it's a modularity error to use that package #8835
Conversation
8d4c5c1 to
f1a0eb8
Compare
| @@ -468,8 +432,34 @@ class VisibilityCheckerPass final { | |||
| path.has_value(); | |||
| } | |||
| if (!causesCycle && !layeringViolation && !strictDependenciesTooLow) { | |||
| if (wasNotImported) { | |||
| // We failed to import the package that defines the symbol | |||
| if (!isExported) { | |||
There was a problem hiding this comment.
Here, !isExported and !wasImported are treated as exclusive, but it's actually possible for both to be true. In a future PR, I'll handle the !isExported && !wasImported case.
| if (!db.errorHint().empty()) { | ||
| e.addErrorNote("{}", db.errorHint()); | ||
| } |
There was a problem hiding this comment.
We typically put the addErrorNote last, would you mind moving it for consistency?
There was a problem hiding this comment.
Ok, moved the addErrorLine call up.
| @@ -4,13 +4,9 @@ | |||
| #include "absl/synchronization/blocking_counter.h" | |||
| #include "ast/treemap/treemap.h" | |||
| #include "common/concurrency/ConcurrentQueue.h" | |||
| #include "common/sort/sort.h" | |||
| #include "common/strings/formatting.h" | |||
There was a problem hiding this comment.
ooc, were these includes in service of assorted rbi-gen stuff? or do you think they became unused before we deleted the rbi-gen stuff?
There was a problem hiding this comment.
Probably before the rbi-gen stuff. For example, this commit got rid of the call to fast_sort. I assume something similar for the other includes.
a1b2e1b to
f502624
Compare
Replicates what we do for imports for exports.
Review commit-by-commit.
Motivation
Don't give an autocorrect that's incorrect.
Test plan
See included automated tests.