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

cargo fix support for exported-private-dependencies #13095

Open
Tracked by #44663
epage opened this issue Dec 1, 2023 · 6 comments
Open
Tracked by #44663

cargo fix support for exported-private-dependencies #13095

epage opened this issue Dec 1, 2023 · 6 comments
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-fix S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review Z-public-dependency Nightly: public-dependency

Comments

@epage
Copy link
Contributor

epage commented Dec 1, 2023

Problem

rust-lang/rfcs#3516 calls for making exported-private-dependencies into an error in new editions. This means it needs to be machine-applicable so cargo fix can migrate it.

The problem is the most useful thing for a machine-applicable fix is to update Cargo.toml but rustc has no knowledge of that.

Proposed Solution

Intercept the message from rustc, before cargo fix (or the "machine applicable lints counter) sees this and add the machine-applicable content.

Notes

No response

@epage epage added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-fix Z-public-dependency Nightly: public-dependency S-triage Status: This issue is waiting on initial triage. S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review labels Dec 1, 2023
@epage
Copy link
Contributor Author

epage commented Dec 1, 2023

What we need is

  • lint code
  • dependency name (toml key)

Looking at the compiler message

{
  "reason": "compiler-message",
  "package_id": "cargo-pub-priv 0.1.0 (path+file:///home/epage/src/personal/dump/cargo-pub-priv)",
  "manifest_path": "/home/epage/src/personal/dump/cargo-pub-priv/Cargo.toml",
  "target": {
    "kind": [
      "lib"
    ],
    "crate_types": [
      "lib"
    ],
    "name": "cargo-pub-priv",
    "src_path": "/home/epage/src/personal/dump/cargo-pub-priv/src/lib.rs",
    "edition": "2021",
    "doc": true,
    "doctest": true,
    "test": true
  },
  "message": {
    "rendered": "warning: trait `FooTrait` from private dependency 'foo' in public interface\n  --> src/lib.rs:11:1\n   |\n11 | pub trait UseFoo: foo::FooTrait {}\n   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n",
    "children": [],
    "code": {
      "code": "exported_private_dependencies",
      "explanation": null
    },
    "level": "warning",
    "message": "trait `FooTrait` from private dependency 'foo' in public interface",
    "spans": [
      {
        "byte_end": 189,
        "byte_start": 158,
        "column_end": 32,
        "column_start": 1,
        "expansion": null,
        "file_name": "src/lib.rs",
        "is_primary": true,
        "label": null,
        "line_end": 11,
        "line_start": 11,
        "suggested_replacement": null,
        "suggestion_applicability": null,
        "text": [
          {
            "highlight_end": 32,
            "highlight_start": 1,
            "text": "pub trait UseFoo: foo::FooTrait {}"
          }
        ]
      }
    ]
  }
}

We have the code but not the dependency name. It'd be ugly and brittle but we could parse the dependency name out of "from private dependency '{name}' in public interface". We ship with rustc so we can make sure this stays up to date as rustc changes, though it'll cause CI failures.

Expectation

  • Parse the json blob, ensuring its a message and has the right code, extracting the dependency name, and constructing a machine applicable lint.
    • Depending on how replacements are encoded, we might need to add an item to children
  • Test to verify we can auto-fix code
  • Test the above with dependency renames
  • Test that the "run cargo fix" message shows up for these with cargo check.

@epage
Copy link
Contributor Author

epage commented Dec 1, 2023

Depending on the details, we might need #13096

@linyihai
Copy link
Contributor

linyihai commented Jan 2, 2024

Is it currently not supported to obtain the specific line number of a certain configuration item?
Give a example, I like to know the line of license and regex in the Cargo.toml.

image

It looks like this issue need this ability to fixed.

And some discussiton can be found in zulip is it possible to include position data for cargo metadata

@epage
Copy link
Contributor Author

epage commented Jan 2, 2024

@Muscraft was going to add code to cargo to get the needed spans in a follow up to #13172. If you need it before, reach out to them for notes they have on the topic

@linyihai
Copy link
Contributor

@Muscraft was going to add code to cargo to get the needed spans in a follow up to #13172. If you need it before, reach out to them for notes they have on the topic

#13172 was merged recently. I'd to take a shot :-) .

@epage
Copy link
Contributor Author

epage commented Jan 12, 2024

Note that I was referring to a PR they'll be working on after that PR. That PR doesn't have the functionality you would need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-fix S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review Z-public-dependency Nightly: public-dependency
Projects
None yet
Development

No branches or pull requests

2 participants