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 info message for -Wall command #48765

Merged
merged 3 commits into from Mar 14, 2018

Conversation

Phlosioneer
Copy link
Contributor

Users coming from other languages (namely C and C++) often expect
to use a -Wall flag. Rustc doesn't support that, and previously it
simply printed that it didn't recognize the "all" lint.

This change makes rustc print out a help message, explaining:

  • Why there is no -Wall flag
  • How to view all the available warnings
  • Point out that the most commonly used warning is -Wunused
  • Instead of using a command-line flag, the user should consider
    a !#[warn(unused)] directive in the root of their crate.

I tried to keep the language consistent with the other usage help. Comment if I should change anything.

closes #10234, if accepted.

Users coming from other languages (namely C and C++) often expect
to use a -Wall flag. Rustc doesn't support that, and previously it
simply printed that it didn't recognize the "all" lint.

This change makes rustc print out a help message, explaining:
- Why there is no -Wall flag
- How to view all the available warnings
- Point out that the most commonly used warning is -Wunused
- Instead of using a command-line flag, the user should consider
  a !#[warn(unused)] directive in the root of their crate.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cramertj (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 6, 2018
@sanxiyn
Copy link
Member

sanxiyn commented Mar 6, 2018

Failed tidy.

tidy error: src/librustc_driver/lib.rs:1392: trailing whitespace

@Phlosioneer
Copy link
Contributor Author

Checks passed.

@cramertj
Copy link
Member

cramertj commented Mar 7, 2018

Not sure who to assign this to... maybe

r? @estebank

@rust-highfive rust-highfive assigned estebank and unassigned cramertj Mar 7, 2018
Use `rustc -W help` to see all available lints. The most used lints that are not
enabled by default covered by -Wunused; however, the best practice is to put
warning settings in the crate root using `#![warn(unused)]` instead of using
the command line flag directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with the feature. I would like to have a native speaker take a look over this. cc @rust-lang/docs

Maybe something closer to:

The flag `-Wall` does not exist in `rustc`. Most useful lints are enabled by default.
Use `rustc -W help` to see all available lints. The most used lints that are not
enabled by default are covered by `-Wunused`; however, the best practice is to
put warning settings in the crate root using `#![warn(LINT_NAME)]` instead of using
the command line flag directly.

Copy link
Member

Choose a reason for hiding this comment

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

The most used lints that are not enabled by default are covered by -Wunused

Is there any particular reason why -Wunused is being called out here? Why unused vs. other lint checks?

the best practice is to put warning settings...

I might say "it's more common to put warning settings...", but I don't feel strongly

Other than that, seems good to me 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused turns on half of the allowed-by-default lints that I've ever seen used:

  • unused-qualifications
  • unused-results
  • unused-import-braces
  • unused-extern-crates

The others that I've personally seen used are:

  • missing-docs
  • trivial-casts
  • trivial-numeric-casts
  • unreachable-pub

And given that a LOT of crates in the ecosystem have #![warn(unused)], I thought it was the most relevant one to mention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, looking at the -W help output, -Wunused only turns on unused-extern-crates... I thought it did all the unused-* lints?
I might have to go step-up my #![warn(unused-*)] lines in my projects.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, looking at the -W help output, -Wunused only turns on unused-extern-crates

taken from rustc -W help

Lint groups provided by rustc:

                   name  sub-lints
                   ----  ---------
               warnings  all lints that are set to issue warnings
              bad-style  non-camel-case-types, non-snake-case, non-upper-case-globals
    future-incompatible  private-in-public, pub-use-of-private-extern-crate, patterns-in-fns-without-body, safe-extern-statics, invalid-type-param-default, legacy-directory-ownership, legacy-imports, legacy-constructor-visibility, resolve-trait-on-defaulted-unit, missing-fragment-specifier, illegal-floating-point-literal-pattern, anonymous-parameters, parenthesized-params-in-types-and-modules, late-bound-lifetime-arguments, safe-packed-borrows, incoherent-fundamental-impls, coerce-never, tyvar-behind-raw-pointer
                 unused  unused-imports, unused-variables, unused-assignments, dead-code, unused-mut, unreachable-code, unreachable-patterns, unused-must-use, unused-unsafe, path-statements, unused-attributes, unused-macros, unused-allocation, unused-doc-comment, unused-extern-crates, unused-features, unused-parens


Compiler plugins can provide additional lints and lint groups. To see a listing of these, re-run `rustc -W help` with a crate filename.

The reference to -Wunused was removed, and some phrasing was changed.
@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 13, 2018

📌 Commit c1337cd has been approved by estebank

@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 Mar 13, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 14, 2018
…, r=estebank

Add info message for -Wall command

Users coming from other languages (namely C and C++) often expect
to use a -Wall flag. Rustc doesn't support that, and previously it
simply printed that it didn't recognize the "all" lint.

This change makes rustc print out a help message, explaining:
- Why there is no -Wall flag
- How to view all the available warnings
- Point out that the most commonly used warning is -Wunused
- Instead of using a command-line flag, the user should consider
  a !#[warn(unused)] directive in the root of their crate.

I tried to keep the language consistent with the other usage help. Comment if I should change anything.

closes rust-lang#10234, if accepted.
bors added a commit that referenced this pull request Mar 14, 2018
Rollup of 12 pull requests

- Successful merges: #48765, #48831, #48840, #48964, #48970, #48971, #48981, #48988, #48991, #48966, #48993, #48874
- Failed merges:
@bors bors merged commit c1337cd into rust-lang:master Mar 14, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Mar 21, 2018

Might be worth it to mention clippy here as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc -Wall to show all possible warnings
8 participants