Skip to content

tests: cover more exported_private_dependencies cases #144082

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mladedav
Copy link
Contributor

@mladedav mladedav commented Jul 17, 2025

This PR adds tests for all missing cases from #71043 and some on top of that. I believe with this, that issue can be closed.

Some of the lints can be improved, e.g. provided_impl_trait and impl From<PublicWithStdImpl> for OtherType lint twice.

cc @epage in case you want to double check I didn't miss anything.

@rustbot
Copy link
Collaborator

rustbot commented Jul 17, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 17, 2025
pub OtherType,
//~^ ERROR type `OtherType` from private dependency 'priv_dep' in public interface [exported_private_dependencies]
OtherType,
PubType,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for symmetry with the struct, should this field be pub?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, fixed.

@mladedav mladedav force-pushed the dm/pub-priv-tests branch from f8cf216 to 2390abe Compare July 18, 2025 08:20
@mladedav
Copy link
Contributor Author

This would conflict with #144098 which will most likely break the tests of RPITIT again.

So I'll convert this to a draft so that this doesn't add work there if it's merged first and once that is merged, I'll update this.

It might also mean that the parent issue will still have to be kept open even after mergint this since the current fix in the linked PR breaks linting about using traits from private dependencies again.

@mladedav mladedav marked this pull request as draft July 18, 2025 08:37
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2025
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Jul 19, 2025

☔ The latest upstream changes (presumably #144166) made this pull request unmergeable. Please resolve the merge conflicts.

@mladedav mladedav force-pushed the dm/pub-priv-tests branch from 2390abe to 725523e Compare July 20, 2025 19:12
@mladedav mladedav marked this pull request as ready for review July 20, 2025 19:14
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 20, 2025
@mladedav
Copy link
Contributor Author

After #144098, RPITIT is not linted again so that's the last thing that is missing (again). There's a link to a crater run in that PR so if that goes well, that too will be properly linted.

Regardless, this can be merged now and #71043 would just have to wait for the resolution of the crater run and the changes behind that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants