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

Add Cargo.toml context to exported-private-dependencies lints #13096

Open
epage opened this issue Dec 1, 2023 · 5 comments
Open

Add Cargo.toml context to exported-private-dependencies lints #13096

epage opened this issue Dec 1, 2023 · 5 comments
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@epage
Copy link
Contributor

epage commented Dec 1, 2023

Problem

Right now the lint just says

warning: trait `FooTrait` from private dependency 'foo' in public interface
  --> src/lib.rs:25:1
   |
25 | pub fn use_foo_trait<F: foo::FooTrait>(_: Local) {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

rustc has no knowledge of Cargo.toml to provide additional context

Proposed Solution

Build on #13095 and #12235 to inject a note in the lint to highlight the dependency in Cargo.toml, saying the dependency is private

Notes

No response

@epage epage added Command-publish A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review labels Dec 1, 2023
@ShoyuVanilla
Copy link
Member

Hi, I'd like to work on this issue.
I believe this issue should be addressed by providing the Cargo.toml path and the span for the problematic dependency as metadata in cargo, then using this data to construct the actual warning message in rustc. Does this approach seem correct?

@epage
Copy link
Contributor Author

epage commented Jan 2, 2025

If I'm understanding correctly, you are suggesting Cargo proactively get the span for all dependencies, rustc add a new CLI flag for getting dependency location and span information, and for Cargo to pass that flag to rustc?

That would lead to a very specialized interface in rustc and would make #13095 much harder.

@ShoyuVanilla
Copy link
Member

That would lead to a very specialized interface in rustc and would make #13095 much harder.

That sounds problematic 🤔 What would be most appropriate approach? I think that there would be two directions for implementing this as rustc doesn't know about Cargo.toml

  1. rustc queries the Cargo.toml span to cargo: My original suggestion, which seems problematic
  2. cargo intercepts rustc's warning message and modify it to include the Cargo.toml span: Not so robust, but implementable

@epage
Copy link
Contributor Author

epage commented Jan 2, 2025

in #13095, I suggested the latter.

@ShoyuVanilla
Copy link
Member

Okay, I'll try in that way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

No branches or pull requests

2 participants