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

lint audit: Implementation + move one lint #2579

Merged
merged 5 commits into from Mar 29, 2018

Conversation

Projects
None yet
2 participants
@oli-obk
Collaborator

oli-obk commented Mar 27, 2018

I still need to figure out how to make warn(clippy) cause warn(clippy_style, clippy_perf, ...)

oli-obk added some commits Mar 27, 2018

@Manishearth

LGTM. There are a bunch of cases where I think another category may also apply; but it's up to you if we should choose that instead. Unsure which.

pub DOUBLE_PARENS, Warn,
declare_clippy_lint! {
pub DOUBLE_PARENS,
complexity,

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

perhaps style? idk. complexity makes sense too

This comment has been minimized.

@oli-obk

oli-obk Mar 29, 2018

Collaborator

My idea of complexity is anything that makes the code look more complex than it is, and everyone can agree that it should be improved. It's a fuzzy boundary, but I want to keep complexity lints as lints that no matter of personal style references, everyone wants to make those changes.

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

ah, that's a reasonable boundary, makes sense

@@ -22,9 +22,9 @@ use utils::{camel_case_from, camel_case_until, in_macro};
/// HummingbirdCake,
/// }
/// ```
declare_lint! {
declare_clippy_lint! {

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

While we're here, we should document it such that the difference between the two is obvious

This comment has been minimized.

@oli-obk

oli-obk Mar 29, 2018

Collaborator

done

pub FALLIBLE_IMPL_FROM, Allow,
declare_clippy_lint! {
pub FALLIBLE_IMPL_FROM,
nursery,

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

should this still be nursery?

(did TryFrom stabilize?)

This comment has been minimized.

@oli-obk

oli-obk Mar 29, 2018

Collaborator

I don't think it stabilized

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

belongs in the nursery then, yes? It's allow because we don't want to make it warn yet, not because we don't want to make it warn ever

pub POSSIBLE_MISSING_COMMA,
Warn,
style,

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

correctness?

pub TOO_MANY_ARGUMENTS,
Warn,
style,

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

perhaps also complexity? idk

This comment has been minimized.

@oli-obk

oli-obk Mar 29, 2018

Collaborator

neither do I, I'll put it there, if ppl complain we can put it back.

pub BOOL_COMPARISON,
Warn,
style,

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

complexity?

pub PRINTLN_EMPTY_STRING,
Warn,
style,

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

the lint description here is incorrect

pub BOX_VEC,
Warn,
complexity,

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

perf?

This comment has been minimized.

@oli-obk

oli-obk Mar 29, 2018

Collaborator

both ^^ it's not just slower or requires more memory, it's just weird

pub LINT_AUTHOR,
Warn,
style, // ok, this is not a style lint, but it's also a noop without the appropriate attribute

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

Can we keep it in the nursery or internal instead?

pub DEEP_CODE_INSPECTION,
Warn,
style, // not a style lint, but essentially a noop without the appropriate attribute

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2018

Collaborator

ditto wrt nursery

oli-obk added some commits Mar 29, 2018

@oli-obk oli-obk merged commit a47734c into master Mar 29, 2018

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@oli-obk oli-obk deleted the lint_audit_mcve branch Mar 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment