Skip to content

Add help for crate arg when crate name is invalid#114058

Merged
bors merged 1 commit intorust-lang:masterfrom
chenyukang:yukang-fix-113981-crate-arg
Jul 25, 2023
Merged

Add help for crate arg when crate name is invalid#114058
bors merged 1 commit intorust-lang:masterfrom
chenyukang:yukang-fix-113981-crate-arg

Conversation

@chenyukang
Copy link
Member

@chenyukang chenyukang commented Jul 25, 2023

Fixes #113981

@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2023

r? @oli-obk

(rustbot has picked a reviewer for you, use r? to override)

@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 25, 2023
@chenyukang chenyukang force-pushed the yukang-fix-113981-crate-arg branch 2 times, most recently from 0ae166d to 8bf4576 Compare July 25, 2023 14:26
fmease
fmease reviewed Jul 25, 2023
View reviewed changes
@chenyukang chenyukang force-pushed the yukang-fix-113981-crate-arg branch from dc7fd43 to e2402f7 Compare July 25, 2023 14:37
oli-obk
oli-obk reviewed Jul 25, 2023
View reviewed changes
tests/run-make/crate-name-help/issue-113981.b.rs Outdated
Copy link
Contributor

@oli-obk oli-obk Jul 25, 2023

Choose a reason for hiding this comment

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

Please check if this could also be a ui test. I'm certain I've accidentally written filenames like that before in ui tests and had to edit them, maybe it just all works out?

Copy link
Member Author

@chenyukang chenyukang Jul 25, 2023

Choose a reason for hiding this comment

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

I tried to use it as a ui test, we have a tidy check before git push:

tidy error: Stray file with UI testing output: "/Users/yukang/rust/tests/ui/suggestions/issue-113981.b.stderr"

Copy link
Member

@fmease fmease Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh lol, that might be caused by rust-tidy assuming that the b in issue-113981.b.stderr is the revision of the test.

Copy link
Member Author

@chenyukang chenyukang Jul 25, 2023

Choose a reason for hiding this comment

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

yes, from the code it's expecting the .rs file of issue-113981.rs exists:

rust/src/tools/tidy/src/ui_tests.rs

Line 120 in 23405bb

tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path);

but we need multiple . in the file name.

Copy link
Member

@fmease fmease Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Right and there's no suitable // ignore-tidy-xxx for this case. Apparently, you can place ignore-tidy in the file path to make tidy ignore it? Not sure how that is supposed to work. E.g. see tests/ui/parser/shebang/issue-71471-ignore-tidy.rs. You might be able to do something similar 🤷‍♂️

Copy link
Member Author

@chenyukang chenyukang Jul 25, 2023

Choose a reason for hiding this comment

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

thanks for the tip, just made a quick fix 🤔
ab36d0e

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 25, 2023
@chenyukang chenyukang force-pushed the yukang-fix-113981-crate-arg branch 3 times, most recently from 78d00bb to ab36d0e Compare July 25, 2023 15:42
fmease
fmease approved these changes Jul 25, 2023
View reviewed changes
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

one nit & maybe commit squashing, then it's good from my side

@chenyukang chenyukang force-pushed the yukang-fix-113981-crate-arg branch from ab36d0e to e3feab8 Compare July 25, 2023 15:48
@fmease
Copy link
Member

fmease commented Jul 25, 2023 •
edited
Loading

Oh, and if you could rename the test file maybe (CC #113345) to not just consist of the issue number? Sorry I forgot. Only if you like though. If you do, you can put // issue: 113981 inside the file.

@chenyukang chenyukang force-pushed the yukang-fix-113981-crate-arg branch from e3feab8 to d326459 Compare July 25, 2023 15:53
@chenyukang chenyukang force-pushed the yukang-fix-113981-crate-arg branch from d326459 to 58fb785 Compare July 25, 2023 15:57
@chenyukang
Copy link
Member Author

chenyukang commented Jul 25, 2023

yes, it's a better style, done!

@chenyukang chenyukang force-pushed the yukang-fix-113981-crate-arg branch from 58fb785 to e0c479e Compare July 25, 2023 16:05
@oli-obk
Copy link
Contributor

oli-obk commented Jul 25, 2023

@bors r=fmease,oli-obk

@bors
Copy link
Collaborator

bors commented Jul 25, 2023

📌 Commit e0c479e has been approved by fmease,oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 25, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2023
@bors
Auto merge of rust-lang#114063 - matthiaskrgr:rollup-c90czu6, r=matth…
18fa7b9 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#114008 (coverage: Obtain the `__llvm_covfun` section name outside a per-function loop)
 - rust-lang#114014 (builtin_macros: expect raw strings too)
 - rust-lang#114043 (docs(LazyLock): add example pass local LazyLock variable to struct)
 - rust-lang#114051 (Add regression test for invalid "unused const" in method)
 - rust-lang#114052 (Suggest `{Option,Result}::as_ref()` instead of `cloned()` in some cases)
 - rust-lang#114058 (Add help for crate arg when crate name is invalid)
 - rust-lang#114060 (abi: unsized field in union - assert to delay bug )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit de5228e into rust-lang:master Jul 25, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 25, 2023
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 31, 2023
@bors
Auto merge of rust-lang#114063 - matthiaskrgr:rollup-c90czu6, r=matth…
61e0aac 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#114008 (coverage: Obtain the `__llvm_covfun` section name outside a per-function loop)
 - rust-lang#114014 (builtin_macros: expect raw strings too)
 - rust-lang#114043 (docs(LazyLock): add example pass local LazyLock variable to struct)
 - rust-lang#114051 (Add regression test for invalid "unused const" in method)
 - rust-lang#114052 (Suggest `{Option,Result}::as_ref()` instead of `cloned()` in some cases)
 - rust-lang#114058 (Add help for crate arg when crate name is invalid)
 - rust-lang#114060 (abi: unsized field in union - assert to delay bug )

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk oli-obk left review comments

@fmease fmease fmease approved these changes

Assignees

@oli-obk oli-obk

Labels

A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.73.0

Development

Successfully merging this pull request may close these issues.

rustc can't read files with more than one dot in filename

5 participants

@chenyukang @rustbot @fmease @oli-obk @bors