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

Mention tool lint attributes #446

Merged
merged 1 commit into from
Oct 16, 2018
Merged

Conversation

Manishearth
Copy link
Member

Being stabilized in rust-lang/rust#54870 , wanted to preserve some documentation. Clippy has more docs on this.

r? @steveklabnik

Tool lints let you use scoped lints, to `allow`, `warn`, `deny` or `forbid` lints of
certain tools.


Copy link
Contributor

Choose a reason for hiding this comment

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

extra new line

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@Havvy
Copy link
Contributor

Havvy commented Oct 13, 2018

This is okay as is. Don't take the rest of what I'm saying as a requirement to make changes, although do tell me if you want to make them. Otherwise, we can merge this as is (though it'd be nice to know Travis will pass...).

With that out of the way, I think that tool lints don't need their own section. Instead, we need a more general section on lint check values that says both identifiers and ident:ident is valid. We should also probably have a grammar for these attributes, like I added for cfg_attr and cfg in #444. That we don't say that the lint names must be valid identifiers is an oversight I did not notice until you posted this PR.

@Manishearth
Copy link
Member Author

Slightly feel they should be in their own section, since the conditional checking of tool lints is important, prefer to merge as-is.

Added an ignore to the test since this is nightly-only right now (stabilized, but hasn't yet reached stable)

@@ -357,6 +357,35 @@ pub mod m3 {
}
```

#### Tool lint attributes (`#[allow(clippy::foo)]`)
Copy link
Contributor

Choose a reason for hiding this comment

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

One nit before I merge: Everything in the parenthesis should not be in the header.

@Manishearth
Copy link
Member Author

Manishearth commented Oct 16, 2018 via email

@Havvy Havvy merged commit 5da9e0b into rust-lang:master Oct 16, 2018
@Havvy
Copy link
Contributor

Havvy commented Oct 16, 2018

💟 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants