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

Implement better help for clippy-driver #4175

Merged
merged 11 commits into from Jun 13, 2019
Merged

Implement better help for clippy-driver #4175

merged 11 commits into from Jun 13, 2019

Conversation

yaahc
Copy link
Member

@yaahc yaahc commented Jun 5, 2019

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 6, 2019
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM

cc @Manishearth since you opened the corresponding issue.

src/lintlist.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
src/lintlist.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
clippy_dev/src/main.rs Outdated Show resolved Hide resolved
src/driver.rs Outdated Show resolved Hide resolved
@yaahc
Copy link
Member Author

yaahc commented Jun 8, 2019

still missing the default lint levels and the group output but its shaping up :)

Screenshot from 2019-06-08 14-24-46

@yaahc
Copy link
Member Author

yaahc commented Jun 8, 2019

and now we have groups, just have to find lint levels then its ready for review and mergemeby

groups

src/driver.rs Outdated Show resolved Hide resolved
src/driver.rs Outdated Show resolved Hide resolved
src/driver.rs Outdated Show resolved Hide resolved
src/driver.rs Outdated Show resolved Hide resolved
src/driver.rs Outdated
print_lints(&lints);

let max_name_len = std::cmp::max(
"warnings".len(),
Copy link
Member Author

Choose a reason for hiding this comment

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

is there a clippy::warnings group?

Copy link
Member

Choose a reason for hiding this comment

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

No, there's only clippy::all which is all warn-by-default and deny-by-default lints. You can find all available lint groups in the README:

  • clippy::all
  • clippy::correctness (deny-by-default)
  • clippy::style (warn)
  • clippy::complexity (warn)
  • clippy::perf (warn)
  • clippy::pedantic (allow)
  • clippy::nursery (allow)
  • clippy::cargo (allow)

There's also clippy::restriction, which is allow-by-default and lints where it doesn't make sense to enable the whole group.

Copy link
Member Author

@yaahc yaahc Jun 10, 2019

Choose a reason for hiding this comment

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

okay, I bring this up because rustc has a hard coded bit for their warnings group because its otherwise not included with their other lint groups, it looks like we've got everything in our groups except all, so I'm going to treat all like our version of warning and include it in the output the same way that rustc does.

Everything else will get included programatically.

src/driver.rs Outdated Show resolved Hide resolved
@oli-obk
Copy link
Contributor

oli-obk commented Jun 13, 2019

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 13, 2019

📌 Commit 2719c1e has been approved by oli-obk

@bors
Copy link
Collaborator

bors commented Jun 13, 2019

⌛ Testing commit 2719c1e with merge bd39cea...

bors added a commit that referenced this pull request Jun 13, 2019
Implement better help for clippy-driver

#4173
sorted_usable_lints.sort_by_key(|lint| lint.name.clone());

std::fs::write(
"../src/lintlist/mod.rs",
Copy link
Member

Choose a reason for hiding this comment

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

After the fact review:

Shouldn't this also be done with the replace_region_in_file function and then only update the content in the ALL_LINTS array? When a PR adds a lint, this file needs to get updated like README.md, ... If the update is done with the replace_region_in_file function (like below), travis will fail if it's not updated. cc @phansch

@bors
Copy link
Collaborator

bors commented Jun 13, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing bd39cea to master...

@bors bors merged commit 2719c1e into rust-lang:master Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants