Support lint tool names in rustc command line options #86639

eholk · 2021-06-25T21:37:19Z

When rustc is running without a lint tool such as clippy enabled, options for lints such as clippy::foo are meant to be ignored. This was already working for those specified by attrs, such as #![allow(clippy::foo)], but this did not work for command line arguments like -A clippy::foo. This PR fixes that issue.

Note that we discovered this issue while discussing rust-lang/cargo#5034.

Fixes #86628.

rust-highfive · 2021-06-25T21:37:22Z

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

camsteffen · 2021-06-30T05:36:22Z

Does this mean we no longer need -- in cargo clippy -- -W clippy::lint?

eholk · 2021-06-30T16:36:52Z

Does this mean we no longer need -- in cargo clippy -- -W clippy::lint?

It shoudn't have any change there. This simply makes it so you can do rust -W clippy::lint without getting an error message.

compiler/rustc_lint/src/context.rs

petrochenkov · 2021-07-01T13:59:24Z

The src/tools/rust-analyzer submodule update looks unintentional and needs to be removed.

eholk · 2021-07-02T00:13:07Z

Thanks for the comments, @petrochenkov. I removed the rust-analyzer submodule update.

eholk · 2021-07-02T00:13:30Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

src/test/ui/lint/command-line-lint-tool.rs

This adds a ui test to make sure rustc accepts lint arguments such as `-A clippy::foo` when clippy is disabled.

This shares a little more code between checking command line and attribute lint specifications.

eholk · 2021-07-07T00:23:34Z

This is a little better now, although probably not quite ready to merge. I think there's some more cleanup still to do. Is it at least on the right path for solving the problem?

I'm also having an issue with my tests. Even after adding the .stderr file, I get the following failure. Any ideas?

unexpected errors (from JSON output): [
    Error {
        line_num: 1,
        kind: Some(
            Error,
        ),
        msg: "1:1: 1:1: unknown lint tool: `unknown_tool` [E0602]",
    },
    Error {
        line_num: 1,
        kind: Some(
            Error,
        ),
        msg: "1:1: 1:1: unknown lint tool: `unknown_tool` [E0602]",
    },
]

petrochenkov · 2021-07-07T13:50:15Z

I'm also having an issue with my tests. Even after adding the .stderr file, I get the following failure. Any ideas?

The tests are missing //~ ERROR annotations.
Although, since the errors are not attached to spans you'll probably have to use // error-pattern: unknown lint tool: `unknown_tool` instead.

src/test/ui/lint/command-line-register-unknown-lint-tool.rs

petrochenkov · 2021-07-07T13:59:43Z

Tests need some adjustments, but otherwise LGTM.

Previously this was using a `.stderr` file, but that does not seem to work for all cases. This change uses `// error-pattern:` instead.

This change merges `check_lint_and_tool_name` into `check_lint_name` in order to avoid having two very similar functions. Also adds the `.stderr` file back for the test case, since apparently it is still needed.

eholk · 2021-07-07T17:57:21Z

Okay, this should be ready to go now.

@rustbot label -S-waiting-on-author +S-waiting-on-review

petrochenkov · 2021-07-07T22:37:44Z

Thanks!
@bors r+

bors · 2021-07-07T22:37:46Z

📌 Commit 4a83a93 has been approved by petrochenkov

Support lint tool names in rustc command line options When rustc is running without a lint tool such as clippy enabled, options for lints such as `clippy::foo` are meant to be ignored. This was already working for those specified by attrs, such as `#![allow(clippy::foo)]`, but this did not work for command line arguments like `-A clippy::foo`. This PR fixes that issue. Note that we discovered this issue while discussing rust-lang/cargo#5034. Fixes rust-lang#86628.

Rollup of 9 pull requests Successful merges: - rust-lang#86639 (Support lint tool names in rustc command line options) - rust-lang#86812 (Recover from `&dyn mut ...` parse errors) - rust-lang#86917 (Add doc comment for `impl From<LayoutError> for TryReserveError`) - rust-lang#86925 (Add self to mailmap) - rust-lang#86927 (Sync rustc_codegen_cranelift) - rust-lang#86932 (Fix ICE when misplaced visibility cannot be properly parsed) - rust-lang#86933 (Clean up rustdoc static files) - rust-lang#86955 (Fix typo in `ops::Drop` docs) - rust-lang#86956 (Revert "Add "every" as a doc alias for "all".") Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned petrochenkov Jun 25, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 25, 2021

This comment has been minimized.

Sign in to view

petrochenkov reviewed Jul 1, 2021

View reviewed changes

compiler/rustc_lint/src/context.rs Show resolved Hide resolved

petrochenkov 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 1, 2021

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 2, 2021

petrochenkov reviewed Jul 2, 2021

View reviewed changes

src/test/ui/lint/command-line-lint-tool.rs Outdated Show resolved Hide resolved

petrochenkov 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 2, 2021

This comment has been minimized.

Sign in to view

eholk added 3 commits July 6, 2021 15:49

Add ui test for command line lints with tool names

65b28a9

This adds a ui test to make sure rustc accepts lint arguments such as `-A clippy::foo` when clippy is disabled.

Parse tool name for command line lint options

1e0db4c

Unify lint tool and lint name checking

8b4f538

This shares a little more code between checking command line and attribute lint specifications.

eholk force-pushed the lint-tool branch from 74c5f78 to 8b4f538 Compare July 7, 2021 00:21

This comment has been minimized.

Sign in to view

petrochenkov 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 7, 2021

petrochenkov reviewed Jul 7, 2021

View reviewed changes

src/test/ui/lint/command-line-register-unknown-lint-tool.rs Outdated Show resolved Hide resolved

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2021

petrochenkov added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 7, 2021

Fix test case

5413d2e

Previously this was using a `.stderr` file, but that does not seem to work for all cases. This change uses `// error-pattern:` instead.

This comment has been minimized.

Sign in to view

Cleanup: unify lint name checking

4a83a93

This change merges `check_lint_and_tool_name` into `check_lint_name` in order to avoid having two very similar functions. Also adds the `.stderr` file back for the test case, since apparently it is still needed.

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 7, 2021

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 7, 2021

JohnTitor mentioned this pull request Jul 7, 2021

Rollup of 9 pull requests #86962

Closed

JohnTitor mentioned this pull request Jul 8, 2021

Rollup of 9 pull requests #86966

Merged

bors merged commit c2d3f5f into rust-lang:master Jul 8, 2021

rustbot added this to the 1.55.0 milestone Jul 8, 2021

eholk deleted the lint-tool branch July 8, 2021 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support lint tool names in rustc command line options #86639

Support lint tool names in rustc command line options #86639

eholk commented Jun 25, 2021

rust-highfive commented Jun 25, 2021

This comment has been minimized.

camsteffen commented Jun 30, 2021

eholk commented Jun 30, 2021

petrochenkov commented Jul 1, 2021

eholk commented Jul 2, 2021

eholk commented Jul 2, 2021

This comment has been minimized.

eholk commented Jul 7, 2021

This comment has been minimized.

petrochenkov commented Jul 7, 2021

petrochenkov commented Jul 7, 2021

This comment has been minimized.

eholk commented Jul 7, 2021 •

edited

Loading

petrochenkov commented Jul 7, 2021

bors commented Jul 7, 2021

Support lint tool names in rustc command line options #86639

Support lint tool names in rustc command line options #86639

Conversation

eholk commented Jun 25, 2021

rust-highfive commented Jun 25, 2021

This comment has been minimized.

camsteffen commented Jun 30, 2021

eholk commented Jun 30, 2021

petrochenkov commented Jul 1, 2021

eholk commented Jul 2, 2021

eholk commented Jul 2, 2021

This comment has been minimized.

eholk commented Jul 7, 2021

This comment has been minimized.

petrochenkov commented Jul 7, 2021

petrochenkov commented Jul 7, 2021

This comment has been minimized.

eholk commented Jul 7, 2021 • edited Loading

petrochenkov commented Jul 7, 2021

bors commented Jul 7, 2021

eholk commented Jul 7, 2021 •

edited

Loading